Skip to content

Conversation

@feanil
Copy link
Contributor

@feanil feanil commented Mar 21, 2025

This is a small increment related to #35803

There are portions of the LMS that were redirecting to the old courseware URL which in turn would redirect to the new MFE url. The changes here bypass the extra hop in many of those cases. This is a pre-requisite to dropping the old legacy page.

@feanil feanil force-pushed the feanil/update_courseware_links branch 3 times, most recently from c0324e8 to e1a3a6b Compare March 24, 2025 18:09
@feanil feanil marked this pull request as ready for review March 24, 2025 18:14
@feanil feanil requested review from a team as code owners March 24, 2025 18:14
@feanil feanil requested review from kdmccormick and ormsbee March 24, 2025 18:14
Copy link
Contributor

@arslanashraf7 arslanashraf7 left a comment

Choose a reason for hiding this comment

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

👍 I was also able to test the redirect updated scenarios in this PR.

@pdpinch
Copy link
Contributor

pdpinch commented Apr 4, 2025

Thanks for the review @arslanashraf7

@feanil I think this is good to go, once you rebase

feanil added 2 commits April 4, 2025 14:01
We want to remove this page and URL endpoint so we're removing all the
references in the code that might point to this page.  It was replaced
by the sequences page in the Learning MFE years ago but the old pages
were never cleaned up. We are replacing the calls with the URL for the
courseware in the learning MFE.

See #35803 for more
details.
The courseware URL is going away but it's just used here to test the
middleware.  That can be test with other urls that are relevant to this
middleware.

Note, I was unable to re-produce the failures so I've put back using the
standard `reverse` logic for fetching the URL in the test.
@feanil feanil force-pushed the feanil/update_courseware_links branch from e1a3a6b to cdf2603 Compare April 4, 2025 18:01
@feanil feanil enabled auto-merge April 4, 2025 18:09
Copy link
Contributor

@arbrandes arbrandes left a comment

Choose a reason for hiding this comment

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

Looks good! 👍🏼

@feanil feanil merged commit 8029185 into master Apr 4, 2025
49 checks passed
@feanil feanil deleted the feanil/update_courseware_links branch April 4, 2025 19:04
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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.

6 participants