Skip to content

Conversation

@haftamuk
Copy link
Contributor

@haftamuk haftamuk commented Oct 12, 2025

Configured subsection prerequisites persist during course olx/xml export/import

Overview

This PR introduces support for subsection (sequential) prerequisites in the OLX course format. Course authors can define prerequisite relationships between subsections with configurable completion criteria including minimum score and completion percentage requirements. Currently this information is lost during course import/export. This PR ensures configured subsection prerequisites are persisted during course olx/xml export/import. It allows course authors to define prerequisite relationships, have them exported to OLX format, and finally, have them properly imported.

Problem Statement

Currently, Open edX supports to define prerequisite relationships between subsections in Studio. This enables course authors from enforcing learning pathways where students must complete certain subsections before accessing others, which is crucial for structured learning experiences. But when a course is exported (XML/OLX) all configured relationships of subsections are lost and authors has to do it again manually in studio. This is time consuming and error prone!

Proposed Solution

  • Extend the element in OLX to support prerequisite definitions
  • Maintain full backward compatibility with existing OLX courses

Key Files

  1. Service Layer (xmodule/seq_block.py)

    • Adds prerequisite information to sequential XML for export.
  2. Import Processing Layer (xmodule/modulestore/xml_importer.py)

    • Processes prerequisite relationships during course import

OLX Format

Prerequisite information attached to sequential xml element

<sequential 
required_content="91d0290972c4488db10d7ca3694e13ca" 
min_score="100" 
min_completion="100" 
... >
    <vertical url_name="some_vertical"/>
   ...
</sequential>

Subsection prerequisite configuration how-to

  1. Settings -> Advanced -> Enable subsection prerequisites -> true
image

Note : Even after configuring a complete learning path for a course you can disable the gating feature by simply turning this in to false. Also, if you switch it back to true, all the previous configured learning path will appear with out further work. This means, switching this setting to true/false will not impact persisted learning path configuration, but will impact how the learner will interact with the course.

Note : When you export a course with this flag set to true, the exported XML will contain the information

<course cert_html_view_enabled="true" discussions_settings="{&quot;enable_graded_units&quot;: false, &quot;enable_in_context&quot;: true, &quot;provider_type&quot;: &quot;openedx&quot;, &quot;unit_level_visibility&quot;: true}" display_name="Saturday1"

enable_subsection_gating="true" 
end="null" language="en" start="2030-01-01T00:00:00Z">
  <chapter url_name="ca81aeb19a7f41a49f05a87c95fe299a"/>
  <chapter url_name="4d13584e169241bb890871bd44059d43"/>
  <wiki slug="Saturday1.Saturday1.Saturday1"/>
</course>

Later during import this setting will be applied to the destination course. This is also the case when it is set to false (the destination course will be enable_subsection_gating set to false)

  1. Make a subsection to serve as a prerequisite
image
  1. Use the subsection as a prerequisite for other subsection
image

How to test

python -Wd -m pytest -p no:randomly --ds=cms.envs.test ./xmodule/modulestore/tests/test_xml_importer.py::TestSequentialPrerequisitesImport::test_gating_api_calls

python -Wd -m pytest -p no:randomly --ds=cms.envs.test ./xmodule/tests/test_sequence.py::SequenceBlockTestCase::test_export_sequence_with_prerequisites

Linked Issue: #36995

Supporting information

  1. Unit Prerequisites Lost During Course Export/Import #36995
  2. https://discuss.openedx.org/t/request-for-comments-course-export-import-and-re-run/16508?u=haftamu.kebede

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 12, 2025
@openedx-webhooks
Copy link

openedx-webhooks commented Oct 12, 2025

Thanks for the pull request, @haftamuk!

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.


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:

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.

@github-project-automation github-project-automation bot moved this to Needs Triage in Contributions Oct 12, 2025
@haftamuk haftamuk force-pushed the ESHE branch 4 times, most recently from c847977 to d43f4b4 Compare October 13, 2025 08:41
@haftamuk haftamuk changed the title feat: Unit Prerequisites Lost During Course Export/Import feat: Subsection(sequential) prerequisites lost during course export/import Oct 13, 2025
@haftamuk haftamuk force-pushed the ESHE branch 4 times, most recently from 8a6be77 to f675094 Compare October 13, 2025 09:28
@haftamuk haftamuk marked this pull request as ready for review October 13, 2025 09:53
Copy link
Contributor

@tecoholic tecoholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haftamuk Hi, the changes look good on the export side. I was able to verify that the <prerequisite> block is added to the output XML as expected. However, when I tried to reimport it in a new course, I ended up with this:

image

So, we are clearly missing the "import" part of the solution. Can you kindly add that functionality?

