feat!: Remove Legacy Preview Functionality #36460
Open
+82
−246
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are a few more mentions but these are all the ones that don't need major further followup. BREAKING CHANGE: The learning MFE now supports preview functionality natively and it is no longer necessary to use a different domain on the LMS to render a preview of course content. See openedx/frontend-app-learning#1455 for more details.
feanil
force-pushed
the
feanil/drop_legacy_preview
branch
from
3eed306 to
4f9b60b
Compare
Since we're no longer using a separate domain, that check always returned false. Remove it and update any places/tests where it is used.
With the removal of the preview check this function is also a no-op now so drop calls to it and update the places where it is called to not change other behavior.
The CoursewareIndex view is going to be removed eventually but for now we're focusing on removing the PREVIEW_LMS_BASE setting. With this change, if someone tries to load the legacy courseware URL from the preview domain it will no longer redirect them to the MFE preview. This is not a problem that will occur for users coming from existing studio links because those links have already been updated to go directly to the new urls. The only way this path could execute is if someone goes directly to the old Preview URL that they saved off platform somewhere. eg. If they bookmarked it for some reason. BREAKING CHANGE: Saved links (including bookmarks) to the legacy preview URLs will no longer redirect to the MFE preview URLs.
This test helper was setting the preview mode for tests by changing the hostname that was set while tests were running. This was mostly not being used to test preview but to run a bunch of legacy courseware tests while defaulting to the new learning MFE for the courseware. This commit updates various tests in the `courseware` app to not rely on the fact that we're in preview to test legacy courseware behavior and instead directly patches either the `_redirect_to_learning_mfe` function or uses the `_get_legacy_courseware_url` or both to be able to have the tests continue to test the legacy coursewary. This will hopefully make the tests more accuarte even though hopefully we'll just be removing many of them soon as a part of the legacy courseware cleanup. We're just doing the preview removal separately to reduce the number of things that are changing at once.
With the other recent cleanup, this function is no longer being referenced by anything so we can just drop it.
feanil
force-pushed
the
feanil/drop_legacy_preview
branch
from
4f9b60b to
5ee3ecd
Compare
feanil
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGE: The learning MFE now supports preview functionality natively and it is no longer necessary to use a different domain on the LMS to render a preview of course content. Visiting the preview domain will no longer give you a different view of the course content.
See openedx/frontend-app-learning#1455 for more details.
This PR removes the legacy functionality, a side-effect of this is that saved links (including bookmarks) to the legacy preview URLs will not redirect to the MFE preview URLs as mentions of that domain have been removed.
This PR depends on #36436 and #36430 to merge first.