diff --git a/common/lib/xmodule/xmodule/annotatable_module.py b/common/lib/xmodule/xmodule/annotatable_module.py index e07f81231d66..a1634ba66cdc 100644 --- a/common/lib/xmodule/xmodule/annotatable_module.py +++ b/common/lib/xmodule/xmodule/annotatable_module.py @@ -19,7 +19,6 @@ ResourceTemplates, shim_xmodule_js, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) @@ -35,7 +34,6 @@ class AnnotatableBlock( RawMixin, XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 0a7f2674f974..89aa266f4057 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -41,7 +41,6 @@ from xmodule.x_module import ( HTMLSnippet, ResourceTemplates, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js @@ -131,7 +130,6 @@ class ProblemBlock( RawMixin, XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, @@ -163,11 +161,6 @@ class ProblemBlock( mako_template = "widgets/problem-edit.html" has_author_view = True - # The capa format specifies that what we call max_attempts in the code - # is the attribute `attempts`. This will do that conversion - metadata_translations = dict(XmlMixin.metadata_translations) - metadata_translations['attempts'] = 'max_attempts' - icon_class = 'problem' uses_xmodule_styles_setup = True diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 3605f36bf828..238d5564fffd 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -27,7 +27,6 @@ ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -44,7 +43,6 @@ class ConditionalBlock( SequenceMixin, MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index a96631954e1d..db75190189ed 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -21,7 +21,6 @@ HTMLSnippet, ResourceTemplates, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) @@ -47,7 +46,6 @@ class ErrorFields: @XBlock.needs('mako') class ErrorBlock( ErrorFields, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, @@ -181,6 +179,16 @@ def from_xml(cls, xml_data, system, id_generator, # pylint: disable=arguments-d return cls._construct(system, xml_data, error_msg, location=id_generator.create_definition('error')) + @classmethod + def parse_xml(cls, node, runtime, keys, id_generator): # lint-amnesty, pylint: disable=unused-argument + """ + Interpret the parsed XML in `node`, creating an XModuleDescriptor. + """ + # It'd be great to not reserialize and deserialize the xml + xml = etree.tostring(node).decode('utf-8') + block = cls.from_xml(xml, runtime, id_generator) + return block + def export_to_xml(self, resource_fs): ''' If the definition data is invalid xml, export it wrapped in an "error" @@ -201,6 +209,25 @@ def export_to_xml(self, resource_fs): err_node.text = self.error_msg return etree.tostring(root, encoding='unicode') + def add_xml_to_node(self, node): + """ + Export this :class:`XModuleDescriptor` as XML, by setting attributes on the provided + `node`. + """ + xml_string = self.export_to_xml(self.runtime.export_fs) + exported_node = etree.fromstring(xml_string) + node.tag = exported_node.tag + node.text = exported_node.text + node.tail = exported_node.tail + + for key, value in exported_node.items(): + if key == 'url_name' and value == 'course' and key in node.attrib: + # if url_name is set in ExportManager then do not override it here. + continue + node.set(key, value) + + node.extend(list(exported_node)) + class NonStaffErrorBlock(ErrorBlock): # pylint: disable=abstract-method """ diff --git a/common/lib/xmodule/xmodule/hidden_module.py b/common/lib/xmodule/xmodule/hidden_module.py index 0e9270f048bf..5999b777e0a7 100644 --- a/common/lib/xmodule/xmodule/hidden_module.py +++ b/common/lib/xmodule/xmodule/hidden_module.py @@ -7,7 +7,6 @@ from xmodule.raw_module import RawMixin from xmodule.xml_module import XmlMixin from xmodule.x_module import ( - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -17,7 +16,6 @@ class HiddenDescriptor( RawMixin, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, XModuleMixin, ): diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index de22e1a823d6..e6ac691cc171 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -30,7 +30,6 @@ ResourceTemplates, shim_xmodule_js, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) from xmodule.xml_module import XmlMixin, name_to_pathname @@ -47,7 +46,7 @@ @XBlock.needs("user") class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, + XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, ): """ The HTML XBlock mixin. diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index d3bce47bb8af..b6c2257c4a02 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -33,7 +33,6 @@ shim_xmodule_js, STUDENT_VIEW, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) @@ -73,7 +72,6 @@ def _get_capa_types(): class LibraryContentBlock( MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/lti_module.py b/common/lib/xmodule/xmodule/lti_module.py index ce17fb0f11c9..a2174f1f678d 100644 --- a/common/lib/xmodule/xmodule/lti_module.py +++ b/common/lib/xmodule/xmodule/lti_module.py @@ -90,7 +90,6 @@ HTMLSnippet, ResourceTemplates, shim_xmodule_js, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -282,7 +281,6 @@ class LTIBlock( XmlMixin, EditingMixin, MakoTemplateBlockBase, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index dab3bf2fada4..869d45cd1564 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -242,10 +242,6 @@ def load_item(self, location, for_parent=None): # lint-amnesty, pylint: disable definition = json_data.get('definition', {}) metadata = json_data.get('metadata', {}) - for old_name, new_name in getattr(class_, 'metadata_translations', {}).items(): - if old_name in metadata: - metadata[new_name] = metadata[old_name] - del metadata[old_name] children = [ self._convert_reference_to_key(childloc) diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index 6ec06009e5d8..c48637d45141 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -28,7 +28,6 @@ ResourceTemplates, shim_xmodule_js, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) from xmodule.xml_module import XmlMixin @@ -41,7 +40,6 @@ class PollBlock( MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/randomize_module.py b/common/lib/xmodule/xmodule/randomize_module.py index 2079c7246fc0..8caeea20f86f 100644 --- a/common/lib/xmodule/xmodule/randomize_module.py +++ b/common/lib/xmodule/xmodule/randomize_module.py @@ -14,7 +14,6 @@ HTMLSnippet, ResourceTemplates, STUDENT_VIEW, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -26,7 +25,6 @@ class RandomizeBlock( SequenceMixin, MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 6ffec460323f..adf566a38a47 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -29,7 +29,6 @@ ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -261,7 +260,6 @@ class SequenceBlock( ProctoringFields, MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 870f6ea5e0d6..069f9995481f 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -30,7 +30,6 @@ ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, - XModuleDescriptorToXBlockMixin, XModuleMixin, XModuleToXBlockMixin, ) @@ -132,7 +131,6 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method ProctoringFields, MakoTemplateBlockBase, XmlMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 51bc6ff2b209..86aadcf16927 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -16,7 +16,6 @@ ResourceTemplates, shim_xmodule_js, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) from xmodule.xml_module import XmlMixin @@ -28,7 +27,6 @@ class CustomTagTemplateBlock( # pylint: disable=abstract-method RawMixin, XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, @@ -139,7 +137,6 @@ def export_to_file(self): class TranslateCustomTagBlock( # pylint: disable=abstract-method - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, XModuleMixin, ): diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index ae4fae9960af..18c8e81e95a2 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -45,7 +45,7 @@ from xmodule.x_module import ( PUBLIC_VIEW, STUDENT_VIEW, HTMLSnippet, ResourceTemplates, shim_xmodule_js, - XModuleMixin, XModuleToXBlockMixin, XModuleDescriptorToXBlockMixin, + XModuleMixin, XModuleToXBlockMixin, ) from xmodule.xml_module import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname @@ -113,7 +113,7 @@ class VideoBlock( VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers, TabsEditingMixin, EmptyDataRawMixin, XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, + XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, LicenseMixin): """ XML source example: diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index 5fc1c737be65..2591772b7a78 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -24,7 +24,6 @@ ResourceTemplates, shim_xmodule_js, XModuleMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, ) log = logging.getLogger(__name__) @@ -49,7 +48,6 @@ class WordCloudBlock( # pylint: disable=abstract-method EmptyDataRawMixin, XmlMixin, EditingMixin, - XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 46ae9ce15682..f12a2189aa4a 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1104,99 +1104,8 @@ def get_template(cls, template_id): return template -class XModuleDescriptorToXBlockMixin: - """ - Common code needed by XModuleDescriptor and XBlocks converted from XModules. - """ - # VS[compat]. Backwards compatibility code that can go away after - # importing 2012 courses. - # A set of metadata key conversions that we want to make - metadata_translations = { - 'slug': 'url_name', - 'name': 'display_name', - } - - @classmethod - def _translate(cls, key): - return cls.metadata_translations.get(key, key) - - # ================================= XML PARSING ============================ - @classmethod - def parse_xml(cls, node, runtime, keys, id_generator): # lint-amnesty, pylint: disable=unused-argument - """ - Interpret the parsed XML in `node`, creating an XModuleDescriptor. - """ - # It'd be great to not reserialize and deserialize the xml - xml = etree.tostring(node).decode('utf-8') - block = cls.from_xml(xml, runtime, id_generator) - return block - - @classmethod - def parse_xml_new_runtime(cls, node, runtime, keys): - """ - This XML lives within Blockstore and the new runtime doesn't need this - legacy XModule code. Use the "normal" XBlock parsing code. - """ - try: - return super().parse_xml_new_runtime(node, runtime, keys) - except AttributeError: - return super().parse_xml(node, runtime, keys, id_generator=None) - - @classmethod - def from_xml(cls, xml_data, system, id_generator): - """ - Creates an instance of this descriptor from the supplied xml_data. - This may be overridden by subclasses. - - Args: - xml_data (str): A string of xml that will be translated into data and children - for this module - - system (:class:`.XMLParsingSystem): - - id_generator (:class:`xblock.runtime.IdGenerator`): Used to generate the - usage_ids and definition_ids when loading this xml - - """ - raise NotImplementedError('Modules must implement from_xml to be parsable from xml') - - def add_xml_to_node(self, node): - """ - Export this :class:`XModuleDescriptor` as XML, by setting attributes on the provided - `node`. - """ - xml_string = self.export_to_xml(self.runtime.export_fs) - exported_node = etree.fromstring(xml_string) - node.tag = exported_node.tag - node.text = exported_node.text - node.tail = exported_node.tail - - for key, value in exported_node.items(): - if key == 'url_name' and value == 'course' and key in node.attrib: - # if url_name is set in ExportManager then do not override it here. - continue - node.set(key, value) - - node.extend(list(exported_node)) - - def export_to_xml(self, resource_fs): - """ - Returns an xml string representing this module, and all modules - underneath it. May also write required resources out to resource_fs. - - Assumes that modules have single parentage (that no module appears twice - in the same course), and that it is thus safe to nest modules as xml - children as appropriate. - - The returned XML should be able to be parsed back into an identical - XModuleDescriptor using the from_xml method with the same system, org, - and course - """ - raise NotImplementedError('Modules must implement export_to_xml to enable xml export') - - @XBlock.needs("i18n") -class XModuleDescriptor(XModuleDescriptorToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method +class XModuleDescriptor(HTMLSnippet, ResourceTemplates, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method """ An XModuleDescriptor is a specification for an element of a course. This could be a problem, an organizational element (a group of content), or a diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 1bfcf03d03c4..8afc6a569589 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -112,21 +112,6 @@ class XmlParserMixin: xml_attributes = Dict(help="Map of unhandled xml attributes, used only for storage between import and export", default={}, scope=Scope.settings) - # VS[compat]. Backwards compatibility code that can go away after - # importing 2012 courses. - # A set of metadata key conversions that we want to make - metadata_translations = { - 'slug': 'url_name', - 'name': 'display_name', - } - - @classmethod - def _translate(cls, key): - """ - VS[compat] - """ - return cls.metadata_translations.get(key, key) - # The attributes will be removed from the definition xml passed # to definition_from_xml, and from the xml returned by definition_to_xml @@ -275,8 +260,6 @@ def load_metadata(cls, xml_object): """ metadata = {'xml_attributes': {}} for attr, val in xml_object.attrib.items(): - # VS[compat]. Remove after all key translations done - attr = cls._translate(attr) if attr in cls.metadata_to_strip: # don't load these @@ -295,7 +278,6 @@ def apply_policy(cls, metadata, policy): through the attrmap. Updates the metadata dict in place. """ for attr, value in policy.items(): - attr = cls._translate(attr) if attr not in cls.fields: # Store unknown attributes coming from policy.json # in such a way that they will export to xml unchanged @@ -561,9 +543,9 @@ def parse_xml(cls, node, runtime, keys, id_generator): Interpret the parsed XML in `node`, creating an XModuleDescriptor. """ if cls.from_xml != XmlMixin.from_xml: - # Skip the parse_xml from XmlParserMixin to get the shim parse_xml - # from XModuleDescriptor, which actually calls `from_xml`. - return super(XmlParserMixin, cls).parse_xml(node, runtime, keys, id_generator) # pylint: disable=bad-super-call + xml = etree.tostring(node).decode('utf-8') + block = cls.from_xml(xml, runtime, id_generator) + return block else: return super().parse_xml(node, runtime, keys, id_generator) @@ -605,9 +587,19 @@ def add_xml_to_node(self, node): `node`. """ if self.export_to_xml != XmlMixin.export_to_xml: # lint-amnesty, pylint: disable=comparison-with-callable - # Skip the add_xml_to_node from XmlParserMixin to get the shim add_xml_to_node - # from XModuleDescriptor, which actually calls `export_to_xml`. - super(XmlParserMixin, self).add_xml_to_node(node) # pylint: disable=bad-super-call + xml_string = self.export_to_xml(self.runtime.export_fs) + exported_node = etree.fromstring(xml_string) + node.tag = exported_node.tag + node.text = exported_node.text + node.tail = exported_node.tail + + for key, value in exported_node.items(): + if key == 'url_name' and value == 'course' and key in node.attrib: + # if url_name is set in ExportManager then do not override it here. + continue + node.set(key, value) + + node.extend(list(exported_node)) else: super().add_xml_to_node(node) diff --git a/common/test/data/conditional/course.xml b/common/test/data/conditional/course.xml index 095fa3bfb8c8..8337bf0d75b2 100644 --- a/common/test/data/conditional/course.xml +++ b/common/test/data/conditional/course.xml @@ -1,6 +1,6 @@ - - + + diff --git a/common/test/data/conditional_and_poll/course/2013_Spring.xml b/common/test/data/conditional_and_poll/course/2013_Spring.xml index 2eea422a2f97..58076a7fada6 100644 --- a/common/test/data/conditional_and_poll/course/2013_Spring.xml +++ b/common/test/data/conditional_and_poll/course/2013_Spring.xml @@ -2,7 +2,7 @@ Take note of this name exactly, you'll need to use it everywhere. --> - + diff --git a/common/test/data/course_ignore/course.xml b/common/test/data/course_ignore/course.xml index 7761c9811ac6..418ad0634c0a 100644 --- a/common/test/data/course_ignore/course.xml +++ b/common/test/data/course_ignore/course.xml @@ -1 +1 @@ - + diff --git a/common/test/data/manual-testing-complete/problem/45c317cb93d447f293ce982a2eccd77d.xml b/common/test/data/manual-testing-complete/problem/45c317cb93d447f293ce982a2eccd77d.xml index db34051c2bd6..45594848487b 100644 --- a/common/test/data/manual-testing-complete/problem/45c317cb93d447f293ce982a2eccd77d.xml +++ b/common/test/data/manual-testing-complete/problem/45c317cb93d447f293ce982a2eccd77d.xml @@ -1,4 +1,4 @@ - +