Repository navigation
fix: keep courseware within the course of its URL - #2228
ihor-romaniuk wants to merge 1 commit into
Conversation
A sequence or unit ID of another course pasted into /course/<courseId>/... rendered that course's content inside the shell of <courseId>. Such IDs are now dropped before anything of the other course is requested: a foreign sequence redirects to the course root, a foreign unit to its sequence.
|
Thanks for the pull request, @ihor-romaniuk! 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 #2228 +/- ##
==========================================
+ Coverage 96.15% 96.17% +0.01%
==========================================
Files 368 369 +1
Lines 6011 6034 +23
Branches 1438 1490 +52
==========================================
+ Hits 5780 5803 +23
Misses 222 222
Partials 9 9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
In the Learning MFE, the ID of a subsection or unit from another course could be pasted into a course's courseware URL —
/learning/course/<course A>/<subsection of B>/<unit of B>— and the page showed course B's unit inside course A: A's header, tabs and outline around B's content. The LMS itself still guards the content, so a learner who is not enrolled in B got an "unexpected error" message instead of B's unit, yet B's unit title still appeared in A's page.Such URLs no longer load anything of the other course. A subsection of another course sends the learner to the root of course A, which resumes them where they left off in A; a unit of another course inside one of A's subsections is dropped, and that subsection opens on its current unit. Learners who navigate normally see no change.
Implementation notes
isBlockInCourseincourseware/data/courseKeys.ts):org+course+runcompared case-insensitively,branch@/version@ignored, a CCX accepts its master course's blocks but not another CCX's. A key format it does not recognise counts as belonging, so nothing that loads today can be hidden by it.getForeignBlockRedirectinCoursewareRedirectsrewrites the URL, andCoursewareContainerdisables the sequence-metadata query for such a route, so the other course's metadata never enters the query cache.render_xblockalready authorises the unit by its own course (enrollment check), so this closes a course-context defect, not a disclosure.Screenshots
Testing
Preconditions: two courses with content, A and B. Learner 1 is enrolled in both, learner 2 only in A.
1. Unit of another course in the URL (learner 1)
/course/<course B>— twoblock-v1:…segments./course/<course A>with the copied part./api/courseware/sequence/<subsection of B>.2. The same URL for a learner who is not enrolled in B (learner 2)
3. Unit of another course inside a subsection of A (learner 1)
/course/<course A>/<subsection of A>/<unit of B>.4. Staff preview
/preview/course/<course A>/<subsection of B>/<unit of B>.5. Regression
/course/<course A>/<unit of A>is still completed to the full URL of that unit.Verified locally
eslint and
tscclean; the full jest suite passes — 1303 tests in 125 suites. With the fix reverted, all 4 new integration tests fail; with only the redirect kept and the query left enabled, 3 of them fail. In Chrome, this branch's dev build against a local Verawood (Tutor 22.0.0) LMS behaves as in scenarios 1–3.