feat!: Remove Legacy Preview Functionality #36460

feanil · 2025-03-28T21:44:50Z

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.

ormsbee · 2025-04-10T17:49:58Z

@feanil: Just a sanity check: this will break Tutor, right? Because Tutor will expect these settings to exist? Things like this?

ormsbee

A couple of questions and minor requests.

ormsbee · 2025-04-10T17:55:04Z

lms/djangoapps/courseware/tests/test_access.py

-        assert not bool(access.has_access(self.student, 'load', self.course, course_key=self.course.id))
+
+        # Student should be able to load the course even if they don't have staff access.
+        assert bool(access.has_access(self.student, 'load', self.course, course_key=self.course.id))


Why did this behavior change?

Previously this test was being run with the preview domain where the student would indeed not have access. Now that the test is being run on the default domain, the student does have access again.

ormsbee · 2025-04-10T17:56:12Z

lms/djangoapps/courseware/tests/test_masquerade.py



-@set_preview_mode(True)
+@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)


Please add a comment explaining why this patch exists here.

ormsbee · 2025-04-10T18:04:06Z

lms/djangoapps/courseware/tests/test_access.py

-        with patch('lms.djangoapps.courseware.access.in_preview_mode') as mock_preview:
-            mock_preview.return_value = True
-            for obj in modules:
-                assert not bool(access.has_access(self.student, 'load', obj, course_key=self.course.id))


Is there an equivalent test to make sure that students cannot load the preview/draft state of unreleased content if they know the correct URL?

Updated to add a check that a student will not have access to the unpublished content when this is not published yet.

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.

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.

Ensure that students can't get access to unpublished content.

feanil

@ormsbee I've responded to the comments, please have another look when you have a moment.

feanil · 2025-05-13T15:49:59Z

lms/djangoapps/courseware/tests/test_access.py

-        with patch('lms.djangoapps.courseware.access.in_preview_mode') as mock_preview:
-            mock_preview.return_value = True
-            for obj in modules:
-                assert not bool(access.has_access(self.student, 'load', obj, course_key=self.course.id))


Updated to add a check that a student will not have access to the unpublished content when this is not published yet.

lms/djangoapps/courseware/tests/test_masquerade.py

ormsbee · 2025-05-13T18:13:15Z

lms/djangoapps/courseware/tests/test_access.py

+            print(self.store.get_branch_setting())

+        print(self.store.get_branch_setting())


Leftover debug code?

Definitely, I'll clean this up before squashing.

ormsbee

Aside from the print statements, this looks good to squash and merge.

Also remove some errant debugging print statements.

edx-pipeline-bot · 2025-05-14T15:07:50Z

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

edx-pipeline-bot · 2025-05-14T15:41:44Z

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

edx-pipeline-bot · 2025-05-14T16:15:40Z

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

edx-pipeline-bot · 2025-05-14T16:40:07Z

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

edx-pipeline-bot · 2025-05-14T17:01:22Z

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

edx-pipeline-bot · 2025-05-14T17:24:20Z

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

edx-pipeline-bot · 2025-05-14T17:24:20Z

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

closes overhangio#1231 The preview page has been migrated to the Learning MFE as of openedx/openedx-platform#36460 This is why we dont need to have a separate domain for the preview site anymore nor the configuration setting for the preview page in core.

closes #1231 The preview page has been migrated to the Learning MFE as of openedx/openedx-platform#36460 This is why we dont need to have a separate domain for the preview site anymore nor the configuration setting for the preview page in core.

* feat!: Remove all trivial mentions of PREVIEW_LMS_BASE 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. * feat: Drop the `in_preview_mode` function. 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. * feat: Drop courseware_mfe_is_active function. 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. * feat!: Drop redirect to preview from the legacy courseware index. 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. * test: Drop the set_preview_mode test helper. 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. * test: Drop the `_get_urls_function` With the other recent cleanup, this function is no longer being referenced by anything so we can just drop it. * test: Test student access to unpublihsed content. Ensure that students can't get access to unpublished content.

We haven't done anything specila for lms-xml for over a decade and we dropped lms-preview support last year when we added preview capabality to the new Authoring MFE. #36460

