diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 78a5b8c1c1c9..308d38bc181f 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -20,7 +20,7 @@ from xmodule.studio_editable import StudioEditableBlock from xmodule.util.builtin_assets import add_webpack_js_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.xml_block import XmlMixin +from xmodule.xml_block import XmlMixin, is_pointer_tag from xmodule.x_module import ( ResourceTemplates, shim_xmodule_js, @@ -335,7 +335,19 @@ def definition_from_xml(cls, xml_object, system): show_tag_list.append(location) else: try: - block = system.process_xml(etree.tostring(child, encoding='unicode')) + def_id = None + def_loaded = False + + if is_pointer_tag(child): + def_id = system.id_generator.create_definition(child.tag, child.get('url_name')) + child, _ = cls.load_definition_xml(child, system, def_id) + def_loaded = True + + block = system.process_xml( + etree.tostring(child, encoding='unicode'), + def_id, + def_loaded=def_loaded, + ) children.append(block.scope_ids.usage_id) except: # lint-amnesty, pylint: disable=bare-except msg = "Unable to load child when parsing Conditional." diff --git a/xmodule/item_bank_block.py b/xmodule/item_bank_block.py index b53617e2c8e4..3866aac985d4 100644 --- a/xmodule/item_bank_block.py +++ b/xmodule/item_bank_block.py @@ -24,7 +24,7 @@ from xmodule.studio_editable import StudioEditableBlock from xmodule.util.builtin_assets import add_webpack_js_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage -from xmodule.xml_block import XmlMixin +from xmodule.xml_block import XmlMixin, is_pointer_tag from xmodule.x_module import ( ResourceTemplates, XModuleMixin, @@ -406,7 +406,21 @@ def definition_from_xml(cls, xml_object, system): for child in xml_object.getchildren(): try: - children.append(system.process_xml(etree.tostring(child)).scope_ids.usage_id) + def_id = None + def_loaded = False + + if is_pointer_tag(child): + def_id = system.id_generator.create_definition(child.tag, child.get('url_name')) + child, _ = cls.load_definition_xml(child, system, def_id) + def_loaded = True + + child_block = system.process_xml( + etree.tostring(child, encoding='unicode'), + def_id, + def_loaded=def_loaded, + ) + + children.append(child_block.scope_ids.usage_id) except (XMLSyntaxError, AttributeError): msg = ( "Unable to load child when parsing {self.usage_key.block_type} Block. " diff --git a/xmodule/modulestore/xml.py b/xmodule/modulestore/xml.py index 738035b19438..af28f21ff430 100644 --- a/xmodule/modulestore/xml.py +++ b/xmodule/modulestore/xml.py @@ -65,9 +65,21 @@ def __init__(self, xmlstore, course_id, course_dir, # lint-amnesty, pylint: dis self.load_error_blocks = load_error_blocks self.modulestore = xmlstore - def process_xml(xml): # lint-amnesty, pylint: disable=too-many-statements - """Takes an xml string, and returns a XBlock created from + def process_xml(xml, def_id=None, def_loaded=False): # lint-amnesty, pylint: disable=too-many-statements + """ + Takes an xml string, and returns an XBlock created from that xml. + + Args: + xml (string): A string containing xml. + def_id (BlockUsageLocator): The :class:`BlockUsageLocator` to use as the + definition_id for the block. + def_loaded (bool): Indicates if the pointer tag definition is loaded from + from the pointed-to file. + + Returns: + XBlock: The fully instantiated :class:`~xblock.core.XBlock`. + """ def make_name_unique(xml_data): @@ -159,11 +171,13 @@ def fallback_name(orig_name=None): try: xml_data = etree.fromstring(xml) - make_name_unique(xml_data) + if not def_loaded: + make_name_unique(xml_data) block = self.xblock_from_node( xml_data, None, # parent_id id_manager, + def_id, ) except Exception as err: # pylint: disable=broad-except if not self.load_error_blocks: diff --git a/xmodule/tests/xml/__init__.py b/xmodule/tests/xml/__init__.py index 90545f1b88bb..835bc605ddcd 100644 --- a/xmodule/tests/xml/__init__.py +++ b/xmodule/tests/xml/__init__.py @@ -42,13 +42,14 @@ def get_policy(usage_id): ) self.id_generator = Mock() - def process_xml(self, xml): # pylint: disable=method-hidden + def process_xml(self, xml, def_id=None, def_loaded=False): # pylint: disable=method-hidden """Parse `xml` as an XBlock, and add it to `self._blocks`""" self.get_asides = Mock(return_value=[]) block = self.xblock_from_node( etree.fromstring(xml), None, CourseLocationManager(self.course_id), + def_id, ) self._blocks[str(block.location)] = block return block @@ -61,7 +62,7 @@ def load_item(self, location, for_parent=None): # pylint: disable=method-hidden class XModuleXmlImportTest(TestCase): """Base class for tests that use basic XML parsing""" @classmethod - def process_xml(cls, xml_import_data): + def process_xml(cls, xml_import_data, def_id=None, def_loaded=False): """Use the `xml_import_data` to import an :class:`XBlock` from XML.""" system = InMemorySystem(xml_import_data) - return system.process_xml(xml_import_data.xml_string) + return system.process_xml(xml_import_data.xml_string, def_id) diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 150e6310853e..1f078ad8ee21 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -23,7 +23,7 @@ from xmodule.util.builtin_assets import add_webpack_js_to_fragment from xmodule.util.misc import is_xblock_an_assignment from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW, XModuleFields -from xmodule.xml_block import XmlMixin +from xmodule.xml_block import XmlMixin, is_pointer_tag log = logging.getLogger(__name__) @@ -250,7 +250,21 @@ def definition_from_xml(cls, xml_object, system): children = [] for child in xml_object: try: - child_block = system.process_xml(etree.tostring(child, encoding='unicode')) + def_id = None + def_loaded = False + + if is_pointer_tag(child): + id_generator = system.id_generator + def_id = id_generator.create_definition(child.tag, child.get('url_name')) + + child, _ = cls.load_definition_xml(child, system, def_id) + def_loaded = True + + child_block = system.process_xml( + etree.tostring(child, encoding='unicode'), + def_id, + def_loaded=def_loaded, + ) children.append(child_block.scope_ids.usage_id) except Exception as exc: # pylint: disable=broad-except log.exception("Unable to load child when parsing Vertical. Continuing...") diff --git a/xmodule/x_module.py b/xmodule/x_module.py index d1d23342b7a9..22fdddd8ae92 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -1543,16 +1543,20 @@ def _usage_id_from_node(self, node, parent_id): """ return self.xblock_from_node(node, parent_id, self.id_generator).scope_ids.usage_id - def xblock_from_node(self, node, parent_id, id_generator=None): + def xblock_from_node(self, node, parent_id, id_generator=None, def_id=None): """ Create an XBlock instance from XML data. Args: - xml_data (string): A string containing valid xml. + node (RestrictedElement): The :class:`openedx.core.lib.safe_lxml.xmlparser.RestrictedElement` + containing the XML data to parse. system (XMLParsingSystem): The :class:`.XMLParsingSystem` used to connect the block to the outside world. id_generator (IdGenerator): An :class:`~xblock.runtime.IdGenerator` that - will be used to construct the usage_id and definition_id for the block. + will be used to construct the usage_id and definition_id for the block + if `def_id` is None. + def_id (BlockUsageLocator): The :class:`BlockUsageLocator` to use as the + definition_id for the block. If None, a new definition_id will be generated. Returns: XBlock: The fully instantiated :class:`~xblock.core.XBlock`. @@ -1567,7 +1571,10 @@ def xblock_from_node(self, node, parent_id, id_generator=None): node.attrib.pop('xblock-family', None) url_name = node.get('url_name') # difference from XBlock.runtime - def_id = id_generator.create_definition(block_type, url_name) + + if not def_id: + def_id = id_generator.create_definition(block_type, url_name) + usage_id = id_generator.create_usage(def_id) keys = ScopeIds(None, block_type, def_id, usage_id)