Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: support parsing the pointer-tag OLX for external XBlocks (#37133)

Browse Source 
Previously, the built-in XBlocks (problem, video, etc) could be parsed
from pointer tag syntax, but externally-defined XBlocks (drag-and-drop,
ORA, etc.) could only be parsed from inline syntax. This is because the
built-in blocks have special parsing logic, defined in XmlMixin,
which is not available to external blocks.

This PR shifts the pointer tag parsing "up a level" such that the parent
blocks parse the pointer tag, regardless of whether the child is built-in
or external:
* vertical (aka unit)
* split_test (aka content experiment)
* itembank (aka problem bank)
* library_content (aka randomized legacy library)

The following parent blocks still lack support for external pointer-tag children;
we will fix this in a follow-up PR:
* randomize
* all externally defined container blocks

Part of: https://github.com/openedx/XBlock/issues/823
This commit is contained in:
M. Tayyab Tahir Qureshi
2025-10-03 20:54:30 +05:00
committed by GitHub
parent c943c223b4
commit d382721cf8
6 changed files with 78 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
xmodule/conditional_block.py
View File

@@ -20,7 +20,7 @@ from xmodule.seq_block import SequenceMixin
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 @@ class ConditionalBlock(
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."

18
xmodule/item_bank_block.py
View File

@@ -24,7 +24,7 @@ from xmodule.mako_block import MakoTemplateBlockBase
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 @@ class ItemBankMixin(
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. "

20
xmodule/modulestore/xml.py
View File

@@ -65,9 +65,21 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # lint-amnesty, pyl
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 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # lint-amnesty, pyl
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:

7
xmodule/tests/xml/__init__.py
View File

@@ -42,13 +42,14 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable
)
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 @@ class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem): # pylint: disable
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)

18
xmodule/vertical_block.py
View File

@@ -23,7 +23,7 @@ from xmodule.studio_editable import StudioEditableBlock
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 @@ class VerticalBlock(
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...")

15
xmodule/x_module.py
View File

@@ -1543,16 +1543,20 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr
"""
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 @@ class XMLParsingSystem(DescriptorSystem): # lint-amnesty, pylint: disable=abstr
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)