feat: add EnterpriseEnrollmentViewProcessor pipeline step for CourseEnrollmentStarted filter#2553
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2553 +/- ##
==========================================
+ Coverage 86.65% 86.68% +0.02%
==========================================
Files 257 258 +1
Lines 16954 16995 +41
Branches 1677 1680 +3
==========================================
+ Hits 14692 14732 +40
Misses 1927 1927
- Partials 335 336 +1
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:
|
a35d99d to
ceabb63
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an EnterpriseEnrollmentPostProcessor pipeline step to post enterprise enrollment + consent on course enrollment events, with accompanying unit tests.
Changes:
- Introduces
enterprise.filters.enrollment.EnterpriseEnrollmentPostProcessorpipeline step. - Adds unit tests covering enterprise vs non-enterprise paths and exception logging behavior.
- Adds minimal
tests/filterspackage init.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| enterprise/filters/enrollment.py | New pipeline step that posts enterprise enrollment and consent, logging failures. |
| enterprise/filters/init.py | Initializes enterprise.filters package. |
| tests/filters/test_enrollment.py | New tests validating API client calls and exception logging. |
| tests/filters/init.py | Initializes tests.filters package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b8aef35 to
21f022a
Compare
21f022a to
67d3376
Compare
4f16652 to
44cccd5
Compare
41fc198 to
a5ed2d3
Compare
There was a problem hiding this comment.
So, I'm just going to submit this review but before you implement fixes I'd like to meet with you synchronously because there's a lot to unpack and we should discuss an alternate path.
It looks like the CourseEnrollmentStarted filter you were planning to use (possibly adopted from my draft) is installed at the model layer, not the view layer containing the enterprise logic we are trying to replace.
Keep in mind enterprise enrollment creation was added nearly 10 years ago to the view layer, and meanwhile the model layer enroll() method is called from management commands, other views, entitlements, program enrollments, auto-auth, tests, etc. (in all over 80 call sites), so THERE BE DRAGONS if we change this.
e92d54e to
c93b4e3
Compare
| assert additions[FILTER_A]['pipeline'] == [STEP_X] | ||
|
|
||
|
|
||
| class TestEnterpriseFiltersConfig(unittest.TestCase): |
There was a problem hiding this comment.
I don't mind this test being added, but curious why you decided to add it? Seems unrelated.
There was a problem hiding this comment.
Not directly related to the work, more related to the general filter initiative as an smoke test
b6fd649 to
d85c7e6
Compare
…nrollmentStarted filter
d85c7e6 to
04865fd
Compare
b886f42 to
0c22832
Compare
02f1f3d to
b5ce226
Compare
pwnage101
left a comment
There was a problem hiding this comment.
LGTM, just remove the dead code and merge.
b5ce226 to
5d5017f
Compare
5d5017f to
4679983
Compare
Description
An EnterpriseEnrollmentViewProcessor pipeline step is defined in enterprise/filters/enrollment.py for the new openedx-filter CourseEnrollmentViewStarted (here).
The pipeline step checks whether the enrollment user is linked to an enterprise customer, and if so, calls the enterprise and consent API clients to post the enterprise course enrollment and provide consent.
Jira
ENT-11570
Testing instructions
Requires all three branches installed together (follow instructions here)
Use this branch, openedx-filters branch openedx/openedx-filters#363, and edx-platform branch kiram15/ENT-11570
Needs ENABLE_ENTERPRISE_INTEGRATION=True, an API key, an EnterpriseCustomer, and a learner linked to it via EnterpriseCustomerUser.
In the LMS shell, POST to the enrollment endpoint as the worker, patching the two API clients to assert they fire:
Expected: 200; the LMS log shows EnterpriseEnrollmentViewProcessor running: …; post_enterprise_course_enrollment is called with (username, course_id); provide_consent is called with username, course_id, and enterprise_customer_uuid.
I also personally tested these other scenarios
Related: