From f491b97b2226a31c543554da8f3480092c4c1d68 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Wed, 23 Jul 2025 10:13:07 -0400 Subject: [PATCH] fix: Update naming for courseware section/subsection. These used to be named chapter and section before but we want to update the courseware index view to use the new names if it's gonna stick around. --- .../courseware/tests/test_masquerade.py | 6 +-- lms/djangoapps/courseware/views/index.py | 24 +++++----- lms/djangoapps/edxnotes/helpers.py | 38 ++++++++-------- lms/djangoapps/edxnotes/tests.py | 44 +++++++++---------- lms/templates/courseware/progress.html | 2 +- lms/urls.py | 18 ++++---- .../test_crowdsource_hinter.py | 6 +-- 7 files changed, 69 insertions(+), 69 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py index ed5947ffc6..0f0eca4dc2 100644 --- a/lms/djangoapps/courseware/tests/test_masquerade.py +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -96,11 +96,11 @@ class MasqueradeTestCase(SharedModuleStoreTestCase, LoginEnrollmentTestCase, Mas Returns the server response for the courseware page. """ url = reverse( - 'courseware_section', + 'courseware_subsection', kwargs={ 'course_id': str(self.course.id), - 'chapter': self.chapter.location.block_id, - 'section': self.sequential.location.block_id, + 'section': self.chapter.location.block_id, + 'subsection': self.sequential.location.block_id, } ) return self.client.get(url) diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 6fa0850632..1034674b7b 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -45,7 +45,7 @@ class CoursewareIndex(View): @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True)) @method_decorator(ensure_valid_course_key) @method_decorator(data_sharing_consent_required) - def get(self, request, course_id, chapter=None, section=None, position=None): + def get(self, request, course_id, section=None, subsection=None, position=None): """ Instead of loading the legacy courseware sequences pages, load the equivalent URL in the learning MFE. This view does not do any auth checks since they are done by @@ -54,8 +54,8 @@ class CoursewareIndex(View): Arguments: request: HTTP request course_id (unicode): course id - chapter (unicode): chapter url_name section (unicode): section url_name + subsection (unicode): subsection url_name position (unicode): position in block, eg of block """ @@ -65,7 +65,7 @@ class CoursewareIndex(View): if not (request.user.is_authenticated or self.enable_unenrolled_access): return redirect_to_login(request.get_full_path()) - # Course load to resolve chapters/sections + # Course load to resolve sections/subsections with modulestore().bulk_operations(self.course_key): course = get_course_with_access( request.user, @@ -76,15 +76,15 @@ class CoursewareIndex(View): check_if_authenticated=True, ) - # Get the chapter, section and unit blocks so that we can redirect to the right content + # Get the section, subsection and unit blocks so that we can redirect to the right content # location in the MFE - section_location = None - if chapter and section: - chapter_block = course.get_child_by(lambda m: m.location.block_id == chapter) - if chapter_block: - section_block = chapter_block.get_child_by(lambda m: m.location.block_id == section) - if section_block: - section_location = section_block.location + subsection_location = None + if section and subsection: + section_block = course.get_child_by(lambda m: m.location.block_id == section) + if section_block: + subsection_block = section_block.get_child_by(lambda m: m.location.block_id == subsection) + if subsection_block: + subsection_location = subsection_block.location try: unit_key = UsageKey.from_string(request.GET.get('activate_block_id', '')) @@ -107,7 +107,7 @@ class CoursewareIndex(View): self.request.user = self.effective_user mfe_url = make_learning_mfe_courseware_url( self.course_key, - section_location, + subsection_location, unit_key, params=request.GET, preview=False diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index f81947ce1e..5e68ae4b20 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -405,35 +405,35 @@ def get_course_position(course_block): """ Return the user's current place in the course. - If this is the user's first time, leads to COURSE/CHAPTER/SECTION. - If this isn't the users's first time, leads to COURSE/CHAPTER. + If this is the user's first time, leads to COURSE/SECTION/SUBSECTION. + If this isn't the users's first time, leads to COURSE/SECTION. - If there is no current position in the course or chapter, then selects + If there is no current position in the course or subsection, then selects the first child. """ urlargs = {'course_id': str(course_block.id)} - chapter = get_current_child(course_block, min_depth=1) - if chapter is None: - log.debug("No chapter found when loading current position in course") - return None - - urlargs['chapter'] = chapter.url_name - if course_block.position is not None: - return { - 'display_name': Text(chapter.display_name_with_default), - 'url': reverse('courseware_chapter', kwargs=urlargs), - } - - # Relying on default of returning first child - section = get_current_child(chapter, min_depth=1) + section = get_current_child(course_block, min_depth=1) if section is None: log.debug("No section found when loading current position in course") return None urlargs['section'] = section.url_name + if course_block.position is not None: + return { + 'display_name': Text(section.display_name_with_default), + 'url': reverse('courseware_section', kwargs=urlargs), + } + + # Relying on default of returning first child + subsection = get_current_child(section, min_depth=1) + if subsection is None: + log.debug("No subsection found when loading current position in course") + return None + + urlargs['subsection'] = subsection.url_name return { - 'display_name': Text(section.display_name_with_default), - 'url': reverse('courseware_section', kwargs=urlargs) + 'display_name': Text(subsection.display_name_with_default), + 'url': reverse('courseware_subsection', kwargs=urlargs) } diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 688cd54386..acc6140d1a 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -244,8 +244,8 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): """ return reverse('courseware_position', kwargs={ 'course_id': course.id, - 'chapter': chapter.url_name, - 'section': section.url_name, + 'section': chapter.url_name, + 'subsection': section.url_name, 'position': position, }) @@ -813,34 +813,34 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): timeout=(settings.EDXNOTES_CONNECT_TIMEOUT, settings.EDXNOTES_READ_TIMEOUT) ) - def test_get_course_position_no_chapter(self): + def test_get_course_position_no_section(self): """ - Returns `None` if no chapter found. + Returns `None` if no section found. """ mock_course_block = MagicMock() mock_course_block.position = 3 mock_course_block.get_children.return_value = [] assert helpers.get_course_position(mock_course_block) is None - def test_get_course_position_to_chapter(self): + def test_get_course_position_to_section(self): """ - Returns a position that leads to COURSE/CHAPTER if this isn't the users's + Returns a position that leads to COURSE/SECTION if this isn't the users's first time. """ mock_course_block = MagicMock(id=self.course.id, position=3) - mock_chapter = MagicMock() - mock_chapter.url_name = 'chapter_url_name' - mock_chapter.display_name_with_default = 'Test Chapter Display Name' + mock_section = MagicMock() + mock_section.url_name = 'section_url_name' + mock_section.display_name_with_default = 'Test Chapter Display Name' - mock_course_block.get_children.return_value = [mock_chapter] + mock_course_block.get_children.return_value = [mock_section] assert helpers.get_course_position(mock_course_block) == { 'display_name': 'Test Chapter Display Name', - 'url': f'/courses/{self.course.id}/courseware/chapter_url_name/', + 'url': f'/courses/{self.course.id}/courseware/section_url_name/', } - def test_get_course_position_no_section(self): + def test_get_course_position_no_subsection(self): """ Returns `None` if no section found. """ @@ -848,27 +848,27 @@ class EdxNotesHelpersTest(ModuleStoreTestCase): mock_course_block.get_children.return_value = [MagicMock()] assert helpers.get_course_position(mock_course_block) is None - def test_get_course_position_to_section(self): + def test_get_course_position_to_subsection(self): """ - Returns a position that leads to COURSE/CHAPTER/SECTION if this is the + Returns a position that leads to COURSE/SECTION/SUBSECTION if this is the user's first time. """ mock_course_block = MagicMock(id=self.course.id, position=None) - mock_chapter = MagicMock() - mock_chapter.url_name = 'chapter_url_name' - mock_course_block.get_children.return_value = [mock_chapter] - mock_section = MagicMock() mock_section.url_name = 'section_url_name' - mock_section.display_name_with_default = 'Test Section Display Name' + mock_course_block.get_children.return_value = [mock_section] - mock_chapter.get_children.return_value = [mock_section] - mock_section.get_children.return_value = [MagicMock()] + mock_subsection = MagicMock() + mock_subsection.url_name = 'subsection_url_name' + mock_subsection.display_name_with_default = 'Test Section Display Name' + + mock_section.get_children.return_value = [mock_subsection] + mock_subsection.get_children.return_value = [MagicMock()] assert helpers.get_course_position(mock_course_block) == { 'display_name': 'Test Section Display Name', - 'url': f'/courses/{self.course.id}/courseware/chapter_url_name/section_url_name/', + 'url': f'/courses/{self.course.id}/courseware/section_url_name/subsection_url_name/', } def test_get_index(self): diff --git a/lms/templates/courseware/progress.html b/lms/templates/courseware/progress.html index 26f1bb59cb..711fad8954 100644 --- a/lms/templates/courseware/progress.html +++ b/lms/templates/courseware/progress.html @@ -187,7 +187,7 @@ username = get_enterprise_learner_generic_name(request) or student.username %endif

