diff --git a/cms/envs/common.py b/cms/envs/common.py index 787db40bd308..84b0742cba9f 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -367,7 +367,6 @@ # 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, diff --git a/cms/envs/devstack-experimental.yml b/cms/envs/devstack-experimental.yml index c08b19045faa..f5d182a82b6b 100644 --- a/cms/envs/devstack-experimental.yml +++ b/cms/envs/devstack-experimental.yml @@ -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: '' diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 63dff1fbb9c2..d1be075007d1 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -45,7 +45,6 @@ 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' diff --git a/cms/envs/mock.yml b/cms/envs/mock.yml index fb2233501ef4..b8935dd3cc21 100644 --- a/cms/envs/mock.yml +++ b/cms/envs/mock.yml @@ -455,7 +455,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 diff --git a/cms/envs/test.py b/cms/envs/test.py index 500c8d538d39..6a71e94c12cd 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -131,7 +131,6 @@ 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}" diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 436cb3514a54..a1a46137fcca 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -34,7 +34,6 @@ 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): diff --git a/lms/djangoapps/courseware/access_utils.py b/lms/djangoapps/courseware/access_utils.py index 99aa1e605bcc..23ec992f8189 100644 --- a/lms/djangoapps/courseware/access_utils.py +++ b/lms/djangoapps/courseware/access_utils.py @@ -25,7 +25,6 @@ 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. diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index 85241ead4a5b..2b5a33b2ac0f 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -8,9 +8,8 @@ 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 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) diff --git a/lms/djangoapps/courseware/tests/test_access.py b/lms/djangoapps/courseware/tests/test_access.py index e0c5f59a83fb..bb6d42025ea6 100644 --- a/lms/djangoapps/courseware/tests/test_access.py +++ b/lms/djangoapps/courseware/tests/test_access.py @@ -49,8 +49,10 @@ 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 @@ def test_student_has_access(self): 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='

This block should not be published.

