feat: add pipeline steps for logistration filters - #2551
Conversation
220b22d to
e717b74
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2551 +/- ##
==========================================
+ Coverage 86.97% 87.02% +0.04%
==========================================
Files 261 262 +1
Lines 17143 17228 +85
Branches 1693 1705 +12
==========================================
+ Hits 14910 14992 +82
- Misses 1896 1897 +1
- Partials 337 339 +2
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:
|
34a6dc4 to
4e1987d
Compare
5b99d92 to
0ee8066
Compare
5cf65c3 to
38361f7
Compare
| @@ -1,17 +1,11 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
I don't mind leaving references to the ticket number in docs, but this filename is a bit weird. Maybe logistration-pipeline-integration-test.sh?
There was a problem hiding this comment.
My goal is to polish and document as much official logic as possible in official locations:
- enterprise/devstack_api.py
- enterprise/roles_api.py
- provision-tpa.py
But stuff in scripts/ are one-time use and not intended for long-term support. The other script, provision-integration-test-ENT-11544.sh for a previous ticket does not even work anymore because so much of devstack_api.py has changed, but there's no problem because it's not a supported feature and not "on our books" as a maintenance liability.
The ticket number in the script name reinforces the temporariness and unsupported nature of the script.
38361f7 to
abd13d1
Compare
42619a3 to
c25f1fb
Compare
kramakrushna
left a comment
There was a problem hiding this comment.
Overall this looks like a solid extraction of the enterprise logistration behavior into openedx-filters, and the new tests/docs are helpful. I have three main concerns before merge:
-
LoginFormEnterpriseOverrides assumes running_pipeline -> kwargs -> details always exists; if that shape changes or is missing, this can crash the login flow.
-
PostLoginEnterpriseRedirect relies on re.search(UUID4_REGEX, redirect_url).group(0) after a regex match; this is brittle and could raise if the URL format changes or is encoded differently.
-
RegistrationFormEnterpriseOverrides intentionally changes how skip_registration_optional_checkboxes is resolved compared with the legacy platform logic. That seems reasonable, but it should be called out more explicitly since it changes behavior in an edge case.
1365f66 to
6d47625
Compare
Implement steps for six logistration filters replacing enterprise logic in the user_authn platform module, and register them in ENTERPRISE_FILTERS_CONFIG: - LogistrationViewEnterpriseContextEnricher (for LogistrationViewContextGenerated) - AuthnMFEEnterpriseContextEnricher (for AuthnMFEContextGenerated) - LogistrationViewEnterpriseCookieSetter (for LogistrationViewRenderCompleted) - LoginFormEnterpriseOverrides (for LoginFormGenerated) - RegistrationFormEnterpriseOverrides (for RegistrationFormGenerated) - PostLoginEnterpriseRedirect (for LoginAltRedirectURLRequested) ENT-11568
6d47625 to
f4c6aaf
Compare
There was a problem hiding this comment.
Pull request overview
Adds enterprise-owned openedx-filters pipeline steps to implement (and document/test) enterprise logistration behavior that previously lived in the platform’s user_authn module, moving that logic behind filter hooks and wiring it into the plugin’s settings.
Critical issues to address:
enterprise/settings/test.pydefinesCOURSE_KEY_REGEXvia an incorrect string replacement that produces an invalid regex (will raisere.errorwhen used).enterprise/filters/logistration.pycan raiseAttributeErrorifopenedx.core.djangoapps.user_api.accountsisn’t importable (accounts is set toNonebut later dereferenced).
Changes:
- Introduces 6 authentication/logistration pipeline steps in
enterprise/filters/logistration.pyand registers them inENTERPRISE_FILTERS_CONFIG(merged intoOPEN_EDX_FILTERS_CONFIG). - Adds comprehensive unit tests for the new pipeline steps.
- Updates devstack integration-test provisioning script and docs; enables Sphinx Markdown + Mermaid support for ADRs.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tox.ini |
Ensures pii_check env installs Django 5.2.x. |
enterprise/filters/logistration.py |
Adds new openedx-filters pipeline steps implementing enterprise logistration behavior. |
enterprise/settings/common.py |
Registers the new authentication filter pipelines in ENTERPRISE_FILTERS_CONFIG and merges into OPEN_EDX_FILTERS_CONFIG. |
enterprise/settings/test.py |
Adds COURSE_KEY_REGEX for standalone tests (currently incorrect). |
tests/filters/test_logistration.py |
New unit tests covering the pipeline step behaviors. |
scripts/provision-integration-test-ENT-11568.sh |
Updates provisioning + manual test plan; adds a preflight validation of auth filter mappings. |
requirements/test.txt |
Bumps openedx-filters to 3.9.0. |
requirements/test-master.txt |
Bumps openedx-filters to 3.9.0. |
requirements/edx-platform-constraints.txt |
Bumps openedx-filters to 3.9.0 under platform constraints. |
requirements/doc.in |
Adds sphinxcontrib-mermaid and myst-parser to doc inputs. |
requirements/doc.txt |
Regenerated pins/comments to include MyST + Mermaid deps and openedx-filters==3.9.0. |
requirements/dev.txt |
Regenerated pins/comments to include MyST + Mermaid deps and openedx-filters==3.9.0. |
docs/conf.py |
Enables .md sources + MyST parser and Mermaid extension. |
docs/decisions/0016-logistration-filters.md |
New ADR documenting the filter-based architecture and flows (Mermaid diagram). |
docs/saml_testing.rst |
Updates SAML devstack prerequisites and streamlines provisioning/testing docs. |
enterprise/__init__.py |
Version bump to 8.8.0. |
CHANGELOG.rst |
Adds 8.8.0 entry dated 2026-08-07. |
Implement steps for five logistration filters replacing logic in the
user_authnplatform module, and register them inENTERPRISE_FILTERS_CONFIG:ENT-11568
Integration Testing in 2U Devstack
Setup:
make dev.up.lms+enterprise-catalog+frontend-app-account+frontend-app-authn/etc/hosts:127.0.0.1 edx.devstack.keycloakuser_authn.enable_enterprise_redirect_to_authnwaffle flag and enable it globally.Control Test:
make dev.provision.keycloak./scripts/provision-integration-test-ENT-11568.shIntegration Test:
pip install -e /edx/src/edx-enterprise/pip install -e /edx/src/openedx-filters/Related: