XBlock Asides won't run for XBlocks that implement their own `parse_xml` method

[ormsbee]

This means that Asides will not run for many third-party XBlocks, or even standard bundled ones like ORA that use

This came up recently in the context of VideoBlock:

• Course import does not save VideoBlock Asides #36489
• fix: save video asides during XML course import #36500

Quoting @asadali145:

I don't have enough in-depth context of XBlock and the LMS Runtime, so my analysis might be wrong. Here are my thoughts on this:

• All of the XML import code that's also responsible for parsing the asides exists in edx-platform. Adding support to the XBlock repo is very much impossible because we don't really interact with the XBlock internals during the parsing. Some responsible classes and methods for Parsing are:
  • XMLModuleStore recursively parses the XML and, in the process, calls the XMLMixin.parse_xml for all of the course blocks.
  • XMLMixin.parse_xml is responsible for parsing the asides. This is one that's overridden by the Video and ORA blocks and responsible for parsing the Asides.
  • ImportSystem is the runtime that is responsible for processing the XML string to XBlock.
• Adding support to LMS Runtime will require refactoring and is also complex due to the checks around the runtime type. ImportSystem is the import runtime.
• Any other approaches:
  • We might be able to add some support in CourseImportManager. Saying this, I think we should have someone with more domain knowledge take a deeper look and we may find a better approach.

Also, we would like to get this PR reviewed & merged before Teak as the general fix requires more time.

I am attaching the code flow for a Problem block that is followed when parsing the Asides for a Problem Block.

asides_parsing_trace.txt

[ormsbee]

FYI @kdmccormick and @bradenmacdonald

[bradenmacdonald]

@ormsbee @kdmccormick Do you think we could DEPR parse_xml in the future? We need to move toward one standardized system for parsing XML that is used by all XBlocks, with much more narrow hooks for each XBlock class to customize it, e.g. to parse the body if necessary and preserve backwards compatibility. The current situation is a mess.

[ormsbee]

I think this is a post-Teak cut discussion, but my off-the-cuff answer is that XBlocks will have a legitimate need to reach for parsing their own XML, and that ORA is a very good example for that. I was thinking we could mitigate this issue by making a simpler call that either gets done implicitly or needs to be manually invoked from complex XBlock's parse_xml.

[salman2013]

The Aximprovements Team looked into this issue in detail, and after reviewing the complexity of the ImportSystem we agree with Ormsbee that XBlocks have a legitimate need to handle their own XML parsing.

We believe we can create a utility method to provide basic parsing functionality, along with Asides XBlock rendering support, which can then be used by XBlocks to meet their custom parsing needs.