feat: redirect to the embargo blocked-message page on error_code 'embargo' - #2055
feat: redirect to the embargo blocked-message page on error_code 'embargo'#2055asadali145 wants to merge 1 commit into
Conversation
…argo' course_access.errorCode can now be 'embargo' (a country-embargo block, reported by the LMS course_metadata endpoint). It previously fell into getAccessDeniedRedirectUrl's default case: dates/progress redirected to course home, but the outline tab rendered anyway - the wrong behavior for a sanctions block, which has no "preview" concept unlike enrollment_required. Adds a dedicated 'embargo' case that redirects every tab, including outline, to the same legacy embargo "blocked message" page (/embargo/blocked-message/courseware/default/) the legacy courseware view already redirects to - reusing the existing /redirect/* landing-page pattern (RedirectPage + CoursewareRedirectLandingPage) used by dashboard/consent/survey redirects. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the pull request, @asadali145! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2055 +/- ##
==========================================
+ Coverage 93.53% 93.59% +0.05%
==========================================
Files 363 367 +4
Lines 5905 6007 +102
Branches 1367 1383 +16
==========================================
+ Hits 5523 5622 +99
- Misses 367 369 +2
- Partials 15 16 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, align with the stated behavior for embargo blocks, and include targeted automated test coverage for both the access-layer redirect and the landing-page route.
Pull request overview
Adds explicit handling for the new course_access.errorCode: 'embargo' case so embargo-blocked learners are redirected away from all Learning MFE course tabs (including outline) to the legacy LMS embargo “blocked message” page via the existing /redirect/* landing-page mechanism.
Changes:
- Add an
'embargo'case ingetAccessDeniedRedirectUrlto redirect all tabs to/redirect/embargo. - Register
/redirect/embargoin the redirect landing page and map it to/embargo/blocked-message/courseware/default/on the LMS. - Add Jest coverage for the new access redirect behavior and the new landing-page route.
File summaries
| File | Description |
|---|---|
| src/shared/access.js | Adds 'embargo' redirect handling to route all tabs to /redirect/embargo. |
| src/shared/access.test.js | Adds tests verifying embargo redirects (outline and non-outline). |
| src/constants.ts | Registers new ROUTES.EMBARGO and REDIRECT_MODES.EMBARGO_REDIRECT constants. |
| src/courseware/CoursewareRedirectLandingPage.jsx | Adds /redirect/embargo landing-page route that redirects to the LMS embargo blocked-message page. |
| src/courseware/CoursewareRedirectLandingPage.test.jsx | Adds test verifying the embargo landing-page redirect target URL. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…stration GlobalRestrictedCountry (added in openedx#36202/openedx#36398) only blocked account registration and profile-country changes - it had no effect on course access. The only mechanism enforcing course access was RestrictedCourse + CountryAccessRule, which needs a row per course, so there was no way to block a country from every course at once. Wires GlobalRestrictedCountry into embargo.api.check_course_access() so a listed country blocks every course, with or without a RestrictedCourse entry. Per-course CountryAccessRule checks still apply on top where configured, staff still bypass every check, and a per-course disable_access_check override can never bypass a global block (only a per-course one). Also closes a related enforcement gap for the Learning MFE: the course_metadata BFF endpoint (the one endpoint the MFE already reads a course_access hasAccess/errorCode flag from to redirect denied learners) now runs the same embargo check directly via a new check_embargo_access() helper, so an embargoed learner's course_access.errorCode comes back as "embargo". This is deliberately scoped to course_metadata only, not the shared check_course_access() that outline/dates/progress/navigation route through - wiring it there too would hard-403 those endpoints, which broke the Learning MFE (companion frontend fix: openedx/frontend-app-learning#2055 makes the MFE redirect away from every tab, including outline, on errorCode "embargo"). Motivating use case: mitodl/hq#13170 (OFAC embargo requirement). No migration (reuses the existing GlobalRestrictedCountry table), no behavior change for existing deployments (both tables are empty by default everywhere). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
course_access.errorCodecan now come back asembargo(a country-embargo block, reported by the LMScourse_metadataendpoint - companion backend change: openedx/openedx-platform#39079). It previously fell intogetAccessDeniedRedirectUrl'sdefaultcase:dates/progressredirected to course home, but the outline tab rendered anyway - the "show the page anyway" behavior that's correct forenrollment_required(so an unenrolled learner can preview the outline), but wrong for a sanctions block, which has no preview concept.Supersedes #2054, which took a different (data-layer) approach to a related embargo-MFE gap; closed in favor of this PR once the backend PR was scoped down to the
course_metadataendpoint only.What changed
src/shared/access.js- adds a dedicated'embargo'case togetAccessDeniedRedirectUrlthat redirects every tab, including outline, to/redirect/embargo.src/constants.ts/src/courseware/CoursewareRedirectLandingPage.jsx- registers that route using the existing/redirect/*landing-page pattern (RedirectPage+CoursewareRedirectLandingPage, the same mechanismdashboard/consent/surveyredirects already use), pointing at the legacy embargo "blocked message" page (/embargo/blocked-message/courseware/default/) so an embargoed learner sees the same sanctions-specific copy the legacy courseware view already redirects to.Implementation details
The redirect target is hardcoded to the generic default blocked-message path rather than passed dynamically from the backend.
RestrictedCoursesupports a per-course custom embargo message, which this won't respect - a deliberate scope trade-off (avoids new backend-to-frontend plumbing for a feature not currently in use), not an oversight.generatePath()strips the trailing slash from the static pattern before the redirect; Django'sAPPEND_SLASHrestores it with a 301, one harmless extra hop on what's already a full-page navigation.Testing
npx jest src/shared/access.test.js src/courseware/CoursewareRedirectLandingPage.test.jsx- all passing, including new cases covering the embargo redirect for both the outline tab and a non-outline tab, and the landing-page route itself.Manual testing
Requires the companion backend PR (openedx/openedx-platform#39079) running - this PR alone has no effect, since nothing produces
errorCode: 'embargo'without it.Mark results as you go -
[x]pass,[!]problem (add a note).edx-platform./admin/embargo/globalrestrictedcountry/), add a country toGlobalRestrictedCountry:settings.EMBARGOisTruefor your devstack (it defaultsTrueunderlms.envs.tutor.development;Falsein generalenvs/common.py)..../learning/course/<course_id>/home./embargo/blocked-message/courseware/default/, showing "This Course Unavailable In Your Country" - from the outline/home tab specifically (previously rendered the course anyway).datesandprogresstabs.GlobalRestrictedCountryrow and reset the test learner'sprofile.country.Additional Context
Depends on / must merge after openedx/openedx-platform#39079 (the backend change is what actually produces
errorCode: 'embargo'- this PR only changes how the frontend reacts to it).