Skip to content

fix: keep courseware within the course of its URL - #2228

Open
ihor-romaniuk wants to merge 1 commit into
openedx:masterfrom
raccoongang:romaniuk/fix/TEA-290/foreign-course-ids-in-courseware-url-master
Open

ihor-romaniuk wants to merge 1 commit into
openedx:masterfrom
raccoongang:romaniuk/fix/TEA-290/foreign-course-ids-in-courseware-url-master

Conversation

@ihor-romaniuk

@ihor-romaniuk ihor-romaniuk commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

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
  • Course membership is read from the opaque keys (isBlockInCourse in courseware/data/courseKeys.ts): org+course+run compared 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.
  • Two halves, both needed: getForeignBlockRedirect in CoursewareRedirects rewrites the URL, and CoursewareContainer disables the sequence-metadata query for such a route, so the other course's metadata never enters the query cache.
  • Not a server-side change: render_xblock already authorises the unit by its own course (enrollment check), so this closes a course-context defect, not a disclosure.

Screenshots

Before After
learner-enrolled-in-a-only-before learner-enrolled-in-a-only-after
learner-enrolled-in-both-before learner-enrolled-in-both-after

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)

  1. Open any unit of course B and copy the part of the URL after /course/<course B> — two block-v1:… segments.
  2. Open course A's courseware and replace everything after /course/<course A> with the copied part.
  3. Expected: the URL changes to a unit of course A (the one last visited), and the header, outline and content all belong to A. In devtools → Network there is no request to /api/courseware/sequence/<subsection of B>.

2. The same URL for a learner who is not enrolled in B (learner 2)

  1. Repeat scenario 1 with learner 2.
  2. Expected: the same result — a unit of course A, nothing of course B in the navigation.

3. Unit of another course inside a subsection of A (learner 1)

  1. Open /course/<course A>/<subsection of A>/<unit of B>.
  2. Expected: the subsection of A opens on its current unit.

4. Staff preview

  1. As staff, open /preview/course/<course A>/<subsection of B>/<unit of B>.
  2. Expected: the root of course A — the same way any unknown ID in a preview URL is handled — resumed within A.

5. Regression

  1. Course B by its own URL still opens B's unit; Previous / Next within course A works; /course/<course A>/<unit of A> is still completed to the full URL of that unit.

Verified locally

eslint and tsc clean; 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.

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.
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Oct 26, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @ihor-romaniuk!

This repository is currently maintained by @openedx/committers-frontend-app-learning.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To 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:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where 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:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@codecov

codecov Bot commented Oct 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.17%. Comparing base (50faa45) to head (4fedac3).
⚠️ Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants