Skip to content

feat: replace enterprise support enrollment imports with filter call - #462

Draft
brobro10000 wants to merge 1 commit into
release-ulmofrom
brobro10000/ENT-11574-enrollment
Draft

feat: replace enterprise support enrollment imports with filter call#462
brobro10000 wants to merge 1 commit into
release-ulmofrom
brobro10000/ENT-11574-enrollment

Conversation

@brobro10000

Copy link
Copy Markdown

ENT-11574

Sibling of the openedx/openedx-platform PR of the same name, carrying the same call-site
change to this deployable fork so it can go through stage/prod ahead of the upstream merge,
per the enterprise plugin ticket runbook. Devstack only boots this repo (not
openedx/openedx-platform), so this is also the branch used for local integration testing.

Companion PR to the already-open support-contact-tag PR set (openedx-filters#390,
edx-enterprise#2688, openedx-platform#39076, edx-platform#455) — this one covers the
other half of ENT-11574's acceptance criteria: enterprise enrollment data for the support
enrollment view.

Removes the direct openedx.features.enterprise_support imports and the
_enterprise_course_enrollments_by_course_id helper from EnrollmentSupportListView,
replacing them with a call to the new SupportEnrollmentDataRequested openedx-filter. No
settings changes in this PR: OPEN_EDX_FILTERS_CONFIG registration for the pipeline step
lives entirely in edx-enterprise's own plugin_settings() (enterprise/settings/common.py),
per the ENT-11830 ownership handoff — already merged in this repo (#281) well before this
branch existed.

lms/djangoapps/support/views/contact_us.py is untouched — this PR is scoped to the
enrollment filter only.

Related PRs

Merge order (per the enterprise plugin ticket runbook)

Merge this after local devstack testing and before the openedx/openedx-platform PR.
Auto-deploys to stage on merge — test in stage, then deploy to prod and confirm working, before
the openedx-platform PR is rebased and merged.

CI note

CI here is expected to be red on Django-app-boot-dependent jobs until openedx-filters and
edx-enterprise are released and this repo's requirements pin is bumped — CI installs the
published PyPI versions, which don't yet contain SupportEnrollmentDataRequested/the new
pipeline step. Not a bug in this diff — it self-resolves per the runbook's release ordering.

Testing

Same test change as the openedx-platform PR: SupportEnrollmentDataRequested.run_filter is
mocked at the call site in lms/djangoapps/support/tests/test_views.py; the pipeline-step
behavior itself is covered by edx-enterprise's own test suite.

Local devstack integration testing (with the openedx-filters, edx-enterprise, and this branch
checked out together) is required before merging.

Sibling of the openedx/openedx-platform PR of the same name, carrying the
same call-site change to this deployable fork so it can go through
stage/prod ahead of the upstream merge, per the enterprise plugin ticket
runbook. Removes the direct openedx.features.enterprise_support imports and
the _enterprise_course_enrollments_by_course_id helper from
EnrollmentSupportListView, replacing them with a call to the new
SupportEnrollmentDataRequested openedx-filter. Per pwnage101's review
feedback on the sibling SupportContactContextRequested filter, run_filter
now returns a tuple of all its inputs (enrollment_data, user) rather than a
bare dict, matching the convention used by every other filter. No settings
changes here: OPEN_EDX_FILTERS_CONFIG registration for the pipeline step
lives in edx-enterprise's own plugin_settings() (enterprise/settings/common.py),
per the ENT-11830 ownership handoff.

ENT-11574
@brobro10000
brobro10000 force-pushed the brobro10000/ENT-11574-enrollment branch from ad89114 to 2ddc176 Compare September 9, 2026 16:38
enterprise_course_enrollments = enterprise_enrollments_by_course_id.get(enrollment['course_id'], [])
enrollment['enterprise_course_enrollments'] = enterprise_course_enrollments
enterprise_enrollments_by_course_id, _ = SupportEnrollmentDataRequested.run_filter(
enrollment_data={}, user=user

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Hm, this is a big red flag that we're passing an empty dict to the filter. You should be passing something useful, like enrollments. Think of it like this: what would a non-enterprise implementer of a pipeline step for this filter need if it needed to augment the enrollments data, not just add a hard-coded "enterprise_course_enrollment" key?

Comment on lines +98 to +101
for enrollment in enrollments:
enrollment['enterprise_course_enrollments'] = enterprise_enrollments_by_course_id.get(
enrollment['course_id'], []
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ delete this whole for-loop. You only needed this because you didn't rely on the filter to augment the enrollments values. Change the fundamental design of the filter to make in-place changes to enrollments, then remove this loop. Another litmus test for whether you've successfully de-enterprised the platform is whether the resulting file after your changes doesn't contain the word "enterprise" at all.

Suggested change
for enrollment in enrollments:
enrollment['enterprise_course_enrollments'] = enterprise_enrollments_by_course_id.get(
enrollment['course_id'], []
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants