feat: remove enterprise enrollment imports and adds CourseEnrollmentStarted filter#306
Open
kiram15 wants to merge 6 commits into
Open
feat: remove enterprise enrollment imports and adds CourseEnrollmentStarted filter#306kiram15 wants to merge 6 commits into
kiram15 wants to merge 6 commits into
Conversation
…from enrollments/views.py
There was a problem hiding this comment.
Pull request overview
This PR decouples enterprise-specific enrollment side effects from the LMS enrollment API endpoint by removing direct enterprise_support dependencies in EnrollmentView, and instead relying on an updated edx-enterprise version (presumably containing the new filter/pipeline integration) to handle enterprise post-enrollment processing.
Changes:
- Bump
edx-enterprisefrom8.0.14to8.0.16across constraints and edx requirement sets. - Remove
openedx.features.enterprise_support.apiimports and thelinked_enterprise_customerconditional enterprise enrollment/consent block fromEnrollmentView.post. - Remove the enterprise-mocking test mixin usage and the enterprise enrollment test that depended on HTTP mocking.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| requirements/edx/base.txt | Bumps edx-enterprise pin to align the platform with the new enterprise-side pipeline behavior. |
| requirements/edx/development.txt | Updates dev requirements to the same edx-enterprise version. |
| requirements/edx/doc.txt | Updates doc requirements to the same edx-enterprise version. |
| requirements/edx/testing.txt | Updates testing requirements to the same edx-enterprise version. |
| requirements/constraints.txt | Updates the global constraint for edx-enterprise to 8.0.16. |
| openedx/core/djangoapps/enrollments/views.py | Removes direct enterprise service calls during enrollment POST handling. |
| openedx/core/djangoapps/enrollments/tests/test_views.py | Removes enterprise-specific test scaffolding and the enterprise enrollment integration test. |
Comments suppressed due to low confidence (1)
openedx/core/djangoapps/enrollments/views.py:775
- The POST enrollment API used to read
linked_enterprise_customerand trigger enterprise enrollment/consent side effects when called with API-key permissions. With this block removed, requests that still sendlinked_enterprise_customerwill now be silently ignored, which is a behavior change and can break existing enterprise-worker integrations.
To make the change safer, consider either (a) supporting backwards compatibility by mapping linked_enterprise_customer to the enterprise_uuid path (or otherwise passing it through to the new edx-enterprise post-processor), or (b) explicitly rejecting/deprecating the parameter with a clear 4xx response so clients fail fast instead of succeeding without the intended enterprise linkage.
enrollment_attributes = request.data.get("enrollment_attributes")
force_enrollment = request.data.get("force_enrollment")
# Check if the force enrollment status is None or a Boolean
if force_enrollment is not None and not isinstance(force_enrollment, bool):
return Response(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
openedx/core/djangoapps/enrollments/tests/test_views.py:1247
- Enterprise enrollment behavior previously had a dedicated test. With the enterprise-specific code moved behind CourseEnrollmentViewStarted, there should be replacement coverage to (1) assert the filter is invoked when linked_enterprise_customer is present and (2) verify PreventEnrollment produces the expected HTTP 400 response. Otherwise regressions in this integration point may go unnoticed.
def test_enrollment_attributes_always_written(self):
""" Enrollment attributes should always be written, regardless of whether
the enrollment is being created or updated.
"""
course_key = self.course.id
Comment on lines
+16
to
+21
| from edx_rest_framework_extensions.auth.jwt.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| JwtAuthentication | ||
| ) | ||
| from edx_rest_framework_extensions.auth.session.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| SessionAuthenticationAllowInactiveUser | ||
| ) |
Comment on lines
+775
to
+781
| try: | ||
| user, course_id, explicit_linked_enterprise, has_api_key_permissions = CourseEnrollmentViewStarted.run_filter( | ||
| user=user, | ||
| course_key=course_id, | ||
| linked_enterprise=explicit_linked_enterprise, | ||
| has_api_key_permissions=has_api_key_permissions, | ||
| ) |
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
da62878 to
31ca3d4
Compare
Comment on lines
+16
to
+21
| from edx_rest_framework_extensions.auth.jwt.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| JwtAuthentication | ||
| ) | ||
| from edx_rest_framework_extensions.auth.session.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| SessionAuthenticationAllowInactiveUser | ||
| ) |
| EnterpriseApiServiceClient, | ||
| enterprise_enabled, | ||
| ) | ||
| from openedx_filters.learning.filters import CourseEnrollmentViewStarted |
Comment on lines
1243
to
1245
| def test_enrollment_attributes_always_written(self): | ||
| """ Enrollment attributes should always be written, regardless of whether | ||
| the enrollment is being created or updated. |
pwnage101
requested changes
Jun 9, 2026
Comment on lines
+775
to
+781
| try: | ||
| user, course_id, explicit_linked_enterprise, has_api_key_permissions = CourseEnrollmentViewStarted.run_filter( | ||
| user=user, | ||
| course_key=course_id, | ||
| linked_enterprise=explicit_linked_enterprise, | ||
| has_api_key_permissions=has_api_key_permissions, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR removes those enterprise imports and the conditional block from the enrollment view, then runs the CourseEnrollmentViewStarted filter to handle the enterprise-specific enrollment logic seperately and outside of edx-platform. This is part of an ongoing initiative to leverage openedx-filters to allow enterprise to customize logic and remove it from edx-platform code.
ENT-11570
Supporting information
This is the openedx-filters PR
This is related edx-enterprise PR
And this is the corresponding openedx-platform PR (in draft mode rn)
Testing instructions
Testing instructions in the edx-enterprise PR.
Deadline
"None" if there's no rush, or provide a specific date or event (and reason) if there is one.
Other information
Include anything else that will help reviewers and consumers understand the change.