', + 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 @@ def test_has_access_with_preview_mode(self): 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 @@ def test__has_access_to_block_beta_user(self): 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 @@ def test__has_access_to_block_in_preview_mode(self, start): ) # 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=[]) diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index 9d211b05ea21..33d100c7a298 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -26,7 +26,7 @@ ) 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 @@ def testMasqueradeCohortAvailable(self, target, expected): 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 @@ def test_staff_debug_with_masquerade(self): 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. """ diff --git a/lms/djangoapps/courseware/tests/test_navigation.py b/lms/djangoapps/courseware/tests/test_navigation.py index 7d6e8cf04365..8c2144ee2a59 100644 --- a/lms/djangoapps/courseware/tests/test_navigation.py +++ b/lms/djangoapps/courseware/tests/test_navigation.py @@ -15,12 +15,11 @@ 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 @@ def setUpTestData(cls): # lint-amnesty, pylint: disable=super-method-not-called 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 ''' diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 6303f07cf351..e3f9aa0d59ab 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -46,7 +46,6 @@ 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.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.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 @@ def test_jump_to_invalid_location(self, preview_mode, store_type): # 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 @@ def test_jump_to_preview_from_sequence(self): 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 @@ def test_jump_to_mfe_from_sequence(self): 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 @@ def test_jump_to_preview_from_block(self): 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 @@ def test_jump_to_mfe_from_block(self): # 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 @@ def test_jump_to_id_invalid_location(self, preview_mode, store_type): ) 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 @@ def test_jump_to_legacy_for_learner_with_staff_only_content(self, store_type, is } ) 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 @@ def _create_global_staff_user(self): 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 @@ def setUp(self): 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 @@ def test_index_success(self): 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 @@ def test_get_redirect_url(self): 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 @@ def test_unicode_handling_in_url(self): # 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 @@ def get_response(self, course): # 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 @@ def student_view(self, context): # pylint: disable=unused-argument @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 @@ def test_student_state(self): 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 @@ def test_should_show_enroll_button(self, course_open_for_self_enrollment, @ddt.ddt -@set_preview_mode(True) class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin): """ Tests CompleteOnView is set up correctly in CoursewareIndex. @@ -2590,7 +2573,8 @@ def setup_course(self, default_store): 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 @@ def test_completion_service_disabled(self): 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 @@ def test_completion_service_enabled(self): @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 @@ def _assert_correct_position(self, response, expected_position): @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 @@ def course_options(self): return options -@set_preview_mode(True) class TestIndexViewCrawlerStudentStateWrites(SharedModuleStoreTestCase): """ Ensure that courseware index requests do not trigger student state writes. @@ -3128,7 +3112,8 @@ def setUp(self): 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 @@ def test_write_by_default(self): 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 @@ def test_writes_with_config(self): # 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 @@ def test_url_generation(self): ) -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. diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index a18074e9ccbf..b0f444f288e8 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -11,9 +11,8 @@ 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 @@ def verify_response(self, expected_response_code=200, url_params=None, is_staff= ('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 @@ def test_courseware_html(self, block_name, mongo_calls): 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: diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index e6070a2e3bd1..f9f083cad42e 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -169,28 +169,12 @@ ) -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): diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index e3bd2b34634f..02f0ef1f7f3f 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -27,7 +27,6 @@ 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 ..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 @@ def _redirect_to_learning_mfe(self): 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 @@ def microfrontend_url(self): 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, diff --git a/lms/envs/devstack-experimental.yml b/lms/envs/devstack-experimental.yml index 43486ca024bc..6811a0e93db8 100644 --- a/lms/envs/devstack-experimental.yml +++ b/lms/envs/devstack-experimental.yml @@ -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: '' diff --git a/lms/envs/minimal.yml b/lms/envs/minimal.yml index 21409fa61aa3..f4f9bd8a8a8a 100644 --- a/lms/envs/minimal.yml +++ b/lms/envs/minimal.yml @@ -11,8 +11,6 @@ --- SECRET_KEY: aseuothsaeotuhaseotisaotenihsaoetih -FEATURES: - PREVIEW_LMS_BASE: "http://localhost" TRACKING_BACKENDS: {} EVENT_TRACKING_BACKENDS: {} JWT_AUTH: {} diff --git a/lms/envs/mock.yml b/lms/envs/mock.yml index 4da53fadb18f..cdcbcf49e5a2 100644 --- a/lms/envs/mock.yml +++ b/lms/envs/mock.yml @@ -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 diff --git a/lms/envs/production.py b/lms/envs/production.py index 4884e0a22c87..b44b1078bf8b 100644 --- a/lms/envs/production.py +++ b/lms/envs/production.py @@ -257,7 +257,6 @@ def get_env_setting(setting): 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 @@ def get_env_setting(setting): 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') diff --git a/lms/envs/test.py b/lms/envs/test.py index b49a71b4711a..4563b571e0bf 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -330,12 +330,8 @@ 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 diff --git a/openedx/features/course_experience/tests/test_url_helpers.py b/openedx/features/course_experience/tests/test_url_helpers.py index dfa84583ab28..b6f134f78002 100644 --- a/openedx/features/course_experience/tests/test_url_helpers.py +++ b/openedx/features/course_experience/tests/test_url_helpers.py @@ -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 .. 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 @@ def test_is_request_from_learning_mfe(self, mfe_url, referrer_url, is_mfe): class GetCoursewareUrlTests(SharedModuleStoreTestCase): """ Test get_courseware_url. - - Mock out `courseware_mfe_is_active`; that is tested elseware. """ @classmethod @@ -121,12 +109,10 @@ def create_test_courses(cls): @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 @@ def create_test_courses(cls): ), ), ( - 'mfe', 'subsection', ( 'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' + @@ -142,7 +127,6 @@ def create_test_courses(cls): ), ), ( - 'mfe', 'unit', ( 'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' + @@ -151,7 +135,6 @@ def create_test_courses(cls): ), ), ( - 'mfe', 'component', ( 'http://learning-mfe/course/course-v1:TestX+UrlHelpers+split' + @@ -159,31 +142,10 @@ def create_test_courses(cls): '/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 @@ def test_get_courseware_url( 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() diff --git a/openedx/features/course_experience/url_helpers.py b/openedx/features/course_experience/url_helpers.py index e1fe4110c613..d80ac511583f 100644 --- a/openedx/features/course_experience/url_helpers.py +++ b/openedx/features/course_experience/url_helpers.py @@ -14,7 +14,6 @@ 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( diff --git a/setup.cfg b/setup.cfg index d83ae7d2765e..df76800f5e17 100644 --- a/setup.cfg +++ b/setup.cfg @@ -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