From 3334325ff9b8c338176cf59db5636fbf2736d604 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Thu, 22 May 2025 10:58:52 -0400 Subject: [PATCH] fix: Drop _get_legacy_courseware_url and related usage. We were running some tests using this function but it is not actually used in the running application anymore so drop those tests and remove the function in preparation for removing the legacy courseware itself. --- lms/djangoapps/courseware/tests/test_views.py | 76 ------------------- lms/djangoapps/courseware/testutils.py | 27 ------- .../features/course_experience/url_helpers.py | 41 ---------- 3 files changed, 144 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 2a27a3fc41..8cf7737f1b 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -108,7 +108,6 @@ 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_legacy_courseware_url, get_learning_mfe_home_url, make_learning_mfe_courseware_url ) @@ -232,31 +231,6 @@ class TestJumpTo(ModuleStoreTestCase): assert response.status_code == 302 assert response.url == expected_redirect_url - # The new courseware experience does not support this sort of course structure; - # it assumes a simple course->chapter->sequence->unit->component tree. - @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() - chapter = BlockFactory.create(category='chapter', parent_location=course.location) - sequence = BlockFactory.create(category='sequential', parent_location=chapter.location) - vertical = BlockFactory.create(category='vertical', parent_location=sequence.location) - nested_sequence = BlockFactory.create(category='sequential', parent_location=vertical.location) - nested_vertical1 = BlockFactory.create(category='vertical', parent_location=nested_sequence.location) - # put a block into nested_vertical1 for completeness - BlockFactory.create(category='html', parent_location=nested_vertical1.location) - nested_vertical2 = BlockFactory.create(category='vertical', parent_location=nested_sequence.location) - block2 = BlockFactory.create(category='html', parent_location=nested_vertical2.location) - - # internal position of block2 will be 1_2 (2nd item withing 1st item) - activate_block_id = urlencode({'activate_block_id': str(block2.location)}) - expected_redirect_url = ( - f'/courses/{course.id}/courseware/{chapter.url_name}/{sequence.url_name}/1?{activate_block_id}' - ) - jumpto_url = f'/courses/{course.id}/jump_to/{block2.location}' - response = self.client.get(jumpto_url) - self.assertRedirects(response, expected_redirect_url, status_code=302, target_status_code=302) - @ddt.data( (False, ModuleStoreEnum.Type.split), (True, ModuleStoreEnum.Type.split), @@ -273,44 +247,6 @@ class TestJumpTo(ModuleStoreTestCase): response = self.client.get(jumpto_url) assert response.status_code == 404 - @ddt.data( - (ModuleStoreEnum.Type.split, False, '1'), - (ModuleStoreEnum.Type.split, True, '2'), - ) - @ddt.unpack - def test_jump_to_legacy_for_learner_with_staff_only_content(self, store_type, is_staff_user, position): - """ - Test for checking correct position in redirect_url for learner when a course has staff-only units. - - (When the MFE is active, it handles this logic itself with the help of the - courseware blocks/metadata/outline APIs, so we don't test for it here.) - """ - with self.store.default_store(store_type): - course = CourseFactory.create() - request = RequestFactory().get('/') - request.user = UserFactory(is_staff=is_staff_user, username="staff") - request.session = {} - course_key = CourseKey.from_string(str(course.id)) - chapter = BlockFactory.create(category='chapter', parent_location=course.location) - sequence = BlockFactory.create(category='sequential', parent_location=chapter.location) - __ = BlockFactory.create(category='vertical', parent_location=sequence.location) - staff_only_vertical = BlockFactory.create(category='vertical', parent_location=sequence.location, - metadata=dict(visible_to_staff_only=True)) - __ = BlockFactory.create(category='vertical', parent_location=sequence.location) - - usage_key = UsageKey.from_string(str(staff_only_vertical.location)).replace(course_key=course_key) - expected_url = reverse( - 'courseware_position', - kwargs={ - 'course_id': str(course.id), - 'chapter': chapter.url_name, - 'section': sequence.url_name, - 'position': position, - } - ) - expected_url += "?{}".format(urlencode({'activate_block_id': str(staff_only_vertical.location)})) - assert expected_url == _get_legacy_courseware_url(usage_key, request) - class IndexQueryTestCase(ModuleStoreTestCase): """ @@ -474,18 +410,6 @@ class CoursewareIndexTestCase(BaseViewsTestCase): assert response.status_code == expected_response_code return response - def test_get_redirect_url(self): - # test the course location - 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_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_legacy_courseware_url(self.section.location) - def test_index_invalid_position(self): request_url = '/'.join([ '/courses', diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index b0f444f288..8730e29e38 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -12,7 +12,6 @@ import ddt from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from lms.djangoapps.courseware.utils import is_mode_upsellable -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 @@ -165,32 +164,6 @@ class RenderXBlockTestMixin(MasqueradeMixin, metaclass=ABCMeta): self.assertNotContains(response, self.html_block.data, status_code=expected_response_code) return response - @ddt.data( - ('vertical_block', 4), - ('html_block', 4), - ) - @ddt.unpack - @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 - be removed actually exist in the full courseware page. - If this test fails, it's probably because the HTML template for courseware - has changed and COURSEWARE_CHROME_HTML_ELEMENTS needs to be updated. - """ - with self.store.default_store(ModuleStoreEnum.Type.split): - self.block_name_to_be_tested = block_name - self.setup_course(ModuleStoreEnum.Type.split) - self.setup_user(admin=True, enroll=True, login=True) - - with check_mongo_calls(mongo_calls): - 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: - self.assertContains(response, chrome_element) - def test_success_enrolled_staff(self): self.setup_course() self.setup_user(admin=True, enroll=True, login=True) diff --git a/openedx/features/course_experience/url_helpers.py b/openedx/features/course_experience/url_helpers.py index be8c02d2bd..dca139d6f1 100644 --- a/openedx/features/course_experience/url_helpers.py +++ b/openedx/features/course_experience/url_helpers.py @@ -44,47 +44,6 @@ def get_courseware_url( return _get_new_courseware_url(usage_key=usage_key, request=request, is_staff=is_staff) -def _get_legacy_courseware_url( - usage_key: UsageKey, - request: Optional[HttpRequest] = None, - is_staff: bool = None -) -> str: - """ - Return the URL to Legacy (LMS-rendered) courseware content. - - Raises: - * ItemNotFoundError if no data at the usage_key. - * NoPathToItem if location not in any class. - """ - ( - course_key, chapter, section, vertical_unused, - position, final_target_id - ) = path_to_location(modulestore(), usage_key, request) - - # choose the appropriate view (and provide the necessary args) based on the - # args provided by the redirect. - # Rely on index to do all error handling and access control. - if chapter is None: - redirect_url = reverse('courseware', args=(str(course_key), )) - elif section is None: - redirect_url = reverse('courseware_chapter', args=(str(course_key), chapter)) - elif position is None: - redirect_url = reverse( - 'courseware_section', - args=(str(course_key), chapter, section) - ) - else: - # Here we use the navigation_index from the position returned from - # path_to_location - we can only navigate to the topmost vertical at the - # moment - redirect_url = reverse( - 'courseware_position', - args=(str(course_key), chapter, section, navigation_index(position)) - ) - redirect_url += "?{}".format(urlencode({'activate_block_id': str(final_target_id)})) - return redirect_url - - def _get_new_courseware_url( usage_key: UsageKey, request: Optional[HttpRequest] = None,