From 3380f88ab3714a5334088724e377961f7173d4fd Mon Sep 17 00:00:00 2001 From: Agrendalath Date: Wed, 16 Mar 2022 00:36:35 +0100 Subject: [PATCH] refactor: delete class `XmlDescriptor` It also adds `@XBlock.needs("i18n")` to `XModuleMixin` because this service is required there. --- cms/djangoapps/contentstore/course_info_model.py | 2 +- xmodule/raw_module.py | 1 - xmodule/tests/test_xml_module.py | 14 +++++++++----- xmodule/x_module.py | 1 + xmodule/xml_module.py | 9 +-------- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index 1013c3939e..5ecfcb1937 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -24,7 +24,7 @@ from xmodule.html_module import CourseInfoBlock # lint-amnesty, pylint: disable from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order -# # This should be in a class which inherits from XmlDescriptor +# # This should be in a class which inherits from XModuleDescriptor log = logging.getLogger(__name__) diff --git a/xmodule/raw_module.py b/xmodule/raw_module.py index 8201f0b63b..06668e3d46 100644 --- a/xmodule/raw_module.py +++ b/xmodule/raw_module.py @@ -4,7 +4,6 @@ import re from lxml import etree from xblock.fields import Scope, String -from xmodule.xml_module import XmlDescriptor # pylint: disable=unused-import from .exceptions import SerializationError diff --git a/xmodule/tests/test_xml_module.py b/xmodule/tests/test_xml_module.py index 9b4d4fc910..09fd771421 100644 --- a/xmodule/tests/test_xml_module.py +++ b/xmodule/tests/test_xml_module.py @@ -20,7 +20,7 @@ from xmodule.tests import get_test_descriptor_system from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml.factories import CourseFactory, ProblemFactory, SequenceFactory from xmodule.x_module import XModuleMixin -from xmodule.xml_module import XmlDescriptor, deserialize_field, serialize_field +from xmodule.xml_module import XmlMixin, deserialize_field, serialize_field class CrazyJsonString(String): @@ -65,7 +65,7 @@ class InheritingFieldDataTest(unittest.TestCase): Tests of InheritingFieldData. """ - class TestableInheritingXBlock(XmlDescriptor): # lint-amnesty, pylint: disable=abstract-method + class TestableInheritingXBlock(XmlMixin): # lint-amnesty, pylint: disable=abstract-method """ An XBlock we can use in these tests. """ @@ -227,11 +227,15 @@ class InheritingFieldDataTest(unittest.TestCase): class EditableMetadataFieldsTest(unittest.TestCase): + class TestableXmlXBlock(XmlMixin, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method + """ + This is subclassing `XModuleMixin` to use metadata fields in the unmixed class. + """ def test_display_name_field(self): editable_fields = self.get_xml_editable_fields(DictFieldData({})) # Tests that the xblock fields (currently tags and name) get filtered out. - # Also tests that xml_attributes is filtered out of XmlDescriptor. + # Also tests that xml_attributes is filtered out of XmlMixin. assert 1 == len(editable_fields), editable_fields self.assert_field_values( editable_fields, 'display_name', XModuleMixin.display_name, @@ -334,13 +338,13 @@ class EditableMetadataFieldsTest(unittest.TestCase): def get_xml_editable_fields(self, field_data): runtime = get_test_descriptor_system() return runtime.construct_xblock_from_class( - XmlDescriptor, + self.TestableXmlXBlock, scope_ids=Mock(), field_data=field_data, ).editable_metadata_fields def get_descriptor(self, field_data): - class TestModuleDescriptor(TestFields, XmlDescriptor): # lint-amnesty, pylint: disable=abstract-method + class TestModuleDescriptor(TestFields, self.TestableXmlXBlock): # lint-amnesty, pylint: disable=abstract-method @property def non_editable_metadata_fields(self): non_editable_fields = super().non_editable_metadata_fields diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 9563abd837..cd21878409 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -279,6 +279,7 @@ class XModuleFields: ) +@XBlock.needs("i18n") class XModuleMixin(XModuleFields, XBlock): """ Fields and methods used by XModules internally. diff --git a/xmodule/xml_module.py b/xmodule/xml_module.py index c57308497b..788f01d78f 100644 --- a/xmodule/xml_module.py +++ b/xmodule/xml_module.py @@ -8,14 +8,12 @@ import os from lxml import etree from lxml.etree import ElementTree, XMLParser -from xblock.core import XBlock, XML_NAMESPACES +from xblock.core import XML_NAMESPACES from xblock.fields import Dict, Scope, ScopeIds from xblock.runtime import KvsFieldData from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata -from .x_module import XModuleMixin - log = logging.getLogger(__name__) # assume all XML files are persisted as utf-8. @@ -505,8 +503,3 @@ class XmlMixin: non_editable_fields = super().non_editable_metadata_fields non_editable_fields.append(XmlMixin.xml_attributes) return non_editable_fields - - -@XBlock.needs("i18n") -class XmlDescriptor(XmlMixin, XModuleMixin): # lint-amnesty, pylint: disable=abstract-method - pass