feat: Enrich account activation email with is_enterprise_learner hint - #2674
Conversation
There was a problem hiding this comment.
Pull request overview
Implements two new Open edX filter pipeline steps in edx-enterprise to support enterprise-aware account activation behavior (email context enrichment and redirect handling), and wires them into the enterprise filters configuration.
Changes:
- Added
ActivationEmailEnterpriseContextEnricherto populateis_enterprise_learnerinto activation email message context. - Added
ActivationRedirectEnterpriseStepto clearredirect_urlfor non-enterprise learners after activation. - Added unit tests and registered both pipelines in
ENTERPRISE_FILTERS_CONFIG.
Critical / high severity issue(s):
- The new pipeline steps call
is_enterprise_learner(...)unconditionally, but the module currently setsis_enterprise_learner = NoneonImportError. In non-LMS environments (including this repo’s test environment), this can raiseTypeError: 'NoneType' object is not callableat runtime unless patched.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
enterprise/filters/accounts.py |
Adds the two new account-activation pipeline steps and introduces the cross-boundary is_enterprise_learner import dependency. |
enterprise/settings/common.py |
Registers the new activation-related filter pipelines in ENTERPRISE_FILTERS_CONFIG. |
tests/filters/test_accounts.py |
Adds test coverage for the new activation email context enrichment and redirect-clearing behavior. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2674 +/- ##
=======================================
Coverage 87.04% 87.05%
=======================================
Files 264 264
Lines 17272 17278 +6
Branches 1707 1707
=======================================
+ Hits 15035 15041 +6
Misses 1898 1898
Partials 339 339
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c8143c2 to
8f01aca
Compare
d54cc1a to
4c3eea3
Compare
4c3eea3 to
5877389
Compare
|
Thanks for the pull request, @marlonkeating! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
cb13dd0 to
67dd6cb
Compare
67dd6cb to
43444ea
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The advertised AccountActivationCompleted redirect behavior is not implemented or registered.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
f30acd4 to
ecdacc8
Compare
is_enterprise_learner hint
ecdacc8 to
bb714f4
Compare
This adds the ActivationEmailEnterpriseContextEnricher pipeline step for the AccountActivationEmailContextGenerated filter. The new pipeline step injects the `is_enterprise_learner` context key which can be used in custom edx themes to influence the activation email template. For 2U, this is only used to trigger an alternative subject line which lacks a goals CTA (irrelevant to most enterprise learners). ENT-11816
bb714f4 to
48e08c0
Compare
Migrate enterprise functionality (populating
is_enterprise_learnerin Braze email context) from openedx-platform into AccountActivationEmailComposed filter.This injects the
is_enterprise_learnercontext key which can be used in custom edx themes to influence the activation email template. For 2U, this is only used to trigger an alternative subject line which lacks a goals CTA which is irrelevant to most enterprise learners.ENT-11816
These should be merged together:
is_enterprise_learnerhint #2674Testing
Setup
Platform settings:
Temp logging:
Non-enterprise user scenario
is_enterprise=False.Enterprise user scenario
is_enterprise=True.