From 556ef4cb39d16f8f6b76fd88963f77e49ad3bec4 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 11 Mar 2021 20:43:16 +0500 Subject: [PATCH 1/4] Created CustomTagTemplateBlock to use with CustomTagBlock instead of RawDescriptor. --- common/lib/xmodule/setup.py | 2 +- common/lib/xmodule/xmodule/template_module.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 22271d305b..07acd8f0ec 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -9,7 +9,6 @@ XMODULES = [ "section = xmodule.backcompat_module:SemanticSectionDescriptor", "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "custom_tag_template = xmodule.raw_module:RawDescriptor", "raw = xmodule.raw_module:RawDescriptor", ] XBLOCKS = [ @@ -20,6 +19,7 @@ XBLOCKS = [ "course = xmodule.course_module:CourseBlock", "course_info = xmodule.html_module:CourseInfoBlock", "customtag = xmodule.template_module:CustomTagBlock", + "custom_tag_template = xmodule.template_module:CustomTagTemplateBlock", "error = xmodule.error_module:ErrorBlock", "hidden = xmodule.hidden_module:HiddenDescriptor", "html = xmodule.html_module:HtmlBlock", diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 8a5211353e..6660e8ebca 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -21,7 +21,7 @@ from xmodule.x_module import ( from xmodule.xml_module import XmlMixin -class CustomTagBlock( +class CustomTagTemplateBlock( # pylint: disable=abstract-method RawMixin, XmlMixin, EditingMixin, @@ -30,7 +30,15 @@ class CustomTagBlock( HTMLSnippet, ResourceTemplates, XModuleMixin, -): # pylint: disable=abstract-method +): + """ + A block which provides templates for CustomTagBlock. The template name + is set on the `impl` attribute of CustomTagBlock. See below for more details + on how to use it. + """ + + +class CustomTagBlock(CustomTagTemplateBlock): # pylint: disable=abstract-method """ This module supports tags of the form From fb02353719419d7277d1453bf1a5fce5880d3625 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 11 Mar 2021 21:02:47 +0500 Subject: [PATCH 2/4] Convert TranslateCustomTagDescriptor to TranslateCustomTagBlock. --- common/lib/xmodule/setup.py | 10 +++--- .../lib/xmodule/xmodule/backcompat_module.py | 22 ------------- common/lib/xmodule/xmodule/template_module.py | 32 +++++++++++++++++++ 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 07acd8f0ec..a1e49da353 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -3,16 +3,12 @@ from setuptools import find_packages, setup XMODULES = [ - "book = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", - "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", - "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor", "raw = xmodule.raw_module:RawDescriptor", ] XBLOCKS = [ "about = xmodule.html_module:AboutBlock", + "book = xmodule.template_module:TranslateCustomTagBlock", "annotatable = xmodule.annotatable_module:AnnotatableBlock", "chapter = xmodule.seq_module:SectionBlock", "conditional = xmodule.conditional_module:ConditionalBlock", @@ -20,9 +16,11 @@ XBLOCKS = [ "course_info = xmodule.html_module:CourseInfoBlock", "customtag = xmodule.template_module:CustomTagBlock", "custom_tag_template = xmodule.template_module:CustomTagTemplateBlock", + "discuss = xmodule.template_module:TranslateCustomTagBlock", "error = xmodule.error_module:ErrorBlock", "hidden = xmodule.hidden_module:HiddenDescriptor", "html = xmodule.html_module:HtmlBlock", + "image = xmodule.template_module:TranslateCustomTagBlock", "library = xmodule.library_root_xblock:LibraryRoot", "library_content = xmodule.library_content_module:LibraryContentBlock", "library_sourced = xmodule.library_sourced_block:LibrarySourcedBlock", @@ -33,12 +31,14 @@ XBLOCKS = [ "problemset = xmodule.seq_module:SequenceBlock", "randomize = xmodule.randomize_module:RandomizeBlock", "sequential = xmodule.seq_module:SequenceBlock", + "slides = xmodule.template_module:TranslateCustomTagBlock", "split_test = xmodule.split_test_module:SplitTestBlock", "static_tab = xmodule.html_module:StaticTabBlock", "unit = xmodule.unit_block:UnitBlock", "vertical = xmodule.vertical_block:VerticalBlock", "video = xmodule.video_module:VideoBlock", "videoalpha = xmodule.video_module:VideoBlock", + "videodev = xmodule.template_module:TranslateCustomTagBlock", "videosequence = xmodule.seq_module:SequenceBlock", "word_cloud = xmodule.word_cloud_module:WordCloudBlock", "wrapper = xmodule.wrapper_module:WrapperBlock", diff --git a/common/lib/xmodule/xmodule/backcompat_module.py b/common/lib/xmodule/xmodule/backcompat_module.py index 471c1b1304..e07911a8b5 100644 --- a/common/lib/xmodule/xmodule/backcompat_module.py +++ b/common/lib/xmodule/xmodule/backcompat_module.py @@ -87,25 +87,3 @@ class SemanticSectionDescriptor(XModuleDescriptor): # lint-amnesty, pylint: dis else: xml_object.tag = 'sequential' return system.process_xml(etree.tostring(xml_object)) - - -class TranslateCustomTagDescriptor(XModuleDescriptor): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring - resources_dir = None - - @classmethod - def from_xml(cls, xml_data, system, id_generator): - """ - Transforms the xml_data from <$custom_tag attr="" attr=""/> to - - """ - - xml_object = etree.fromstring(xml_data) - system.error_tracker(Text('WARNING: the <{tag}> tag is deprecated. ' - 'Instead, use . ') - .format(tag=xml_object.tag)) - - tag = xml_object.tag - xml_object.tag = 'customtag' - xml_object.attrib['impl'] = tag - - return system.process_xml(etree.tostring(xml_object)) diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 6660e8ebca..8729190a07 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -20,6 +20,8 @@ from xmodule.x_module import ( ) from xmodule.xml_module import XmlMixin +from openedx.core.djangolib.markup import Text + class CustomTagTemplateBlock( # pylint: disable=abstract-method RawMixin, @@ -132,3 +134,33 @@ class CustomTagBlock(CustomTagTemplateBlock): # pylint: disable=abstract-method to export them in a file with yet another layer of indirection. """ return False + + +class TranslateCustomTagBlock( # pylint: disable=abstract-method + XModuleDescriptorToXBlockMixin, + XModuleToXBlockMixin, + XModuleMixin, +): + """ + Converts olx of the form `<$custom_tag attr="" attr=""/>` to CustomTagBlock + of the form ``. + """ + resources_dir = None + + @classmethod + def from_xml(cls, xml_data, system, id_generator): + """ + Transforms the xml_data from <$custom_tag attr="" attr=""/> to + + """ + + xml_object = etree.fromstring(xml_data) + system.error_tracker(Text('WARNING: the <{tag}> tag is deprecated. ' + 'Instead, use . ') + .format(tag=xml_object.tag)) + + tag = xml_object.tag + xml_object.tag = 'customtag' + xml_object.attrib['impl'] = tag + + return system.process_xml(etree.tostring(xml_object)) From 78cd8be24df308a50d09deb9095166280572953a Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 11 Mar 2021 21:12:05 +0500 Subject: [PATCH 3/4] XMLModuleStore should use HiddenDescriptor instead of RawDescriptor. In https://github.com/edx/edx-platform/pull/25955 `HiddenDescriptor` (which was a subclass of `RawDescriptor` with a custom `student_view()`) was converted to an XBlock. It is used as the `default_class` by the `CachingDescriptorSystem` classes. However `RawDescriptor` is still being used by `XMLModuleStore`. This has been replaced by `HiddenDescriptor` as well. --- cms/djangoapps/contentstore/tests/test_courseware_index.py | 2 +- cms/envs/test.py | 2 +- common/lib/xmodule/xmodule/modulestore/tests/django_utils.py | 4 ++-- .../xmodule/modulestore/tests/test_mixed_modulestore.py | 2 +- common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py | 2 +- .../xmodule/modulestore/tests/test_split_modulestore.py | 2 +- .../xmodule/modulestore/tests/test_split_w_old_mongo.py | 2 +- common/lib/xmodule/xmodule/modulestore/tests/utils.py | 2 +- common/lib/xmodule/xmodule/modulestore/xml_importer.py | 4 ++-- lms/djangoapps/courseware/tests/test_module_render.py | 4 ++-- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index ca49b5013a..661f75b391 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -101,7 +101,7 @@ class MixedWithOptionsTestCase(MixedSplitTestCase): DATABASE = 'test_mongo_%s' % uuid4().hex[:5] COLLECTION = 'modulestore' ASSET_COLLECTION = 'assetstore' - DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' + DEFAULT_CLASS = 'xmodule.hidden_module.HiddenDescriptor' RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' modulestore_options = { 'default_class': DEFAULT_CLASS, diff --git a/cms/envs/test.py b/cms/envs/test.py index e95c7e8fa6..7da57c79bd 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -98,7 +98,7 @@ BLOCK_STRUCTURES_SETTINGS['PRUNING_ACTIVE'] = True update_module_store_settings( MODULESTORE, module_store_options={ - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': TEST_ROOT / "data", }, doc_store_settings={ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index baebc10a42..3f7729b4b1 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -94,7 +94,7 @@ def draft_mongo_store_config(data_dir): """ modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': data_dir, 'render_template': 'common.djangoapps.edxmako.shortcuts.render_to_string' } @@ -121,7 +121,7 @@ def split_mongo_store_config(data_dir): Defines split module store. """ modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': data_dir, 'render_template': 'common.djangoapps.edxmako.shortcuts.render_to_string', } diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index b983a6b79f..e70320ed2f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -79,7 +79,7 @@ class CommonMixedModuleStoreSetup(CourseComparisonTest): COLLECTION = 'modulestore' ASSET_COLLECTION = 'assetstore' FS_ROOT = DATA_DIR - DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' + DEFAULT_CLASS = 'xmodule.hidden_module.HiddenDescriptor' RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' MONGO_COURSEID = 'MITx/999/2013_Spring' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 4d81e6081e..08da4cfa75 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -49,7 +49,7 @@ DB = 'test_mongo_%s' % uuid4().hex[:5] COLLECTION = 'modulestore' ASSET_COLLECTION = 'assetstore' FS_ROOT = DATA_DIR # TODO (vshnayder): will need a real fs_root for testing load_item -DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' +DEFAULT_CLASS = 'xmodule.hidden_module.HiddenDescriptor' RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 9779640677..fb4768ee6b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -61,7 +61,7 @@ class SplitModuleTest(unittest.TestCase): 'collection': 'modulestore', } modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': tempdir.mkdtemp_clean(), 'xblock_mixins': (InheritanceMixin, XModuleMixin, EditInfoMixin) } diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index 7cd1537cfa..2798bcc9fe 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -42,7 +42,7 @@ class SplitWMongoCourseBootstrapper(unittest.TestCase): } modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': '', 'render_template': mock.Mock(return_value=""), 'xblock_mixins': (InheritanceMixin, XModuleMixin) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index 5208ad183c..60a22b3d11 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -105,7 +105,7 @@ class MixedSplitTestCase(TestCase): """ RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': '{}: {}, {}'.format(t_n, repr(d), repr(ctx)) modulestore_options = { - 'default_class': 'xmodule.raw_module.RawDescriptor', + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', 'fs_root': DATA_DIR, 'render_template': RENDER_TEMPLATE, 'xblock_mixins': (EditInfoMixin, InheritanceMixin, LocationMixin, XModuleMixin), diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 24194131af..a331779bd1 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -227,7 +227,7 @@ class ImportManager: def __init__( self, store, user_id, data_dir, source_dirs=None, - default_class='xmodule.raw_module.RawDescriptor', + default_class='xmodule.hidden_module.HiddenDescriptor', load_error_modules=True, static_content_store=None, target_id=None, verbose=False, do_import_static=True, do_import_python_lib=True, @@ -1182,7 +1182,7 @@ def validate_course_policy(module_store, course_id): def perform_xlint( # lint-amnesty, pylint: disable=missing-function-docstring data_dir, source_dirs, - default_class='xmodule.raw_module.RawDescriptor', + default_class='xmodule.hidden_module.HiddenDescriptor', load_error_modules=True, xblock_mixins=(LocationMixin, XModuleMixin)): err_cnt = 0 diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 9b474668b5..0c8472abe6 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -2504,7 +2504,7 @@ class TestDisabledXBlockTypes(ModuleStoreTestCase): def test_get_item(self, default_ms): with self.store.default_store(default_ms): course = CourseFactory() - self._verify_descriptor('video', course, 'RawDescriptorWithMixins') + self._verify_descriptor('video', course, 'HiddenDescriptorWithMixins') @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) def test_dynamic_updates(self, default_ms): @@ -2519,7 +2519,7 @@ class TestDisabledXBlockTypes(ModuleStoreTestCase): # Now simulate a new request cache. self.store.request_cache.data.clear() - self._verify_descriptor('problem', course, 'RawDescriptorWithMixins', item_usage_id) + self._verify_descriptor('problem', course, 'HiddenDescriptorWithMixins', item_usage_id) def _verify_descriptor(self, category, course, descriptor, item_id=None): """ From 23d1e5b65433a57e38e72edc1ca3d3db165ee4b4 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Thu, 11 Mar 2021 21:19:31 +0500 Subject: [PATCH 4/4] Remove unused RawDescriptor and EmptyDataRawDescriptor. --- common/lib/xmodule/setup.py | 1 - common/lib/xmodule/xmodule/raw_module.py | 16 ---------------- .../xblock_discussion/__init__.py | 3 +-- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index a1e49da353..b314a10e17 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -4,7 +4,6 @@ from setuptools import find_packages, setup XMODULES = [ "section = xmodule.backcompat_module:SemanticSectionDescriptor", - "raw = xmodule.raw_module:RawDescriptor", ] XBLOCKS = [ "about = xmodule.html_module:AboutBlock", diff --git a/common/lib/xmodule/xmodule/raw_module.py b/common/lib/xmodule/xmodule/raw_module.py index a07f51e658..c840ab2f56 100644 --- a/common/lib/xmodule/xmodule/raw_module.py +++ b/common/lib/xmodule/xmodule/raw_module.py @@ -99,14 +99,6 @@ class RawMixin: return block -class RawDescriptor(RawMixin, XmlDescriptor, XMLEditingDescriptor): - """ - Module that provides a raw editing view of its data and children. It - requires that the definition xml is valid. - """ - pass # lint-amnesty, pylint: disable=unnecessary-pass - - class EmptyDataRawMixin: """ Common code between EmptyDataRawDescriptor and XBlocks converted from XModules. @@ -125,11 +117,3 @@ class EmptyDataRawMixin: if self.data: return etree.fromstring(self.data) return etree.Element(self.category) - - -class EmptyDataRawDescriptor(EmptyDataRawMixin, XmlDescriptor, XMLEditingDescriptor): - """ - Version of RawDescriptor for modules which may have no XML data, - but use XMLEditingDescriptor for import/export handling. - """ - pass # lint-amnesty, pylint: disable=unnecessary-pass diff --git a/openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion/__init__.py b/openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion/__init__.py index 573f722275..893fc6bf7c 100644 --- a/openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion/__init__.py +++ b/openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion/__init__.py @@ -19,7 +19,6 @@ from xblockutils.studio_editable import StudioEditableXBlockMixin from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.xblock_builtin import get_css_dependencies, get_js_dependencies -from xmodule.raw_module import RawDescriptor from xmodule.xml_module import XmlParserMixin @@ -74,7 +73,7 @@ class DiscussionXBlock(XBlock, StudioEditableXBlockMixin, XmlParserMixin): # li has_author_view = True # Tells Studio to use author_view # support for legacy OLX format - consumed by XmlParserMixin.load_metadata - metadata_translations = dict(RawDescriptor.metadata_translations) + metadata_translations = dict(XmlParserMixin.metadata_translations) metadata_translations['id'] = 'discussion_id' metadata_translations['for'] = 'discussion_target'