fix: save video asides during XML course import

[asadali145]

Description

During course import, the video asides are not loaded from the XML properly. This results in inconsistent states after the export and reimport of a course. This works fine for the Problem and Other blocks.

This PR saves the Asides for the videos during course XML import.

Supporting information

#36489

Testing instructions

• Checkout master branch
• Create an Aside that applies to video blocks and problem blocks.

  • Alternatively, you can use ol_openedx_chat

    • Create a studio config in CMS Admin at /admin/xblock_config/studioconfig/. Keep it enabled.
    • Go to the CMS Shell and run pip install ol-openedx-chat
    • Now restart CMS
    • Go to {LMS_BASE_URL}/admin/waffle/flag/ and add waffle flag ol_openedx_chat.ol_openedx_chat_enabled. Keep it enabled.
    • Now go to Any course in CMS, and open advanced settings.
    • Now add the below code in Other Course Settings

     {
        "OL_OPENEDX_CHAT_PROBLEM_BLOCK_ENABLED": true,
        "OL_OPENEDX_CHAT_VIDEO_BLOCK_ENABLED": true
     }
    

    • If you cannot find Other Course Settings, you will need to enable it by doing FEATURES["ENABLE_OTHER_COURSE_SETTINGS"] = True in CMS settings.
    • Once all of this is done, you can go to a Unit in the CMS. You will see Enable AI Chat Assistant against problem and video blocks.
• Now, change the Aside fields to a value other than the defaults.
  • If you are testing with ol_openedx_chat](https://github.com/mitodl/open-edx-plugins/tree/main/src/ol_openedx_chat), you should disable the Enable AI Chat Assistant for a few problems and videos.
• Export the course
• Import it in another course in CMS.
• You will notice that for videos, it does not retain the Aside state from the exported course. It retains the state for the Problems.
• Now check out this branch and repeat the above steps.

Deadline

24-02-25
So that it is included in Teak.

[openedx-webhooks]

Thanks for the pull request, @asadali145!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[openedx-webhooks]

Thanks for the pull request, @asadali145!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[ormsbee]

Does this affect any XBlock class that overrides parse_xml? Like ORA for instance?

[asadali145]

Does this affect any XBlock class that overrides parse_xml? Like ORA for instance?

@ormsbee No, this won't affect any other XBlocks. This PR just fixes the VideoBlock.

[ormsbee]

Sorry, I realize this fix only affects VideoBlock. But does the bug that it's fixing for videos also happen with other XBlocks that implement their own parse_xml, like ORA?

[asadali145]

Sorry, I realize this fix only affects VideoBlock. But does the bug that it's fixing for videos also happen with other XBlocks that implement their own parse_xml, like ORA?

@ormsbee Yes, this bug exists in ORA. I looked at the code, and XBlock Asides are not handled in parse_xml. Then, I tested it with our plugin.

Any XBlock that overrides the parse_xml but does not handle the asides should have this bug.

[ormsbee]

Is there a way we can add support at the LMS runtime or xblock repo level so that third-party XBlocks can support asides by default, without having to understand anything about them?

[asadali145]

Is there a way we can add support at the LMS runtime or xblock repo level so that third-party XBlocks can support asides by default, without having to understand anything about them?

@ormsbee Sorry, my reply took longer than expected.

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]

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

Sure, that's fair, and I'll review this as a local fix. My primary concern is that asides will be much more useful and easier to maintain over the long term if XBlock authors have to really understand them and account for them in their own code. And asides are much less likely to get deprecated over the long term if they are both useful and easy to maintain.

FYI to @kdmccormick

Reviewing this now...

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[ormsbee]

@kdmccormick: do we have to worry about Asides breaking when built-in XBlocks are extracted?

[kdmccormick]

@ormsbee Can you be more specific about what you think would break?

If someone's aside directly references XBlock code in edx-platform, then yes, that will break. It will also break if it directly references the dotted path of the block's class (xmodule.html_block.HtmlBlock).

But if someone's aside just depends on the XBlock framework APIs and block tag (e.g. html) then nothing should break. It would be good for us to test with some existing aside to be sure.

[ormsbee]

@kdmccormick: I'm not worried about people importing directly from edx-platform, only that if they make an Aside that wraps html blocks, that it continues to work after HTMLBlock is extracted, given this issue:

• XBlock Asides won't run for XBlocks that implement their own parse_xml method #36576

[kdmccormick]

I see. I'll keep this in mind, and follow up on the issue you linked.