diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 983471e1ebef..6240ac6787dd 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -407,7 +407,7 @@ def create_export_tarball(course_block, course_key, context, status=None): Updates the context with any error information if applicable. """ - name = course_block.url_name + name = course_block.usage_key.block_id export_file = NamedTemporaryFile(prefix=name + '.', suffix=".tar.gz") # lint-amnesty, pylint: disable=consider-using-with root_dir = path(mkdtemp()) diff --git a/cms/djangoapps/contentstore/tests/test_outlines.py b/cms/djangoapps/contentstore/tests/test_outlines.py index 108edd82df43..6007a67e2b8e 100644 --- a/cms/djangoapps/contentstore/tests/test_outlines.py +++ b/cms/djangoapps/contentstore/tests/test_outlines.py @@ -313,8 +313,8 @@ def test_missing_display_names(self): ) outline, _errs = get_outline_from_modulestore(self.course_key) - assert outline.sections[0].title == section.url_name - assert outline.sections[0].sequences[0].title == sequence.url_name + assert outline.sections[0].title == section.usage_key.block_id + assert outline.sections[0].sequences[0].title == sequence.usage_key.block_id def test_empty_user_partition_groups(self): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 170c341c32be..76f0dadd28ce 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -475,16 +475,16 @@ def test_library_import(self): ) # Refresh library. library = self.store.get_library(lib_key) - children = [self.store.get_item(child).url_name for child in library.children] + children = [self.store.get_item(child).usage_key.block_id for child in library.children] self.assertEqual(len(children), 2) - self.assertIn(test_block.url_name, children) - self.assertIn(test_block2.url_name, children) + self.assertIn(test_block.usage_key.block_id, children) + self.assertIn(test_block2.usage_key.block_id, children) unchanged_lib = self.store.get_library(unchanged_key) - children = [self.store.get_item(child).url_name for child in unchanged_lib.children] + children = [self.store.get_item(child).usage_key.block_id for child in unchanged_lib.children] self.assertEqual(len(children), 2) - self.assertIn(test_block3.url_name, children) - self.assertIn(test_block4.url_name, children) + self.assertIn(test_block3.usage_key.block_id, children) + self.assertIn(test_block4.usage_key.block_id, children) extract_dir = path(tempfile.mkdtemp(dir=settings.DATA_DIR)) # the extract_dir needs to be passed as a relative dir to @@ -507,16 +507,16 @@ def test_library_import(self): self.assertEqual(lib_key, library_items[0].location.library_key) library = self.store.get_library(lib_key) - children = [self.store.get_item(child).url_name for child in library.children] + children = [self.store.get_item(child).usage_key.block_id for child in library.children] self.assertEqual(len(children), 3) - self.assertNotIn(test_block.url_name, children) - self.assertNotIn(test_block2.url_name, children) + self.assertNotIn(test_block.usage_key.block_id, children) + self.assertNotIn(test_block2.usage_key.block_id, children) unchanged_lib = self.store.get_library(unchanged_key) - children = [self.store.get_item(child).url_name for child in unchanged_lib.children] + children = [self.store.get_item(child).usage_key.block_id for child in unchanged_lib.children] self.assertEqual(len(children), 2) - self.assertIn(test_block3.url_name, children) - self.assertIn(test_block4.url_name, children) + self.assertIn(test_block3.usage_key.block_id, children) + self.assertIn(test_block4.usage_key.block_id, children) @ddt.data( ModuleStoreEnum.Branch.draft_preferred, @@ -910,7 +910,7 @@ def test_library_export(self): publish_item=False, youtube_id_1_0=youtube_id ) - name = library.url_name + name = library.usage_key.block_id lib_key = library.location.library_key root_dir = path(tempfile.mkdtemp()) try: @@ -921,8 +921,8 @@ def test_library_export(self): self.assertEqual(lib_xml.get('library'), lib_key.library) block = lib_xml.find('video') self.assertIsNotNone(block) - self.assertEqual(block.get('url_name'), video_block.url_name) - with open(root_dir / name / 'video' / video_block.url_name + '.xml') as xml_block: + self.assertEqual(block.get('url_name'), video_block.usage_key.block_id) + with open(root_dir / name / 'video' / video_block.usage_key.block_id + '.xml') as xml_block: video_xml = lxml.etree.XML(xml_block.read()) self.assertEqual(video_xml.tag, 'video') self.assertEqual(video_xml.get('youtube_id_1_0'), youtube_id) @@ -1101,7 +1101,7 @@ def test_content_library_export_import(self): ) source_library = self.store.get_library(source_library1_key) - self.assertEqual(source_library.url_name, 'library') + self.assertEqual(source_library.usage_key.block_id, 'library') # Import the exported library into a different content library. import_library_from_xml( diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 6a814fb3ef3c..317c1ceefcf4 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -211,14 +211,17 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ if section.hide_from_toc: continue - is_section_active = (chapter.url_name == active_chapter and section.url_name == active_section) + is_section_active = ( + chapter.usage_key.block_id == active_chapter + and section.usage_key.block_id == active_section + ) if is_section_active: found_active_section = True section_context = { # xss-lint: disable=python-deprecated-display-name 'display_name': section.display_name_with_default_escaped, - 'url_name': section.url_name, + 'url_name': section.usage_key.block_id, 'format': section.format if section.format is not None else '', 'due': section.due, 'active': is_section_active, @@ -230,10 +233,10 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ if is_section_active: if last_processed_section: previous_of_active_section = last_processed_section.copy() - previous_of_active_section['chapter_url_name'] = last_processed_chapter.url_name + previous_of_active_section['chapter_url_name'] = last_processed_chapter.usage_key.block_id elif found_active_section and not next_of_active_section: next_of_active_section = section_context.copy() - next_of_active_section['chapter_url_name'] = chapter.url_name + next_of_active_section['chapter_url_name'] = chapter.usage_key.block_id sections.append(section_context) last_processed_section = section_context @@ -243,9 +246,9 @@ def toc_for_course(user, request, course, active_chapter, active_section, field_ # xss-lint: disable=python-deprecated-display-name 'display_name': chapter.display_name_with_default_escaped, 'display_id': display_id, - 'url_name': chapter.url_name, + 'url_name': chapter.usage_key.block_id, 'sections': sections, - 'active': chapter.url_name == active_chapter + 'active': chapter.usage_key.block_id == active_chapter }) return { 'chapters': toc_chapters, diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index bbf9d5394705..bfbc7342f37c 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -883,7 +883,7 @@ def get_course_syllabus_section(course, section_key): try: filesys = course.runtime.resources_fs # first look for a run-specific version - dirs = [path("syllabus") / course.url_name, path("syllabus")] + dirs = [path("syllabus") / course.usage_key.block_id, path("syllabus")] filepath = find_file(filesys, dirs, section_key + ".html") with filesys.open(filepath) as html_file: return replace_static_urls( diff --git a/lms/djangoapps/courseware/tests/test_entrance_exam.py b/lms/djangoapps/courseware/tests/test_entrance_exam.py index 64bba8edfa52..678eba62a5bc 100644 --- a/lms/djangoapps/courseware/tests/test_entrance_exam.py +++ b/lms/djangoapps/courseware/tests/test_entrance_exam.py @@ -217,7 +217,7 @@ def test_get_entrance_exam_content(self): test get entrance exam content method """ exam_chapter = get_entrance_exam_content(self.request.user, self.course) - assert exam_chapter.url_name == self.entrance_exam.url_name + assert exam_chapter.usage_key.block_id == self.entrance_exam.usage_key.block_id assert not user_has_passed_entrance_exam(self.request.user, self.course) answer_entrance_exam_problem(self.course, self.request, self.problem_1) @@ -350,8 +350,8 @@ def _return_table_of_contents(self): self.request.user, self.request, self.course, - self.entrance_exam.url_name, - self.exam_1.url_name, + self.entrance_exam.usage_key.block_id, + self.exam_1.usage_key.block_id, self.field_data_cache ) return toc['chapters'] diff --git a/lms/djangoapps/discussion/django_comment_client/tests/utils.py b/lms/djangoapps/discussion/django_comment_client/tests/utils.py index bc3fdffa11d0..a4869e6d5939 100644 --- a/lms/djangoapps/discussion/django_comment_client/tests/utils.py +++ b/lms/djangoapps/discussion/django_comment_client/tests/utils.py @@ -123,6 +123,6 @@ def topic_name_to_id(course, name): """ return "{course}_{run}_{name}".format( course=course.location.course, - run=course.url_name, + run=course.usage_key.block_id, name=name ) diff --git a/lms/djangoapps/edxnotes/helpers.py b/lms/djangoapps/edxnotes/helpers.py index 5e68ae4b2003..3d55300ccda0 100644 --- a/lms/djangoapps/edxnotes/helpers.py +++ b/lms/djangoapps/edxnotes/helpers.py @@ -417,7 +417,7 @@ def get_course_position(course_block): log.debug("No section found when loading current position in course") return None - urlargs['section'] = section.url_name + urlargs['section'] = section.usage_key.block_id if course_block.position is not None: return { 'display_name': Text(section.display_name_with_default), @@ -430,7 +430,7 @@ def get_course_position(course_block): log.debug("No subsection found when loading current position in course") return None - urlargs['subsection'] = subsection.url_name + urlargs['subsection'] = subsection.usage_key.block_id return { '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 46920e927ad9..6c0d019f2bbd 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -244,8 +244,8 @@ def _get_unit_url(self, course, chapter, section, position=1): """ return reverse('courseware_position', kwargs={ 'course_id': course.id, - 'section': chapter.url_name, - 'subsection': section.url_name, + 'section': chapter.usage_key.block_id, + 'subsection': section.usage_key.block_id, 'position': position, }) @@ -830,7 +830,7 @@ def test_get_course_position_to_section(self): mock_course_block = MagicMock(id=self.course.id, position=3) mock_section = MagicMock() - mock_section.url_name = 'section_url_name' + mock_section.usage_key.block_id = 'section_url_name' mock_section.display_name_with_default = 'Test Chapter Display Name' mock_course_block.get_children.return_value = [mock_section] @@ -856,11 +856,11 @@ def test_get_course_position_to_subsection(self): mock_course_block = MagicMock(id=self.course.id, position=None) mock_section = MagicMock() - mock_section.url_name = 'section_url_name' + mock_section.usage_key.block_id = 'section_url_name' mock_course_block.get_children.return_value = [mock_section] mock_subsection = MagicMock() - mock_subsection.url_name = 'subsection_url_name' + mock_subsection.usage_key.block_id = 'subsection_url_name' mock_subsection.display_name_with_default = 'Test Section Display Name' mock_section.get_children.return_value = [mock_subsection] diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index dd43dcaee88b..1a2aa6a2635c 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -121,7 +121,6 @@ def get_seconds_since_epoch(date_time): 'visible_to_staff_only', 'location', 'number', - 'url_name', 'display_name_with_default', 'display_name_with_default_escaped', 'start_date_is_still_default', diff --git a/openedx/core/lib/xblock_serializer/block_serializer.py b/openedx/core/lib/xblock_serializer/block_serializer.py index 117f584d598a..7e8b6ff3065b 100644 --- a/openedx/core/lib/xblock_serializer/block_serializer.py +++ b/openedx/core/lib/xblock_serializer/block_serializer.py @@ -82,7 +82,7 @@ def _serialize_block(self, block) -> etree.Element: # Handles a weird case where url_name is not part of olx.attrib even if it is # set in block. Known case is with openassessment blocks. if "url_name" not in olx.attrib and hasattr(block, "url_name"): - olx.attrib['url_name'] = block.url_name + olx.attrib['url_name'] = block.usage_key.block_id else: # The url_name attribute can come either because it was already in the # block's field data, or because this class adds it in the calls above. diff --git a/xmodule/html_block.py b/xmodule/html_block.py index 27a2b51e4a31..8c1c15861278 100644 --- a/xmodule/html_block.py +++ b/xmodule/html_block.py @@ -321,7 +321,7 @@ def definition_to_xml(self, resource_fs): ''' # Write html to file, return an empty tag - pathname = name_to_pathname(self.url_name) + pathname = name_to_pathname(self.usage_key.block_id) filepath = '{category}/{pathname}.html'.format( category=self.category, pathname=pathname diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 6f583d8beff2..927ab18a9039 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -168,7 +168,7 @@ def author_view(self, context): fragment.add_content(self.runtime.service(self, 'mako').render_cms_template( "library-block-author-preview-header.html", { 'max_count': max_count, - 'display_name': self.display_name or self.url_name, + 'display_name': self.display_name or self.usage_key.block_id, })) context['can_edit_visibility'] = False context['can_move'] = False diff --git a/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index 01b64275524b..82512b1b09be 100644 --- a/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -186,7 +186,7 @@ def test_split_course_export_import(self): source_course = source_store.get_course(source_course_key, depth=None, lazy=False) # lint-amnesty, pylint: disable=no-member - assert source_course.url_name == 'course' + assert source_course.usage_key.block_id == 'course' export_dir_path = path(self.export_dir) policy_dir = export_dir_path / 'exported_source_course' / 'policies' / source_course_key.run @@ -206,4 +206,4 @@ def test_split_course_export_import(self): dest_course = dest_store.get_course(dest_course_key, depth=None, lazy=False) # lint-amnesty, pylint: disable=no-member - assert dest_course.url_name == 'course' + assert dest_course.usage_key.block_id == 'course' diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index 61c66eefe467..ef0bf5ed5421 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -226,7 +226,9 @@ def test_import(self): other_block = CapaFactory.create() assert block.get_score().raw_earned == 0 - assert block.url_name != other_block.url_name, "Factory should be creating unique names for each problem" + assert ( + block.usage_key.block_id != other_block.usage_key.block_id + ), "Factory should be creating unique names for each problem" def test_correct(self): """ diff --git a/xmodule/tests/test_import.py b/xmodule/tests/test_import.py index c92801ad22e9..debc71d28487 100644 --- a/xmodule/tests/test_import.py +++ b/xmodule/tests/test_import.py @@ -442,8 +442,8 @@ def test_policy_loading(self): toy = self.get_course('toy') two_toys = self.get_course('two_toys') - assert toy.url_name == '2012_Fall' - assert two_toys.url_name == 'TT_2012_Fall' + assert toy.usage_key.block_id == '2012_Fall' + assert two_toys.usage_key.block_id == 'TT_2012_Fall' toy_ch = toy.get_children()[0] two_toys_ch = two_toys.get_children()[0] @@ -513,7 +513,7 @@ def test_colon_in_url_name(self): assert len(chapters) == 5 ch2 = chapters[1] - assert ch2.url_name == 'secret:magic' + assert ch2.usage_key.block_id == 'secret:magic' print("Ch2 location: ", ch2.location) @@ -567,8 +567,8 @@ def test_url_name_mangling(self): for i in (2, 3): video = sections[i] # Name should be 'video_{hash}' - print(f"video {i} url_name: {video.url_name}") - assert len(video.url_name) == (len('video_') + 12) + print(f"video {i} url_name: {video.usage_key.block_id}") + assert len(video.usage_key.block_id) == (len('video_') + 12) def test_poll_and_conditional_import(self): modulestore = XMLModuleStore(DATA_DIR, source_dirs=['conditional_and_poll']) diff --git a/xmodule/tests/test_split_test_block.py b/xmodule/tests/test_split_test_block.py index 7acbaa25f6ba..65223a4ad61f 100644 --- a/xmodule/tests/test_split_test_block.py +++ b/xmodule/tests/test_split_test_block.py @@ -142,7 +142,7 @@ def setUp(self): @ddt.unpack def test_child(self, user_tag, child_url_name): self.user_partition.scheme.current_group = self.user_partition.groups[user_tag] - assert self.split_test_block.child_block.url_name == child_url_name + assert self.split_test_block.child_block.usage_key.block_id == child_url_name @ddt.data((0, 'HTML FOR GROUP 0'), (1, 'HTML FOR GROUP 1')) @ddt.unpack @@ -153,14 +153,17 @@ def test_get_html(self, user_tag, child_content): @ddt.data(0, 1) def test_child_missing_tag_value(self, _user_tag): # If user_tag has a missing value, we should still get back a valid child url - assert self.split_test_block.child_block.url_name in ['split_test_cond0', 'split_test_cond1'] + assert self.split_test_block.child_block.usage_key.block_id in ['split_test_cond0', 'split_test_cond1'] @ddt.data(100, 200, 300, 400, 500, 600, 700, 800, 900, 1000) def test_child_persist_new_tag_value_when_tag_missing(self, _user_tag): # If a user_tag has a missing value, a group should be saved/persisted for that user. # So, we check that we get the same url_name when we call on the url_name twice. # We run the test ten times so that, if our storage is failing, we'll be most likely to notice it. - assert self.split_test_block.child_block.url_name == self.split_test_block.child_block.url_name + assert ( + self.split_test_block.child_block.usage_key.block_id + == self.split_test_block.child_block.usage_key.block_id + ) # Patch the definition_to_xml for the html children. @patch('xmodule.html_block.HtmlBlock.definition_to_xml') diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index e1850fe5b6f3..06d3a11dcf91 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -696,7 +696,7 @@ def definition_to_xml(self, resource_fs): # lint-amnesty, pylint: disable=too-m youtube_string = create_youtube_string(self) if youtube_string: xml.set('youtube', str(youtube_string)) - xml.set('url_name', self.url_name) + xml.set('url_name', self.usage_key.block_id) attrs = [ ('display_name', self.display_name), ('show_captions', json.dumps(self.show_captions)), diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 9eb407a105c0..f436a34f4667 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -346,15 +346,6 @@ def location(self, value): usage_id=value, ) - @property - def url_name(self): - """ - Return the URL-friendly name for this block. - - Preferred forms for new code: `self.usage_key.block_id` - """ - return self.usage_key.block_id - @property def display_name_with_default(self): """ @@ -1552,7 +1543,7 @@ def resource_url(self, resource): def add_block_as_child_node(self, block, node): """Append the block’s XML to the given parent XML node.""" child = etree.SubElement(node, block.category) - child.set("url_name", block.url_name) + child.set("url_name", block.usage_key.block_id) block.add_xml_to_node(child) def publish(self, block, event_type, event_data): diff --git a/xmodule/xml_block.py b/xmodule/xml_block.py index dac8fd93d37b..e6dc402f1af4 100644 --- a/xmodule/xml_block.py +++ b/xmodule/xml_block.py @@ -495,7 +495,7 @@ def add_xml_to_node(self, node): " This could mean data loss!!!", attr, val, - self.url_name, + self.usage_key.block_id, ) for key, value in self.xml_attributes.items(): @@ -504,7 +504,7 @@ def add_xml_to_node(self, node): if self.export_to_file(): # Write the definition to a file - url_path = name_to_pathname(self.url_name) + url_path = name_to_pathname(self.usage_key.block_id) # if folder is course then create file with name {course_run}.xml filepath = self._format_filepath( self.category, @@ -524,7 +524,7 @@ def add_xml_to_node(self, node): # Do not override an existing value for the course. if not node.get("url_name"): - node.set("url_name", self.url_name) + node.set("url_name", self.usage_key.block_id) # Special case for course pointers: if self.category == "course":