diff --git a/common/lib/xmodule/xmodule/block_metadata_utils.py b/common/lib/xmodule/xmodule/block_metadata_utils.py new file mode 100644 index 0000000000..3969419597 --- /dev/null +++ b/common/lib/xmodule/xmodule/block_metadata_utils.py @@ -0,0 +1,80 @@ +""" +Simple utility functions that operate on block metadata. + +This is a place to put simple functions that operate on block metadata. It +allows us to share code between the XModuleMixin and CourseOverview and +BlockStructure. +""" + + +def url_name_for_block(block): + """ + Given a block, returns the block's URL name. + + Arguments: + block (XModuleMixin|CourseOverview|BlockStructureBlockData): + Block that is being accessed + """ + return block.location.name + + +def display_name_with_default(block): + """ + Calculates the display name for a block. + + Default to the display_name if it isn't None, else fall back to creating + a name based on the URL. + + Unlike the rest of this module's functions, this function takes an entire + course descriptor/overview as a parameter. This is because a few test cases + (specifically, {Text|Image|Video}AnnotationModuleTestCase.test_student_view) + create scenarios where course.display_name is not None but course.location + is None, which causes calling course.url_name to fail. So, although we'd + like to just pass course.display_name and course.url_name as arguments to + this function, we can't do so without breaking those tests. + + Note: This method no longer escapes as it once did, so the caller must + ensure it is properly escaped where necessary. + + Arguments: + block (XModuleMixin|CourseOverview|BlockStructureBlockData): + Block that is being accessed + """ + return ( + block.display_name if block.display_name is not None + else url_name_for_block(block).replace('_', ' ') + ) + + +def display_name_with_default_escaped(block): + """ + DEPRECATED: use display_name_with_default + + Calculates the display name for a block with some HTML escaping. + This follows the same logic as display_name_with_default, with + the addition of the escaping. + + Here is an example of how to move away from this method in Mako html: + Before: + ${course.display_name_with_default_escaped} + + After: + ${course.display_name_with_default | h} + If the context is Javascript in Mako, you'll need to follow other best practices. + + Note: Switch to display_name_with_default, and ensure the caller + properly escapes where necessary. + + Note: This newly introduced method should not be used. It was only + introduced to enable a quick search/replace and the ability to slowly + migrate and test switching to display_name_with_default, which is no + longer escaped. + + Arguments: + block (XModuleMixin|CourseOverview|BlockStructureBlockData): + Block that is being accessed + """ + # This escaping is incomplete. However, rather than switching this to use + # markupsafe.escape() and fixing issues, better to put that energy toward + # migrating away from this method altogether. + return display_name_with_default(block).replace('<', '<').replace('>', '>') diff --git a/common/lib/xmodule/xmodule/course_metadata_utils.py b/common/lib/xmodule/xmodule/course_metadata_utils.py index 0a6a3ba73c..0586d10274 100644 --- a/common/lib/xmodule/xmodule/course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/course_metadata_utils.py @@ -32,78 +32,6 @@ def clean_course_key(course_key, padding_char): ) -def url_name_for_course_location(location): - """ - Given a course's usage locator, returns the course's URL name. - - Arguments: - location (BlockUsageLocator): The course's usage locator. - """ - return location.name - - -def display_name_with_default(course): - """ - Calculates the display name for a course. - - Default to the display_name if it isn't None, else fall back to creating - a name based on the URL. - - Unlike the rest of this module's functions, this function takes an entire - course descriptor/overview as a parameter. This is because a few test cases - (specifically, {Text|Image|Video}AnnotationModuleTestCase.test_student_view) - create scenarios where course.display_name is not None but course.location - is None, which causes calling course.url_name to fail. So, although we'd - like to just pass course.display_name and course.url_name as arguments to - this function, we can't do so without breaking those tests. - - Note: This method no longer escapes as it once did, so the caller must - ensure it is properly escaped where necessary. - - Arguments: - course (CourseDescriptor|CourseOverview): descriptor or overview of - said course. - """ - return ( - course.display_name if course.display_name is not None - else course.url_name.replace('_', ' ') - ) - - -def display_name_with_default_escaped(course): - """ - DEPRECATED: use display_name_with_default - - Calculates the display name for a course with some HTML escaping. - This follows the same logic as display_name_with_default, with - the addition of the escaping. - - Here is an example of how to move away from this method in Mako html: - Before: - ${course.display_name_with_default_escaped} - - After: - ${course.display_name_with_default | h} - If the context is Javascript in Mako, you'll need to follow other best practices. - - Note: Switch to display_name_with_default, and ensure the caller - properly escapes where necessary. - - Note: This newly introduced method should not be used. It was only - introduced to enable a quick search/replace and the ability to slowly - migrate and test switching to display_name_with_default, which is no - longer escaped. - - Arguments: - course (CourseDescriptor|CourseOverview): descriptor or overview of - said course. - """ - # This escaping is incomplete. However, rather than switching this to use - # markupsafe.escape() and fixing issues, better to put that energy toward - # migrating away from this method altogether. - return course.display_name_with_default.replace('<', '<').replace('>', '>') - - def number_for_course_location(location): """ Given a course's block usage locator, returns the course's number. diff --git a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py index 7feeda2c1a..f733bba3d6 100644 --- a/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py +++ b/common/lib/xmodule/xmodule/tests/test_course_metadata_utils.py @@ -7,11 +7,13 @@ from unittest import TestCase from django.utils.timezone import UTC -from xmodule.course_metadata_utils import ( - clean_course_key, - url_name_for_course_location, +from xmodule.block_metadata_utils import ( + url_name_for_block, display_name_with_default, display_name_with_default_escaped, +) +from xmodule.course_metadata_utils import ( + clean_course_key, number_for_course_location, has_course_started, has_course_ended, @@ -130,9 +132,9 @@ class CourseMetadataUtilsTestCase(TestCase): "course_MNXXK4TTMUWXMMJ2KVXGS5TFOJZWS5DZLAVUGUZNGIYDGK2ZGIYDSNQ~" ), ]), - FunctionTest(url_name_for_course_location, [ - TestScenario((self.demo_course.location,), self.demo_course.location.name), - TestScenario((self.html_course.location,), self.html_course.location.name), + FunctionTest(url_name_for_block, [ + TestScenario((self.demo_course,), self.demo_course.location.name), + TestScenario((self.html_course,), self.html_course.location.name), ]), FunctionTest(display_name_with_default_escaped, [ # Test course with no display name. diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 84d1b5763a..d43f6f3e0b 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -27,7 +27,7 @@ from xblock.fields import ( from xblock.fragment import Fragment from xblock.runtime import Runtime, IdReader, IdGenerator -from xmodule import course_metadata_utils +from xmodule import block_metadata_utils from xmodule.fields import RelativeTime from xmodule.errortracker import exc_info_to_str from xmodule.modulestore.exceptions import ItemNotFoundError @@ -340,7 +340,7 @@ class XModuleMixin(XModuleFields, XBlock): @property def url_name(self): - return course_metadata_utils.url_name_for_course_location(self.location) + return block_metadata_utils.url_name_for_block(self) @property def display_name_with_default(self): @@ -348,7 +348,7 @@ class XModuleMixin(XModuleFields, XBlock): Return a display name for the module: use display_name if defined in metadata, otherwise convert the url name. """ - return course_metadata_utils.display_name_with_default(self) + return block_metadata_utils.display_name_with_default(self) @property def display_name_with_default_escaped(self): @@ -363,7 +363,7 @@ class XModuleMixin(XModuleFields, XBlock): migrate and test switching to display_name_with_default, which is no longer escaped. """ - return course_metadata_utils.display_name_with_default_escaped(self) + return block_metadata_utils.display_name_with_default_escaped(self) @property def tooltip_title(self): diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index ce8ccf53cc..8cfdb6e515 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -20,7 +20,7 @@ from lms.djangoapps import django_comment_client from openedx.core.djangoapps.models.course_details import CourseDetails from static_replace.models import AssetBaseUrlConfig from util.date_utils import strftime_localized -from xmodule import course_metadata_utils +from xmodule import course_metadata_utils, block_metadata_utils from xmodule.course_module import CourseDescriptor, DEFAULT_START_DATE from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore @@ -317,14 +317,14 @@ class CourseOverview(TimeStampedModel): """ Returns this course's URL name. """ - return course_metadata_utils.url_name_for_course_location(self.location) + return block_metadata_utils.url_name_for_block(self) @property def display_name_with_default(self): """ Return reasonable display name for the course. """ - return course_metadata_utils.display_name_with_default(self) + return block_metadata_utils.display_name_with_default(self) @property def display_name_with_default_escaped(self): @@ -338,7 +338,7 @@ class CourseOverview(TimeStampedModel): migrate and test switching to display_name_with_default, which is no longer escaped. """ - return course_metadata_utils.display_name_with_default_escaped(self) + return block_metadata_utils.display_name_with_default_escaped(self) def has_started(self): """