feat: remove enterprise enrollment imports and adds CourseEnrollmentViewStarted filter#306
Open
kiram15 wants to merge 1 commit into
Open
feat: remove enterprise enrollment imports and adds CourseEnrollmentViewStarted filter#306kiram15 wants to merge 1 commit into
kiram15 wants to merge 1 commit into
Conversation
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
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
da62878 to
31ca3d4
Compare
pwnage101
requested changes
Jun 9, 2026
kiram15
changed the title
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
521a53e to
82533f2
Compare
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
8ad0ccf to
1568606
Compare
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
9084d93 to
cdc2980
Compare
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
deb1647 to
5da32d7
Compare
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
5da32d7 to
1e4f0f1
Compare
Comment on lines
+129
to
+132
| sed '/social-auth-app-django>=5.9.0,<6.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt | ||
| sed '/social-auth-core>=4.9.1,<5.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt |
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
1e4f0f1 to
3f6c535
Compare
Comment on lines
+129
to
+132
| sed '/social-auth-app-django>=5.9.0,<6.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt | ||
| sed '/social-auth-core>=4.9.1,<5.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt |
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
3f6c535 to
ee8fbfa
Compare
Comment on lines
+129
to
+132
| sed '/social-auth-app-django>=5.9.0,<6.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt | ||
| sed '/social-auth-core>=4.9.1,<5.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt |
…iewStarted filter
kiram15
force-pushed
the
kiram15/ENT-11570
branch
from
5530ce6 to
bec0228
Compare
pwnage101
reviewed
Jul 14, 2026
Comment on lines
-16
to
+21
| from edx_rest_framework_extensions.auth.jwt.authentication import ( | ||
| from edx_rest_framework_extensions.auth.jwt.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| JwtAuthentication, | ||
| ) # lint-amnesty, pylint: disable=wrong-import-order | ||
| from edx_rest_framework_extensions.auth.session.authentication import ( | ||
| ) | ||
| from edx_rest_framework_extensions.auth.session.authentication import ( # lint-amnesty, pylint: disable=wrong-import-order | ||
| SessionAuthenticationAllowInactiveUser, | ||
| ) # lint-amnesty, pylint: disable=wrong-import-order | ||
| ) |
Member
There was a problem hiding this comment.
As much as copilot doesn't like how these imports were structured, you should not change them. Just keep your changes scoped to exactly what's needed so that they don't cause issues down the line. If you really want to fix these imports then do so in a separate PR.
pwnage101
reviewed
Jul 14, 2026
| from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase | ||
| from xmodule.modulestore.tests.factories import CourseFactory | ||
|
|
||
| PATH = "openedx.core.djangoapps.enrollments.tests.test_filters.TestCourseEnrollmentViewStartedPipelineStep" |
Member
There was a problem hiding this comment.
This might work, but PATH is a risky and not so helpful name. Pick a more specific helpful name, like maybe TEST_STEP_PATH
pwnage101
reviewed
Jul 14, 2026
Comment on lines
-207
to
-215
| # Check if the user is masquerading as a student and get the masqueraded user object | ||
| if user_is_masquerading and masquerade_object.role == 'student': | ||
| try: | ||
| User = get_user_model() | ||
| # If the masqueraded user does not exist, we will continue with the original user object. | ||
| username = masquerade_object.user_name or 'audit' | ||
| user = User.objects.get(username=username) | ||
| except User.DoesNotExist: | ||
| pass |
Member
There was a problem hiding this comment.
Is this change related to this PR or ticket?
pwnage101
reviewed
Jul 14, 2026
Comment on lines
-95
to
+101
| # Date: 2024-08-06 | ||
| # social-auth-app-django 5.4.2 introduces a new migration that will not play nicely with large installations. This will touch | ||
| # user tables, which are quite large, especially on instances like edx.org. | ||
| # We are pinning this until after all the smaller migrations get handled and then we can migrate this all at once. | ||
| # Issue for unpinning: https://github.com/edx/edx-arch-experiments/issues/760 | ||
| # Date: 2026-07-13 | ||
| # social-auth-app-django>=5.9.0 requires Django>=5.2, but edx-platform is still on Django<5.0. | ||
| # social-auth-core>=4.9.1 requires cryptography>=46.0.7 which conflicts with other deps at older versions. | ||
| # The corresponding lines are stripped from common_constraints.txt via sed in the Makefile. | ||
| # Remove these overrides after the Django 5.2 upgrade. | ||
| social-auth-app-django<=5.4.1 | ||
| social-auth-core<4.9.1 |
Member
There was a problem hiding this comment.
Did you need to do this because it was blocking otherwise? If it's really needed, I'm much rather see these social-auth changes in a separate PR.
pwnage101
reviewed
Jul 14, 2026
Comment on lines
+129
to
+132
| sed '/social-auth-app-django>=5.9.0,<6.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt | ||
| sed '/social-auth-core>=4.9.1,<5.0.0/d' requirements/common_constraints.txt > requirements/common_constraints.tmp | ||
| mv requirements/common_constraints.tmp requirements/common_constraints.txt |
Member
There was a problem hiding this comment.
Same here, I don't want to see these Makefile changes in this PR at all.
pwnage101
requested changes
Jul 14, 2026
Comment on lines
+25
to
+28
|
|
||
| # 2026-07-06: Constrain social-auth-* packages to the latest version compatible with | ||
| # edx-drf-extensions<=10.6.0. (Newer versions require a POST instead of a GET.) | ||
| # Tracking issue: https://github.com/openedx/edx-drf-extensions/issues/561 |
Member
There was a problem hiding this comment.
I think i see what's happening. You ran make upgrade which is more or less the right thing to do. I'd like you to be a criminal and just not do any of these things. Let the next automated run of make upgrade handle all this mess and not pollute your commit.
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.