From 18ec99dd0b909041307196f2e2b1d4eea165e189 Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Tue, 26 Apr 2022 05:09:36 -0400 Subject: [PATCH 1/9] feat: change default title for text xblock --- cms/djangoapps/contentstore/views/item.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 69c3117417..420f8c1a5b 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -427,13 +427,16 @@ def xblock_view_handler(request, usage_key_string, view_name): # Note that the container view recursively adds headers into the preview fragment, # so only the "Pages" view requires that this extra wrapper be included. + display_label = xblock.display_name or xblock.scope_ids.block_type + if not xblock.display_name and xblock.scope_ids.block_type == 'html': + display_label = 'Text' if is_pages_view: fragment.content = render_to_string('component.html', { 'xblock_context': context, 'xblock': xblock, 'locator': usage_key, 'preview': fragment.content, - 'label': xblock.display_name or xblock.scope_ids.block_type, + 'label': display_label, }) else: raise Http404 From 109e68ca943839537e1c701f4795522f88f03f8b Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Tue, 3 May 2022 12:21:56 -0400 Subject: [PATCH 2/9] feat: html xblock default title is now 'Text' --- cms/djangoapps/contentstore/utils.py | 7 +++++++ cms/templates/studio_xblock_wrapper.html | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1e0d3e60c6..775de560ce 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -702,6 +702,13 @@ def get_sibling_urls(subsection, unit_location): # pylint: disable=too-many-s next_url = reverse_usage_url('container_handler', next_loc) return prev_url, next_url +def determine_label(display_name, block_type): + if display_name: + return display_name + if block_type == 'html': + return 'Text' + return block_type + @contextmanager def translation_language(language): diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 15a2d3a857..5f3b5bf875 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -2,7 +2,7 @@ <%! from django.utils.translation import gettext as _ from cms.djangoapps.contentstore.views.helpers import xblock_studio_url -from cms.djangoapps.contentstore.utils import is_visible_to_specific_partition_groups, get_editor_page_base_url +from cms.djangoapps.contentstore.utils import is_visible_to_specific_partition_groups, get_editor_page_base_url, determine_label from lms.lib.utils import is_unit from openedx.core.djangolib.js_utils import ( dump_js_escaped_json, js_escaped_string @@ -17,7 +17,7 @@ xblock_url = xblock_studio_url(xblock) show_inline = xblock.has_children and not xblock_url section_class = "level-nesting" if show_inline else "level-element" collapsible_class = "is-collapsible" if xblock.has_children else "" -label = xblock.display_name_with_default or xblock.scope_ids.block_type +label = determine_label(xblock.display_name_with_default, xblock.scope_ids.block_type) messages = xblock.validate().to_json() block_is_unit = is_unit(xblock) %> From a9a945094e0c05847861e85731e418eed66fba9f Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Mon, 16 May 2022 12:07:47 -0400 Subject: [PATCH 3/9] feat: add comment --- cms/djangoapps/contentstore/utils.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 775de560ce..8939956caf 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -703,6 +703,10 @@ def get_sibling_urls(subsection, unit_location): # pylint: disable=too-many-s return prev_url, next_url def determine_label(display_name, block_type): + """ + Returns the name of the xblock to display in studio. + Please see TNL-9838. + """ if display_name: return display_name if block_type == 'html': From c63d871e5c9f9f71c267cb9d61d1299b5d99a126 Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Fri, 27 May 2022 02:58:07 -0400 Subject: [PATCH 4/9] feat: add testing and i18n --- .../contentstore/tests/test_utils.py | 30 +++++++++++++++++++ cms/djangoapps/contentstore/utils.py | 7 ++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 2ed40b6942..832b6eccad 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -736,3 +736,33 @@ class ValidateCourseOlxTests(CourseTestCase): ignore=ignore, allowed_xblocks=allowed_xblocks ) + +class DetermineLabelTestCase(TestCase): + """Tests for xblock Title quirks""" + + def validate_html_replaced_with_text(self): + """ + Tests that display names for "html" xblocks are repleaced with "Text" when the display name is otherwise unset. + """ + display_name = None + block_type = "html" + result = utils.determine_label(display_name, block_type) + self.assertTrue(result == "Text") + + def validate_set_titles_not_replaced(self): + """ + Tests that display names for "html" xblocks are not repleaced with "Text" when the display name is set. + """ + display_name = "Something" + block_type = "html" + result = utils.determine_label(display_name, block_type) + self.assertTrue(result == display_name) + + def validate_set_titles_not_replaced(self): + """ + Tests that display names for non-"html" xblocks are not repleaced with "Text" when the display name is set. + """ + display_name = None + block_type = "something else" + result = utils.determine_label(display_name, block_type) + self.assertTrue(result == display_name) \ No newline at end of file diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 8939956caf..c2a3cd1931 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -707,11 +707,10 @@ def determine_label(display_name, block_type): Returns the name of the xblock to display in studio. Please see TNL-9838. """ - if display_name: - return display_name + label = display_name if block_type == 'html': - return 'Text' - return block_type + label = _("Text") + return label @contextmanager From 701bfbc8c6eb0606fcc087d9573b10b0b14c88ee Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Fri, 27 May 2022 03:24:18 -0400 Subject: [PATCH 5/9] feat: pep8 fixes --- cms/djangoapps/contentstore/tests/test_utils.py | 4 +++- cms/djangoapps/contentstore/utils.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 832b6eccad..04c3504e51 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -737,6 +737,7 @@ class ValidateCourseOlxTests(CourseTestCase): allowed_xblocks=allowed_xblocks ) + class DetermineLabelTestCase(TestCase): """Tests for xblock Title quirks""" @@ -765,4 +766,5 @@ class DetermineLabelTestCase(TestCase): display_name = None block_type = "something else" result = utils.determine_label(display_name, block_type) - self.assertTrue(result == display_name) \ No newline at end of file + self.assertTrue(result == display_name) + \ No newline at end of file diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c2a3cd1931..bb5a53fb94 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -702,6 +702,7 @@ def get_sibling_urls(subsection, unit_location): # pylint: disable=too-many-s next_url = reverse_usage_url('container_handler', next_loc) return prev_url, next_url + def determine_label(display_name, block_type): """ Returns the name of the xblock to display in studio. From cf84c3bc645318302b62b31c8bff3a7ab27d7bfb Mon Sep 17 00:00:00 2001 From: rayzhou-bit Date: Fri, 27 May 2022 03:48:33 -0400 Subject: [PATCH 6/9] feat: more pep8 fix --- cms/djangoapps/contentstore/tests/test_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 04c3504e51..5a19fc0a9d 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -767,4 +767,3 @@ class DetermineLabelTestCase(TestCase): block_type = "something else" result = utils.determine_label(display_name, block_type) self.assertTrue(result == display_name) - \ No newline at end of file From 0678a444ecb726fc4e98115a63b17c3db261f13e Mon Sep 17 00:00:00 2001 From: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com> Date: Fri, 27 May 2022 10:23:39 -0700 Subject: [PATCH 7/9] feat: missed a translation in item.py --- cms/djangoapps/contentstore/views/item.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 420f8c1a5b..2ef9b80d38 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -429,7 +429,7 @@ def xblock_view_handler(request, usage_key_string, view_name): # so only the "Pages" view requires that this extra wrapper be included. display_label = xblock.display_name or xblock.scope_ids.block_type if not xblock.display_name and xblock.scope_ids.block_type == 'html': - display_label = 'Text' + display_label = _("Text") if is_pages_view: fragment.content = render_to_string('component.html', { 'xblock_context': context, From 9c8b0265885170f564222e56ece95c8195923455 Mon Sep 17 00:00:00 2001 From: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com> Date: Fri, 27 May 2022 10:47:27 -0700 Subject: [PATCH 8/9] feat: assertEqual --- cms/djangoapps/contentstore/tests/test_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 5a19fc0a9d..1a4f2f6e91 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -748,7 +748,7 @@ class DetermineLabelTestCase(TestCase): display_name = None block_type = "html" result = utils.determine_label(display_name, block_type) - self.assertTrue(result == "Text") + self.assertEqual(result, display_name) def validate_set_titles_not_replaced(self): """ @@ -757,7 +757,7 @@ class DetermineLabelTestCase(TestCase): display_name = "Something" block_type = "html" result = utils.determine_label(display_name, block_type) - self.assertTrue(result == display_name) + self.assertEqual(result, display_name) def validate_set_titles_not_replaced(self): """ @@ -766,4 +766,4 @@ class DetermineLabelTestCase(TestCase): display_name = None block_type = "something else" result = utils.determine_label(display_name, block_type) - self.assertTrue(result == display_name) + self.assertEqual(result, display_name) From 8422f50a160f26e431f31ac1c40dfeea6792cbbc Mon Sep 17 00:00:00 2001 From: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com> Date: Fri, 27 May 2022 11:05:52 -0700 Subject: [PATCH 9/9] feat: fix test --- cms/djangoapps/contentstore/tests/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 1a4f2f6e91..d0c76125e9 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -759,7 +759,7 @@ class DetermineLabelTestCase(TestCase): result = utils.determine_label(display_name, block_type) self.assertEqual(result, display_name) - def validate_set_titles_not_replaced(self): + def validate_non_html_blocks_titles_not_replaced(self): """ Tests that display names for non-"html" xblocks are not repleaced with "Text" when the display name is set. """