Skip to content

[BD-04] Create CustomTagTemplateBlock, convert TranslateCustomTagDescriptor to XBlock and remove RawDescriptor #26973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ormsbee merged 4 commits into from
Mar 12, 2021
Merged

[BD-04] Create CustomTagTemplateBlock, convert TranslateCustomTagDescriptor to XBlock and remove RawDescriptor #26973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
556ef4c
Created CustomTagTemplateBlock to use with CustomTagBlock instead of …
symbolist Mar 11, 2021
fb02353
Convert TranslateCustomTagDescriptor to TranslateCustomTagBlock.
symbolist Mar 11, 2021
78cd8be
XMLModuleStore should use HiddenDescriptor instead of RawDescriptor.
symbolist Mar 11, 2021
23d1e5b
Remove unused RawDescriptor and EmptyDataRawDescriptor.
symbolist Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/tests/test_courseware_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cms/envs/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
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={
Expand Down
13 changes: 6 additions & 7 deletions common/lib/xmodule/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,23 @@
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",
"custom_tag_template = xmodule.raw_module:RawDescriptor",
"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",
"course = xmodule.course_module:CourseBlock",
"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",
Expand All @@ -33,12 +30,14 @@
"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",
Expand Down
22 changes: 0 additions & 22 deletions common/lib/xmodule/xmodule/backcompat_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,3 @@ def from_xml(cls, xml_data, system, id_generator):
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

@symbolist symbolist Mar 11, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to template_module.py.

resources_dir = None

@classmethod
def from_xml(cls, xml_data, system, id_generator):
"""
Transforms the xml_data from <$custom_tag attr="" attr=""/> to
<customtag attr="" attr="" impl="$custom_tag"/>
"""

xml_object = etree.fromstring(xml_data)
system.error_tracker(Text('WARNING: the <{tag}> tag is deprecated. '
'Instead, use <customtag impl="{tag}" attr1="..." attr2="..."/>. ')
.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))
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Expand All @@ -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',
}
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
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': ''


Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/modulestore/tests/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/modulestore/xml_importer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
16 changes: 0 additions & 16 deletions common/lib/xmodule/xmodule/raw_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,6 @@ def parse_xml_new_runtime(cls, node, runtime, keys):
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.
Expand All @@ -125,11 +117,3 @@ def definition_to_xml(self, resource_fs): # lint-amnesty, pylint: disable=unuse
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
44 changes: 42 additions & 2 deletions common/lib/xmodule/xmodule/template_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
)
from xmodule.xml_module import XmlMixin

from openedx.core.djangolib.markup import Text

class CustomTagBlock(

class CustomTagTemplateBlock( # pylint: disable=abstract-method

@symbolist symbolist Mar 11, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CustomTagDescriptor inherited from RawDescriptor and custom_tag_template pointed to RawDescriptor as well. So I have made this the base class and CustomTagBlock inherited from it.

RawMixin,
XmlMixin,
EditingMixin,
Expand All @@ -30,7 +32,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
<customtag option="val" option2="val2" impl="tagname"/>
Expand Down Expand Up @@ -124,3 +134,33 @@ def export_to_file(self):
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 `<customtag attr="" attr="" impl="$custom_tag"/>`.
"""
resources_dir = None

@classmethod
def from_xml(cls, xml_data, system, id_generator):
"""
Transforms the xml_data from <$custom_tag attr="" attr=""/> to
<customtag attr="" attr="" impl="$custom_tag"/>
"""

xml_object = etree.fromstring(xml_data)
system.error_tracker(Text('WARNING: the <{tag}> tag is deprecated. '
'Instead, use <customtag impl="{tag}" attr1="..." attr2="..."/>. ')
.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))
4 changes: 2 additions & 2 deletions lms/djangoapps/courseware/tests/test_module_render.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2504,7 +2504,7 @@ def setUp(self):
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):
Expand All @@ -2519,7 +2519,7 @@ def test_dynamic_updates(self, default_ms):

# 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):
"""
Expand Down
3 changes: 1 addition & 2 deletions openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

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


Expand Down Expand Up @@ -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)

@symbolist symbolist Mar 11, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metadata_translations is specified in only two places: XModuleDescriptorToXBlockMixin and XmlParserMixin. Both have the same values.

metadata_translations['id'] = 'discussion_id'
metadata_translations['for'] = 'discussion_target'

Expand Down