Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat!: Remove Legacy Preview Functionality (#36460)

Browse Source 
* 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 https://github.com/openedx/frontend-app-learning/issues/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.
This commit is contained in:
Feanil Patel
2025-05-14 08:59:11 -04:00
committed by GitHub
parent 2d66047f7c
commit 88c7cd7bf3
23 changed files with 114 additions and 246 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
cms/envs/common.py
View File

@@ -378,7 +378,6 @@ FEATURES = {
# Prevent auto auth from creating superusers or modifying existing users
'RESTRICT_AUTOMATIC_AUTH': True,
'PREVIEW_LMS_BASE': "preview.localhost:18000",
'ENABLE_GRADE_DOWNLOADS': True,
'ENABLE_MKTG_SITE': False,
'ENABLE_DISCUSSION_HOME_PANEL': True,

1
cms/envs/devstack-experimental.yml
View File

@@ -324,7 +324,6 @@ FEATURES:
ENABLE_SYSADMIN_DASHBOARD: false
ENABLE_THIRD_PARTY_AUTH: true
ENABLE_VIDEO_UPLOAD_PIPELINE: false
PREVIEW_LMS_BASE: preview.localhost:18000
SHOW_FOOTER_LANGUAGE_SELECTOR: false
SHOW_HEADER_LANGUAGE_SELECTOR: false
FEEDBACK_SUBMISSION_EMAIL: ''

1
cms/envs/devstack.py
View File

@@ -45,7 +45,6 @@ EMAIL_FILE_PATH = '/edx/src/ace_messages/'
LMS_BASE = 'localhost:18000'
LMS_ROOT_URL = f'http://{LMS_BASE}'
FEATURES['PREVIEW_LMS_BASE'] = "preview." + LMS_BASE
FRONTEND_REGISTER_URL = LMS_ROOT_URL + '/register'

1
cms/envs/mock.yml
View File

@@ -454,7 +454,6 @@ FEATURES:
LTI_1P3_ENABLED: true
MILESTONES_APP: true
PREVENT_CONCURRENT_LOGINS: true
PREVIEW_LMS_BASE: preview.courses.localhost
SEGMENT_IO_LMS: true
SEPARATE_VERIFICATION_FROM_PAYMENT: true
SHOW_FOOTER_LANGUAGE_SELECTOR: true

1
cms/envs/test.py
View File

@@ -131,7 +131,6 @@ DATABASES = {
LMS_BASE = "localhost:8000"
LMS_ROOT_URL = f"http://{LMS_BASE}"
FEATURES["PREVIEW_LMS_BASE"] = "preview.localhost"
CMS_BASE = "localhost:8001"
CMS_ROOT_URL = f"http://{CMS_BASE}"

6
lms/djangoapps/courseware/access.py
View File

@@ -34,7 +34,6 @@ from lms.djangoapps.courseware.access_utils import (
check_course_open_for_learner,
check_start_date,
debug,
in_preview_mode
)
from lms.djangoapps.courseware.masquerade import get_masquerade_role, is_masquerading_as_student
from lms.djangoapps.ccx.custom_exception import CCXLocatorValidationException
@@ -158,11 +157,6 @@ def has_access(user, action, obj, course_key=None):
if not user:
user = AnonymousUser()
# Preview mode is only accessible by staff.
if in_preview_mode() and course_key:
if not has_staff_access_to_preview_mode(user, course_key):
return ACCESS_DENIED
# delegate the work to type-specific functions.
# (start with more specific types, then get more general)
if isinstance(obj, CourseBlock):

12
lms/djangoapps/courseware/access_utils.py
View File

@@ -25,7 +25,6 @@ from lms.djangoapps.courseware.access_response import (
from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_student
from openedx.features.course_experience import COURSE_ENABLE_UNENROLLED_ACCESS_FLAG, COURSE_PRE_START_ACCESS_FLAG
from xmodule.course_block import COURSE_VISIBILITY_PUBLIC # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.util.xmodule_django import get_current_request_hostname # lint-amnesty, pylint: disable=wrong-import-order
DEBUG_ACCESS = False
log = getLogger(__name__)
@@ -138,7 +137,7 @@ def check_start_date(user, days_early_for_beta, start, course_key, display_error
if start_dates_disabled and not masquerading_as_student:
return ACCESS_GRANTED
else:
if start is None or in_preview_mode() or get_course_masquerade(user, course_key):
if start is None or get_course_masquerade(user, course_key):
return ACCESS_GRANTED
if now is None:
@@ -158,15 +157,6 @@ def check_start_date(user, days_early_for_beta, start, course_key, display_error
return StartDateError(start, display_error_to_user=display_error_to_user)
def in_preview_mode():
"""
Returns whether the user is in preview mode or not.
"""
hostname = get_current_request_hostname()
preview_lms_base = settings.FEATURES.get("PREVIEW_LMS_BASE", None)
return bool(preview_lms_base and hostname and hostname.split(":")[0] == preview_lms_base.split(":")[0])
def check_course_open_for_learner(user, course):
"""
Check if the course is open for learners based on the start date.

12
lms/djangoapps/courseware/tests/helpers.py
View File

@@ -8,9 +8,8 @@ import re
import json
from collections import OrderedDict
from datetime import timedelta
from unittest.mock import Mock, patch
from unittest.mock import Mock
from django.conf import settings
from django.contrib import messages
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from django.test import TestCase
@@ -20,7 +19,6 @@ from django.utils.timezone import now
from xblock.field_data import DictFieldData
from common.djangoapps.edxmako.shortcuts import render_to_string
from lms.djangoapps.courseware import access_utils
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link
from lms.djangoapps.courseware.masquerade import MasqueradeView
@@ -460,11 +458,3 @@ def get_context_dict_from_string(data):
sorted(json.loads(validated_data['metadata']).items(), key=lambda t: t[0])
)
return validated_data
def set_preview_mode(preview_mode: bool):
"""
A decorator to force the preview mode on or off.
"""
hostname = settings.FEATURES.get('PREVIEW_LMS_BASE') if preview_mode else None
return patch.object(access_utils, 'get_current_request_hostname', new=lambda: hostname)

77
lms/djangoapps/courseware/tests/test_access.py
View File

@@ -49,8 +49,10 @@ from xmodule.course_block import ( # lint-amnesty, pylint: disable=wrong-import
CATALOG_VISIBILITY_CATALOG_AND_ABOUT,
CATALOG_VISIBILITY_NONE
)
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.tests.django_utils import ( # lint-amnesty, pylint: disable=wrong-import-order
ModuleStoreTestCase,
SharedModuleStoreTestCase
@@ -245,35 +247,54 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes
course_key = self.course.id
chapter = BlockFactory.create(category="chapter", parent_location=self.course.location)
overview = CourseOverview.get_from_id(course_key)
subsection = BlockFactory.create(category="sequential", parent_location=chapter.location)
unit = BlockFactory.create(category="vertical", parent_location=subsection.location)
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
html_block = BlockFactory.create(
category="html",
parent_location=unit.location,
display_name="Unpublished Block",
data='<p>This block should not be published.</p>',
publish_item=False,
)
# Enroll student to the course
CourseEnrollmentFactory(user=self.student, course_id=self.course.id)
modules = [
self.course,
overview,
chapter,
overview,
subsection,
unit,
]
with patch('lms.djangoapps.courseware.access.in_preview_mode') as mock_preview:
mock_preview.return_value = False
for obj in modules:
assert bool(access.has_access(self.student, 'load', obj, course_key=self.course.id))
# Student should have access to modules they're enrolled in
for obj in modules:
assert bool(access.has_access(
self.student,
'load',
self.store.get_item(obj.location),
course_key=self.course.id)
)
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))
# If the document is not published yet, it should return an error when we try to fetch
# it from the store. This check confirms that the student would not be able to access it.
with pytest.raises(ItemNotFoundError):
self.store.get_item(html_block.location)
@patch('lms.djangoapps.courseware.access.in_preview_mode', Mock(return_value=True))
def test_has_access_with_preview_mode(self):
def test_has_access_based_on_roles(self):
"""
Tests particular user's can access content via has_access in preview mode.
Tests user access to content based on their role.
"""
assert bool(access.has_access(self.global_staff, 'staff', self.course, course_key=self.course.id))
assert bool(access.has_access(self.course_staff, 'staff', self.course, course_key=self.course.id))
assert bool(access.has_access(self.course_instructor, 'staff', self.course, course_key=self.course.id))
assert not bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id))
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))
# When masquerading is true, user should not be able to access staff content
with patch('lms.djangoapps.courseware.access.is_masquerading_as_student') as mock_masquerade:
@@ -281,10 +302,9 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes
assert not bool(access.has_access(self.global_staff, 'staff', self.course, course_key=self.course.id))
assert not bool(access.has_access(self.student, 'staff', self.course, course_key=self.course.id))
@patch('lms.djangoapps.courseware.access_utils.in_preview_mode', Mock(return_value=True))
def test_has_access_in_preview_mode_with_group(self):
def test_has_access_with_content_groups(self):
"""
Test that a user masquerading as a member of a group sees appropriate content in preview mode.
Test that a user masquerading as a member of a group sees appropriate content.
"""
# Note about UserPartition and UserPartition Group IDs: these must not conflict with IDs used
# by dynamic user partitions.
@@ -423,24 +443,6 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes
assert bool(access._has_access_to_block(self.beta_user, 'load', mock_unit, course_key=self.course.id))
@ddt.data(None, YESTERDAY, TOMORROW)
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
@patch(
'lms.djangoapps.courseware.access_utils.get_current_request_hostname',
Mock(return_value='preview.localhost')
)
def test__has_access_to_block_in_preview_mode(self, start):
"""
Tests that block has access in preview mode.
"""
mock_unit = Mock(location=self.course.location, user_partitions=[])
mock_unit._class_tags = {} # Needed for detached check in _has_access_to_block
mock_unit.visible_to_staff_only = False
mock_unit.start = self.DATES[start]
mock_unit.merged_group_access = {}
self.verify_access(mock_unit, True)
@ddt.data(
(TOMORROW, access_response.StartDateError),
(None, None),
@@ -448,10 +450,11 @@ class AccessTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase, MilestonesTes
) # ddt throws an error if I don't put the None argument there
@ddt.unpack
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
@patch('lms.djangoapps.courseware.access_utils.get_current_request_hostname', Mock(return_value='localhost'))
def test__has_access_to_block_when_not_in_preview_mode(self, start, expected_error_type):
def test__has_access_to_block_with_start_date(self, start, expected_error_type):
"""
Tests that block has no access when start date in future & without preview.
Tests that block access follows start date rules.
Access should be denied when start date is in the future and granted when
start date is in the past or not set.
"""
expected_access = expected_error_type is None
mock_unit = Mock(location=self.course.location, user_partitions=[])

14
lms/djangoapps/courseware/tests/test_masquerade.py
View File

@@ -26,7 +26,7 @@ from lms.djangoapps.courseware.masquerade import (
)
from lms.djangoapps.courseware.tests.helpers import (
LoginEnrollmentTestCase, MasqueradeMixin, masquerade_as_group_member, set_preview_mode,
LoginEnrollmentTestCase, MasqueradeMixin, masquerade_as_group_member
)
from lms.djangoapps.courseware.tests.test_submitting_problems import ProblemSubmissionTestMixin
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
@@ -244,14 +244,20 @@ class TestMasqueradeOptionsNoContentGroups(StaffMasqueradeTestCase):
assert is_target_available == expected
@set_preview_mode(True)
# These tests are testing a capability of the old courseware page. We have to not
# force redirect to the new MFE in order to be able to load the old pages which are
# being tested by this page.
#
# This is a temporary change, until we can remove the old courseware pages
# all together.
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
class TestStaffMasqueradeAsStudent(StaffMasqueradeTestCase):
"""
Check for staff being able to masquerade as student.
"""
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_staff_debug_with_masquerade(self):
def test_staff_debug_with_masquerade(self, mock_redirect):
"""
Tests that staff debug control is not visible when masquerading as a student.
"""
@@ -267,7 +273,7 @@ class TestStaffMasqueradeAsStudent(StaffMasqueradeTestCase):
self.verify_staff_debug_present(True)
@patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_show_answer_for_staff(self):
def test_show_answer_for_staff(self, mock_redirect):
"""
Tests that "Show Answer" is not visible when masquerading as a student.
"""

10
lms/djangoapps/courseware/tests/test_navigation.py
View File

@@ -15,12 +15,11 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
from common.djangoapps.student.tests.factories import GlobalStaffFactory
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase, set_preview_mode
from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase
from openedx.features.course_experience import DISABLE_COURSE_OUTLINE_PAGE_FLAG
from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url
@set_preview_mode(True)
class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
"""
Check that navigation state is saved properly.
@@ -73,6 +72,13 @@ class TestNavigation(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
def setUp(self):
super().setUp()
self.login(self.user.email, 'test')
self.patcher = patch(
'lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
self.mock_redirect = self.patcher.start()
def tearDown(self):
self.patcher.stop()
super().tearDown()
def assertTabActive(self, tabname, response):
''' Check if the progress tab is active in the tab set '''

114
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -46,7 +46,6 @@ import lms.djangoapps.courseware.views.views as views
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
from common.djangoapps.student.models import CourseEnrollment
from common.djangoapps.student.roles import CourseStaffRole
from common.djangoapps.student.tests.factories import (
TEST_PASSWORD,
AdminFactory,
@@ -75,7 +74,7 @@ from lms.djangoapps.courseware.access_utils import check_course_open_for_learner
from lms.djangoapps.courseware.model_data import FieldDataCache, set_score
from lms.djangoapps.courseware.block_render import get_block, handle_xblock_callback
from lms.djangoapps.courseware.tests.factories import StudentModuleFactory
from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text, set_preview_mode
from lms.djangoapps.courseware.tests.helpers import MasqueradeMixin, get_expiration_banner_text
from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin
from lms.djangoapps.courseware.toggles import (
COURSEWARE_MICROFRONTEND_ALWAYS_OPEN_AUXILIARY_SIDEBAR,
@@ -108,7 +107,7 @@ from openedx.features.course_experience import (
)
from openedx.features.course_experience.tests.views.helpers import add_course_mode
from openedx.features.course_experience.url_helpers import (
get_courseware_url,
_get_legacy_courseware_url,
get_learning_mfe_home_url,
make_learning_mfe_courseware_url
)
@@ -152,12 +151,10 @@ class TestJumpTo(ModuleStoreTestCase):
# This is fragile, but unfortunately the problem is that within the LMS we
# can't use the reverse calls from the CMS
with set_preview_mode(False):
response = self.client.get(jumpto_url)
response = self.client.get(jumpto_url)
assert response.status_code == 302
assert response.url == expected_redirect_url
@set_preview_mode(False)
def test_jump_to_preview_from_sequence(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
@@ -171,7 +168,6 @@ class TestJumpTo(ModuleStoreTestCase):
assert response.status_code == 302
assert response.url == expected_redirect_url
@set_preview_mode(False)
def test_jump_to_mfe_from_sequence(self):
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
@@ -184,7 +180,6 @@ class TestJumpTo(ModuleStoreTestCase):
assert response.status_code == 302
assert response.url == expected_redirect_url
@set_preview_mode(False)
def test_jump_to_preview_from_block(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
@@ -211,7 +206,6 @@ class TestJumpTo(ModuleStoreTestCase):
assert response.status_code == 302
assert response.url == expected_redirect_url
@set_preview_mode(False)
def test_jump_to_mfe_from_block(self):
course = CourseFactory.create()
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
@@ -239,7 +233,7 @@ class TestJumpTo(ModuleStoreTestCase):
# The new courseware experience does not support this sort of course structure;
# it assumes a simple course->chapter->sequence->unit->component tree.
@set_preview_mode(True)
@patch.object(views, 'get_courseware_url', new=_get_legacy_courseware_url)
def test_jump_to_legacy_from_nested_block(self):
with self.store.default_store(ModuleStoreEnum.Type.split):
course = CourseFactory.create()
@@ -275,11 +269,9 @@ class TestJumpTo(ModuleStoreTestCase):
) if preview_mode else (
f'/courses/{course.id}/jump_to/NoSuchPlace'
)
with set_preview_mode(False):
response = self.client.get(jumpto_url)
response = self.client.get(jumpto_url)
assert response.status_code == 404
@set_preview_mode(True)
@ddt.data(
(ModuleStoreEnum.Type.split, False, '1'),
(ModuleStoreEnum.Type.split, True, '2'),
@@ -316,17 +308,17 @@ class TestJumpTo(ModuleStoreTestCase):
}
)
expected_url += "?{}".format(urlencode({'activate_block_id': str(staff_only_vertical.location)}))
assert expected_url == get_courseware_url(usage_key, request)
assert expected_url == _get_legacy_courseware_url(usage_key, request)
@set_preview_mode(True)
class IndexQueryTestCase(ModuleStoreTestCase):
"""
Tests for query count.
"""
NUM_PROBLEMS = 20
def test_index_query_counts(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_index_query_counts(self, mock_redirect):
# TODO: decrease query count as part of REVO-28
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
with self.store.default_store(ModuleStoreEnum.Type.split):
@@ -427,27 +419,8 @@ class BaseViewsTestCase(ModuleStoreTestCase, MasqueradeMixin):
self.global_staff = GlobalStaffFactory.create() # pylint: disable=attribute-defined-outside-init
assert self.client.login(username=self.global_staff.username, password=TEST_PASSWORD)
def _get_urls(self): # lint-amnesty, pylint: disable=missing-function-docstring
lms_url = reverse(
'courseware_section',
kwargs={
'course_id': str(self.course_key),
'chapter': str(self.chapter.location.block_id),
'section': str(self.section2.location.block_id),
},
) + '?foo=b$r'
mfe_url = '{}/course/{}/{}?foo=b%24r'.format(
settings.LEARNING_MICROFRONTEND_URL,
self.course_key,
self.section2.location
)
preview_url = "http://" + settings.FEATURES.get('PREVIEW_LMS_BASE') + lms_url
return lms_url, mfe_url, preview_url
@ddt.ddt
@set_preview_mode(True)
class CoursewareIndexTestCase(BaseViewsTestCase):
"""
Tests for the courseware index view, used for instructor previews.
@@ -456,7 +429,8 @@ class CoursewareIndexTestCase(BaseViewsTestCase):
super().setUp()
self._create_global_staff_user() # this view needs staff permission
def test_index_success(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_index_success(self, mock_redirect):
response = self._verify_index_response()
self.assertContains(response, self.problem2.location.replace(branch=None, version_guid=None))
@@ -468,7 +442,6 @@ class CoursewareIndexTestCase(BaseViewsTestCase):
self.assertNotContains(response, self.problem.location.replace(branch=None, version_guid=None))
self.assertContains(response, self.problem2.location.replace(branch=None, version_guid=None))
@set_preview_mode(True)
def test_index_nonexistent_chapter(self):
self._verify_index_response(expected_response_code=404, chapter_name='non-existent')
@@ -505,12 +478,12 @@ class CoursewareIndexTestCase(BaseViewsTestCase):
assert '/courses/{course_key}/courseware?{activate_block_id}'.format(
course_key=str(self.course_key),
activate_block_id=urlencode({'activate_block_id': str(self.course.location)})
) == get_courseware_url(self.course.location)
) == _get_legacy_courseware_url(self.course.location)
# test a section location
assert '/courses/{course_key}/courseware/Chapter_1/Sequential_1/?{activate_block_id}'.format(
course_key=str(self.course_key),
activate_block_id=urlencode({'activate_block_id': str(self.section.location)})
) == get_courseware_url(self.section.location)
) == _get_legacy_courseware_url(self.section.location)
def test_index_invalid_position(self):
request_url = '/'.join([
@@ -542,7 +515,8 @@ class CoursewareIndexTestCase(BaseViewsTestCase):
# TODO: TNL-6387: Remove test
@override_waffle_flag(DISABLE_COURSE_OUTLINE_PAGE_FLAG, active=True)
def test_accordion(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_accordion(self, mock_redirect):
"""
This needs a response_context, which is not included in the render_accordion's main method
returning a render_to_string, so we will render via the courseware URL in order to include
@@ -1148,13 +1122,22 @@ class TestProgressDueDate(BaseDueDateTests):
# TODO: LEARNER-71: Delete entire TestAccordionDueDate class
@set_preview_mode(True)
class TestAccordionDueDate(BaseDueDateTests):
"""
Test that the accordion page displays due dates correctly
"""
__test__ = True
def setUp(self):
super().setUp()
self.patcher = patch(
'lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
self.mock_redirect = self.patcher.start()
def tearDown(self):
self.patcher.stop()
super().tearDown()
def get_response(self, course):
""" Returns the HTML for the accordion """
return self.client.get(
@@ -2363,13 +2346,13 @@ class ViewCheckerBlock(XBlock):
@ddt.ddt
@set_preview_mode(True)
class TestIndexView(ModuleStoreTestCase):
"""
Tests of the courseware.views.index view.
"""
@XBlock.register_temp_plugin(ViewCheckerBlock, 'view_checker')
def test_student_state(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_student_state(self, mock_redirect):
"""
Verify that saved student state is loaded for xblocks rendered in the index view.
"""
@@ -2409,7 +2392,8 @@ class TestIndexView(ModuleStoreTestCase):
self.assertContains(response, "ViewCheckerPassed", count=3)
@XBlock.register_temp_plugin(ActivateIDCheckerBlock, 'id_checker')
def test_activate_block_id(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_activate_block_id(self, mock_redirect):
course = CourseFactory.create()
with self.store.bulk_operations(course.id):
chapter = BlockFactory.create(parent=course, category='chapter')
@@ -2517,7 +2501,6 @@ class TestIndexView(ModuleStoreTestCase):
@ddt.ddt
@set_preview_mode(True)
class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin):
"""
Tests CompleteOnView is set up correctly in CoursewareIndex.
@@ -2590,7 +2573,8 @@ class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin
CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
assert self.client.login(username=self.user.username, password=self.user_password)
def test_completion_service_disabled(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_completion_service_disabled(self, mock_redirect):
self.setup_course(ModuleStoreEnum.Type.split)
@@ -2600,7 +2584,8 @@ class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin
response = self.client.get(self.section_2_url)
self.assertNotContains(response, 'data-mark-completed-on-view-after-delay')
def test_completion_service_enabled(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_completion_service_enabled(self, mock_redirect):
self.override_waffle_switch(True)
@@ -2652,7 +2637,6 @@ class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin
@ddt.ddt
@set_preview_mode(True)
class TestIndexViewWithVerticalPositions(ModuleStoreTestCase):
"""
Test the index view to handle vertical positions. Confirms that first position is loaded
@@ -2704,7 +2688,8 @@ class TestIndexViewWithVerticalPositions(ModuleStoreTestCase):
@ddt.data(("-1", 1), ("0", 1), ("-0", 1), ("2", 2), ("5", 1))
@ddt.unpack
def test_vertical_positions(self, input_position, expected_position):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_vertical_positions(self, input_position, expected_position, mock_redirect):
"""
Tests the following cases:
* Load first position when negative position inputted.
@@ -3097,7 +3082,6 @@ class TestRenderXBlockSelfPaced(TestRenderXBlock): # lint-amnesty, pylint: disa
return options
@set_preview_mode(True)
class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase):
"""
Ensure that courseware index requests do not trigger student state writes.
@@ -3128,7 +3112,8 @@ class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase):
super().setUp()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
def test_write_by_default(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_write_by_default(self, mock_redirect):
"""By default, always write student state, regardless of user agent."""
with patch('lms.djangoapps.courseware.model_data.UserStateCache.set_many') as patched_state_client_set_many:
# Simulate someone using Chrome
@@ -3140,7 +3125,8 @@ class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase):
self._load_courseware('edX-downloader/0.1')
assert patched_state_client_set_many.called
def test_writes_with_config(self):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_writes_with_config(self, mock_redirect):
"""Test state writes (or lack thereof) based on config values."""
CrawlersConfig.objects.create(known_user_agents='edX-downloader,crawler_foo', enabled=True)
with patch('lms.djangoapps.courseware.model_data.UserStateCache.set_many') as patched_state_client_set_many:
@@ -3158,7 +3144,10 @@ class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase):
# Disabling the crawlers config should revert us to default behavior
CrawlersConfig.objects.create(enabled=False)
self.test_write_by_default()
# Disabling the violation because pylint just can't see that we'll get the mock_redirect param passed in via the
# patch.
self.test_write_by_default() # pylint: disable=no-value-for-parameter
def _load_courseware(self, user_agent):
"""Helper to load the actual courseware page."""
@@ -3321,25 +3310,6 @@ class MFEUrlTests(TestCase):
)
class PreviewTests(BaseViewsTestCase):
"""
Make sure we allow the Legacy view for course previews.
"""
def test_learner_redirect(self):
# learners will be redirected by default
lms_url, mfe_url, __ = self._get_urls()
assert self.client.get(lms_url).url == mfe_url
def test_preview_no_redirect(self):
__, __, preview_url = self._get_urls()
with set_preview_mode(True):
# Previews server from PREVIEW_LMS_BASE will not redirect to the mfe
course_staff = UserFactory.create(is_staff=False)
CourseStaffRole(self.course_key).add_users(course_staff)
self.client.login(username=course_staff.username, password=TEST_PASSWORD)
assert self.client.get(preview_url).status_code == 200
class ContentOptimizationTestCase(ModuleStoreTestCase):
"""
Test our ability to make browser optimizations based on XBlock content.

9
lms/djangoapps/courseware/testutils.py
View File

@@ -11,9 +11,8 @@ from urllib.parse import urlencode
import ddt
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from lms.djangoapps.courseware.tests.helpers import set_preview_mode
from lms.djangoapps.courseware.utils import is_mode_upsellable
from openedx.features.course_experience.url_helpers import get_courseware_url
from openedx.features.course_experience.url_helpers import _get_legacy_courseware_url
from common.djangoapps.student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
@@ -171,8 +170,8 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
('html_block', 4),
)
@ddt.unpack
@set_preview_mode(True)
def test_courseware_html(self, block_name, mongo_calls):
@patch('lms.djangoapps.courseware.views.index.CoursewareIndex._redirect_to_learning_mfe', return_value=None)
def test_courseware_html(self, block_name, mongo_calls, mock_redirect):
"""
To verify that the removal of courseware chrome elements is working,
we include this test here to make sure the chrome elements that should
@@ -186,7 +185,7 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta):
self.setup_user(admin=True, enroll=True, login=True)
with check_mongo_calls(mongo_calls):
url = get_courseware_url(self.block_to_be_tested.location)
url = _get_legacy_courseware_url(self.block_to_be_tested.location)
response = self.client.get(url)
expected_elements = self.block_specific_chrome_html_elements + self.COURSEWARE_CHROME_HTML_ELEMENTS
for chrome_element in expected_elements:

20
lms/djangoapps/courseware/toggles.py
View File

@@ -169,28 +169,12 @@ ENABLE_COURSE_DISCOVERY_DEFAULT_LANGUAGE_FILTER = WaffleSwitch(
)
def courseware_mfe_is_active() -> bool:
"""
Should we serve the Learning MFE as the canonical courseware experience?
"""
from lms.djangoapps.courseware.access_utils import in_preview_mode # avoid a circular import
# We only use legacy views for the Studio "preview mode" feature these days, while everyone else gets the MFE
return not in_preview_mode()
def course_exit_page_is_active(course_key):
return (
courseware_mfe_is_active() and
COURSEWARE_MICROFRONTEND_COURSE_EXIT_PAGE.is_enabled(course_key)
)
return COURSEWARE_MICROFRONTEND_COURSE_EXIT_PAGE.is_enabled(course_key)
def courseware_mfe_progress_milestones_are_active(course_key):
return (
courseware_mfe_is_active() and
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES.is_enabled(course_key)
)
return COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES.is_enabled(course_key)
def streak_celebration_is_active(course_key):

8
lms/djangoapps/courseware/views/index.py
View File

@@ -27,7 +27,6 @@ from web_fragments.fragment import Fragment
from xmodule.course_block import COURSE_VISIBILITY_PUBLIC
from xmodule.modulestore.django import modulestore
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW
from xmodule.util.xmodule_django import get_current_request_hostname
from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
from common.djangoapps.student.models import CourseEnrollment
@@ -63,7 +62,7 @@ from ..masquerade import check_content_start_date_for_masquerade_user, setup_mas
from ..model_data import FieldDataCache
from ..block_render import get_block_for_descriptor, toc_for_course
from ..permissions import MASQUERADE_AS_STUDENT
from ..toggles import ENABLE_OPTIMIZELY_IN_COURSEWARE, courseware_mfe_is_active
from ..toggles import ENABLE_OPTIMIZELY_IN_COURSEWARE
from .views import CourseTabView
log = logging.getLogger("edx.courseware.views.index")
@@ -171,8 +170,7 @@ class CoursewareIndex(View):
Can the user access this sequence in the courseware MFE? If so, redirect to MFE.
"""
# If the MFE is active, prefer that
if courseware_mfe_is_active():
raise Redirect(self.microfrontend_url)
raise Redirect(self.microfrontend_url)
@property
def microfrontend_url(self):
@@ -188,7 +186,7 @@ class CoursewareIndex(View):
unit_key = None
except InvalidKeyError:
unit_key = None
is_preview = settings.FEATURES.get('PREVIEW_LMS_BASE') == get_current_request_hostname()
is_preview = False
url = make_learning_mfe_courseware_url(
self.course_key,
self.section.location if self.section else None,

1
lms/envs/devstack-experimental.yml
View File

@@ -355,7 +355,6 @@ FEATURES:
ENABLE_SYSADMIN_DASHBOARD: false
ENABLE_THIRD_PARTY_AUTH: true
ENABLE_VIDEO_UPLOAD_PIPELINE: false
PREVIEW_LMS_BASE: preview.localhost:18000
SHOW_FOOTER_LANGUAGE_SELECTOR: false
SHOW_HEADER_LANGUAGE_SELECTOR: false
FEEDBACK_SUBMISSION_EMAIL: ''

2
lms/envs/minimal.yml
View File

@@ -11,8 +11,6 @@
---
SECRET_KEY: aseuothsaeotuhaseotisaotenihsaoetih
FEATURES:
PREVIEW_LMS_BASE: "http://localhost"
TRACKING_BACKENDS: {}
EVENT_TRACKING_BACKENDS: {}
JWT_AUTH: {}

1
lms/envs/mock.yml
View File

@@ -619,7 +619,6 @@ FEATURES:
NOTICES_SNOOZE_COUNT_LIMIT: 5
NOTICES_SNOOZE_HOURS: 5
PREVENT_CONCURRENT_LOGINS: true
PREVIEW_LMS_BASE: preview-deploy_host
SEND_LEARNING_CERTIFICATE_LIFECYCLE_EVENTS_TO_BUS: true
SEPARATE_VERIFICATION_FROM_PAYMENT: true
SHOW_FOOTER_LANGUAGE_SELECTOR: true

9
lms/envs/production.py
View File

@@ -257,7 +257,6 @@ for feature, value in _YAML_TOKENS.get('FEATURES', {}).items():
ALLOWED_HOSTS = [
"*",
_YAML_TOKENS.get('LMS_BASE'),
FEATURES['PREVIEW_LMS_BASE'],
]
# Cache used for location mapping -- called many times with the same key/value
@@ -326,14 +325,6 @@ if FEATURES['ENABLE_CORS_HEADERS'] or FEATURES.get('ENABLE_CROSS_DOMAIN_CSRF_COO
CORS_ALLOW_INSECURE = _YAML_TOKENS.get('CORS_ALLOW_INSECURE', False)
CROSS_DOMAIN_CSRF_COOKIE_DOMAIN = _YAML_TOKENS.get('CROSS_DOMAIN_CSRF_COOKIE_DOMAIN')
# PREVIEW DOMAIN must be present in HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS for the preview to show draft changes
if 'PREVIEW_LMS_BASE' in FEATURES and FEATURES['PREVIEW_LMS_BASE'] != '':
PREVIEW_DOMAIN = FEATURES['PREVIEW_LMS_BASE'].split(':')[0]
# update dictionary with preview domain regex
HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS.update({
PREVIEW_DOMAIN: 'draft-preferred'
})
############### Mixed Related(Secure/Not-Secure) Items ##########
LMS_SEGMENT_KEY = _YAML_TOKENS.get('SEGMENT_KEY')

6
lms/envs/test.py
View File

@@ -330,12 +330,8 @@ YOUTUBE_PORT = 8031
LTI_PORT = 8765
VIDEO_SOURCE_PORT = 8777
FEATURES['PREVIEW_LMS_BASE'] = "preview.localhost"
############### Module Store Items ##########
PREVIEW_DOMAIN = FEATURES['PREVIEW_LMS_BASE'].split(':')[0]
HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS = {
PREVIEW_DOMAIN: 'draft-preferred'
}
HOSTNAME_MODULESTORE_DEFAULT_MAPPINGS = {}
################### Make tests faster

42
openedx/features/course_experience/tests/test_url_helpers.py
View File

@@ -1,8 +1,6 @@
"""
Test some of the functions in url_helpers
"""
from unittest import mock
import ddt
from django.test import TestCase
from django.test.client import RequestFactory
@@ -14,14 +12,6 @@ from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
from .. import url_helpers
def _patch_courseware_mfe_is_active(ret_val):
return mock.patch.object(
url_helpers,
'courseware_mfe_is_active',
return_value=ret_val,
)
@ddt.ddt
class IsLearningMfeTests(TestCase):
"""
@@ -53,8 +43,6 @@ class IsLearningMfeTests(TestCase):
class GetCoursewareUrlTests(SharedModuleStoreTestCase):
"""
Test get_courseware_url.
Mock out `courseware_mfe_is_active`; that is tested elseware.
"""
@classmethod
@@ -121,12 +109,10 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
@ddt.data(
(
'mfe',
'course_run',
'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split'
),
(
'mfe',
'section',
(
'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' +
@@ -134,7 +120,6 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
),
),
(
'mfe',
'subsection',
(
'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' +
@@ -142,7 +127,6 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
),
),
(
'mfe',
'unit',
(
'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' +
@@ -151,7 +135,6 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
),
),
(
'mfe',
'component',
(
'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' +
@@ -159,31 +142,10 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
'/block-v1:TestX+UrlHelpers+split+type@vertical+block@Generated_Unit'
),
),
(
'legacy',
'course_run',
'/courses/course-v1:TestX+UrlHelpers+split/courseware',
),
(
'legacy',
'subsection',
'/courses/course-v1:TestX+UrlHelpers+split/courseware/Generated_Section/Generated_Subsection/',
),
(
'legacy',
'unit',
'/courses/course-v1:TestX+UrlHelpers+split/courseware/Generated_Section/Generated_Subsection/1',
),
(
'legacy',
'component',
'/courses/course-v1:TestX+UrlHelpers+split/courseware/Generated_Section/Generated_Subsection/1',
)
)
@ddt.unpack
def test_get_courseware_url(
self,
active_experience,
structure_level,
expected_path,
):
@@ -196,9 +158,7 @@ class GetCoursewareUrlTests(SharedModuleStoreTestCase):
check that the expected path (URL without querystring) is returned by `get_courseware_url`.
"""
block = self.items[structure_level]
with _patch_courseware_mfe_is_active(active_experience == 'mfe') as mock_mfe_is_active:
url = url_helpers.get_courseware_url(block.location)
url = url_helpers.get_courseware_url(block.location)
path = url.split('?')[0]
assert path == expected_path
course_run = self.items['course_run']
mock_mfe_is_active.assert_called_once()

7
openedx/features/course_experience/url_helpers.py
View File

@@ -14,7 +14,6 @@ from django.urls import reverse
from opaque_keys.edx.keys import CourseKey, UsageKey
from six.moves.urllib.parse import urlencode, urlparse
from lms.djangoapps.courseware.toggles import courseware_mfe_is_active
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.search import navigation_index, path_to_location # lint-amnesty, pylint: disable=wrong-import-order
@@ -42,11 +41,7 @@ def get_courseware_url(
* ItemNotFoundError if no data at the `usage_key`.
* NoPathToItem if we cannot build a path to the `usage_key`.
"""
if courseware_mfe_is_active():
get_url_fn = _get_new_courseware_url
else:
get_url_fn = _get_legacy_courseware_url
return get_url_fn(usage_key=usage_key, request=request, is_staff=is_staff)
return _get_new_courseware_url(usage_key=usage_key, request=request, is_staff=is_staff)
def _get_legacy_courseware_url(

5
setup.cfg
View File

@@ -98,11 +98,6 @@ ignore_imports =
# -> openedx.core.djangoapps.course_groups.cohorts
# -> lms.djangoapps.courseware.courses
openedx.core.djangoapps.course_groups.cohorts -> lms.djangoapps.courseware.courses
# cms.djangoapps.models.settings.course_metadata
# -> openedx.features.course_experience
# -> openedx.features.course_experience.url_helpers
# -> lms.djangoapps.courseware.toggles
openedx.features.course_experience.url_helpers -> lms.djangoapps.courseware.toggles
# cms.djangoapps.contentstore.[various]
# -> openedx.features.content_type_gating.partitions
# -> lms.djangoapps.commerce.utils