%else: - + ${ section.display_name} %if (total > 0 or earned > 0) and section.show_grades(staff_access): diff --git a/lms/urls.py b/lms/urls.py index 6cd51e94cf..092dcb6be9 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -482,21 +482,21 @@ urlpatterns += [ name='courseware', ), re_path( - r'^courses/{}/courseware/(?P[^/]*)/$'.format( - settings.COURSE_ID_PATTERN, - ), - CoursewareIndex.as_view(), - name='courseware_chapter', - ), - re_path( - r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/$'.format( + r'^courses/{}/courseware/(?P
[^/]*)/$'.format( settings.COURSE_ID_PATTERN, ), CoursewareIndex.as_view(), name='courseware_section', ), re_path( - r'^courses/{}/courseware/(?P[^/]*)/(?P
[^/]*)/(?P[^/]*)/?$'.format( + r'^courses/{}/courseware/(?P
[^/]*)/(?P[^/]*)/$'.format( + settings.COURSE_ID_PATTERN, + ), + CoursewareIndex.as_view(), + name='courseware_subsection', + ), + re_path( + r'^courses/{}/courseware/(?P
[^/]*)/(?P[^/]*)/(?P[^/]*)/?$'.format( settings.COURSE_ID_PATTERN, ), CoursewareIndex.as_view(), diff --git a/openedx/tests/xblock_integration/test_crowdsource_hinter.py b/openedx/tests/xblock_integration/test_crowdsource_hinter.py index 61f1097481..ecd3c09dd6 100644 --- a/openedx/tests/xblock_integration/test_crowdsource_hinter.py +++ b/openedx/tests/xblock_integration/test_crowdsource_hinter.py @@ -55,11 +55,11 @@ class TestCrowdsourceHinter(SharedModuleStoreTestCase, LoginEnrollmentTestCase): ) cls.course_url = reverse( - 'courseware_section', + 'courseware_subsection', kwargs={ 'course_id': str(cls.course.id), - 'chapter': 'Overview', - 'section': 'Welcome', + 'section': 'Overview', + 'subsection': 'Welcome', } )