Additionally, I would recommend adding some unit test around the newly created functions. This is a very "core" part of the system and it's best to have unit tests in place.

@mphilbrick211 mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Oct 15, 2025
@haftamuk
Copy link
Contributor Author

So, we are clearly missing the "import" part of the solution. Can you kindly add that functionality?

Kindly see the import functionality is added!

@haftamuk
Copy link
Contributor Author

Additionally, I would recommend adding some unit test around the newly created functions. This is a very "core" part of the system and it's best to have unit tests in place.

@tecoholic I will proceed to ensure existing unit tests pass and introduce new unit tests for the main changes made.
Thank you.

@tecoholic
Copy link
Contributor

@haftamuk Thanks for updating the PR with the import implementation. It looks like you have used a Generative AI / LLM for the implementation and hence can't be used as is. Kindly consult the [official policy on use of LLMs for acceptable usage guidelines. In short, you can use it to experiment and create solutions, but the final code should be from the developer.

That doesn't mean we will have to discard this work. Let's use this as a base for the final solution.

I have an important request - for future work on this PR, please do not force-push changes. This will allow us to work ahead incrementally and rollback things if we end up doing something we don't want in the final solution. Kindly see my inline comments in the diff for specific details.

@tecoholic
Copy link
Contributor

@haftamuk I realized, there is lot of things I have noted in comments. So, let's go step by step in addressing them

  1. Just do a pass on cleaning up the single line changes like empty lines, comments..etc.,
  2. Next, let's focus on the export XML format - this comment

Ping me for a review after that. Let's move on to the import part of the changes after that.

@tecoholic
Copy link
Contributor

@Agrendalath Thank you for explaining things in detail.

@haftamuk haftamuk force-pushed the ESHE branch 7 times, most recently from 294c6ec to c9920ce Compare November 14, 2025 16:27
@haftamuk haftamuk changed the title feat: Subsection(sequential) prerequisites lost during course export/import feat: Configured subsection prerequisites persist during course olx/xml export/import Nov 14, 2025
@haftamuk haftamuk marked this pull request as ready for review November 14, 2025 16:51
@haftamuk
Copy link
Contributor Author

Hello @Agrendalath ,
If you have time, if you can have a look at the PR if it is ready to merge!

Thank you very much.

@Agrendalath
Copy link
Member

@tecoholic , would you be able to review this? I can review it as the CC in the next sprint, but I still have a few things to address in the current one.

cc: @haftamuk

@tecoholic
Copy link
Contributor

@Agrendalath Absolutely. I will review it this sprint and you can do the CC in next.

Copy link
Contributor

@tecoholic tecoholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haftamuk This is looking great. I have a few more asks on the tests department. Can you kindly address those?

self.mock_sequential = mock.Mock()
self.mock_sequential.location = self.sequential_location

def test_gating_api_calls(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests only the happy path where the the mock_sequential has all the necessary values. More test cases convering the other code paths like the if conditional, the try...except block where the values are either missing or not in the expected format (int, string..etc) should be added to this test case.

assert metadata["prev_url"] == "prev_url"

@patch("openedx.core.lib.gating.api.get_required_content")
def test_export_sequence_with_prerequisites(self, mock_get_required_content):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the importer testcase, this only checks the happy path of the code. Let's add test cases where the 'if' conditions in add_prerequisite_to_xml produce other resutls as well.

@haftamuk haftamuk force-pushed the ESHE branch 5 times, most recently from e8b582c to 2ff0c86 Compare November 20, 2025 16:14
@haftamuk haftamuk requested a review from tecoholic November 20, 2025 16:42
Copy link
Contributor

@tecoholic tecoholic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@haftamuk 👍

  • I tested this: I verified that the export and import of courses preserves pre-requisites in sequentials.
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation - Handled in a separate PR.

…rt/import

     *  feat: adds prerequisite information attributes to exported OLX. e.g.
<sequential
required_content="91d0290972c4488db10d7ca3694e13ca"
min_score="100"
min_completion="100"
... >
    <vertical url_name="some_vertical"/>
   ...
</sequential>

    * feat: parse prerequisite information attributes during import and
create gating relationship.

Issue Link: openedx#36995
@haftamuk
Copy link
Contributor Author

Hello @Agrendalath,
I am just trying to pull this to your visibility for a CC review.

Thank you

@tecoholic
Copy link
Contributor

@Agrendalath Hi, a gentle reminder on taking a look at this PR.

@Agrendalath
Copy link
Member

Sorry for the delay, @haftamuk, @tecoholic. I will review this later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Waiting on Author

Development

Successfully merging this pull request may close these issues.

4 participants