We haven't done anything specila for lms-xml for over a decade and we dropped lms-preview support last year when we added preview capabality to the new Authoring MFE. openedx#36460

feanil changed the base branch from master to feanil/remove_courseware_sock March 28, 2025 21:45

feanil force-pushed the feanil/drop_legacy_preview branch 2 times, most recently from 4f9b60b to 5ee3ecd Compare March 31, 2025 15:38

feanil changed the title ~~feanil/drop legacy preview~~ feat!: Remove Legacy Preview Functionality Mar 31, 2025

feanil marked this pull request as ready for review March 31, 2025 16:41

feanil requested a review from kdmccormick as a code owner March 31, 2025 16:41

feanil force-pushed the feanil/remove_courseware_sock branch from 32bd1ab to 1829eb7 Compare April 7, 2025 14:29

feanil requested review from a team as code owners April 7, 2025 14:29

feanil force-pushed the feanil/drop_legacy_preview branch from 5ee3ecd to 9215044 Compare April 7, 2025 14:31

AhtishamShahid removed the request for review from a team April 8, 2025 06:01

Base automatically changed from feanil/remove_courseware_sock to master April 9, 2025 14:14

feanil force-pushed the feanil/drop_legacy_preview branch from 9215044 to 7a16b64 Compare April 9, 2025 16:54

ormsbee requested changes Apr 10, 2025

View reviewed changes

kdmccormick removed their request for review April 17, 2025 17:06

feanil added 6 commits April 24, 2025 11:43

feat: Drop the in_preview_mode function.

6b70b85

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.

feat: Drop courseware_mfe_is_active function.

f128ee9

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.

test: Drop the _get_urls_function

beb9f23

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 7a16b64 to 6e5db47 Compare April 29, 2025 19:47

test: Test student access to unpublihsed content.

4865cab

Ensure that students can't get access to unpublished content.

feanil force-pushed the feanil/drop_legacy_preview branch from fdae7b8 to 4865cab Compare April 30, 2025 17:44

feanil commented May 13, 2025

View reviewed changes

docs: Update lms/djangoapps/courseware/tests/test_masquerade.py

ab4a81e

feanil requested a review from ormsbee May 13, 2025 15:54

ormsbee reviewed May 13, 2025

View reviewed changes

ormsbee approved these changes May 13, 2025

View reviewed changes

style: Fix various lint issues.

3f80924

Also remove some errant debugging print statements.

feanil merged commit 88c7cd7 into master May 14, 2025
67 of 72 checks passed

feanil deleted the feanil/drop_legacy_preview branch May 14, 2025 12:59

feanil mentioned this pull request May 14, 2025

Update to the edx-platform Preview Functionality overhangio/tutor#1231

Closed

Danyal-Faheem mentioned this pull request May 22, 2025

depr!: remove preview site configurations overhangio/tutor#1238

Merged

Danyal-Faheem mentioned this pull request Jul 7, 2025

depr!: remove preview site configurations overhangio/tutor#1248

Merged

feanil added a commit that referenced this pull request Dec 18, 2025

feat: Drop unsupported service variants.

4e724fd

We haven't done anything specila for lms-xml for over a decade and we dropped lms-preview support last year when we added preview capabality to the new Authoring MFE. #36460

feanil mentioned this pull request Dec 18, 2025

feat: Drop unsupported service variants. #37790

Merged

feanil added a commit that referenced this pull request Dec 18, 2025

feat: Drop unsupported service variants.

0f7a7e5

We haven't done anything specila for lms-xml for over a decade and we dropped lms-preview support last year when we added preview capabality to the new Authoring MFE. #36460



		@set_preview_mode(True)
		@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)

		print(self.store.get_branch_setting())

		print(self.store.get_branch_setting())

feat!: Remove Legacy Preview Functionality #36460

feat!: Remove Legacy Preview Functionality #36460

Uh oh!

Conversation

feanil commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ormsbee commented Apr 10, 2025

Uh oh!

ormsbee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

feanil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormsbee left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

edx-pipeline-bot commented May 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feanil commented Mar 28, 2025 •

edited

Loading