Skip to content

Fix ordering for mitxonline programs#3360

Open
mbertrand wants to merge 5 commits into
mainfrom
mb/mitxonline_program_order
Open

Fix ordering for mitxonline programs#3360
mbertrand wants to merge 5 commits into
mainfrom
mb/mitxonline_program_order

Conversation

@mbertrand
Copy link
Copy Markdown
Member

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/10765

Description (What does it do?)

Refactors assignment of child resource position numbers, based on the mitxonline req_tree attribute

How can this be tested?

Copilot AI review requested due to automatic review settings May 20, 2026 15:17
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors MITx Online program child ordering so program/course relationships preserve the traversal order defined by the MITx Online req_tree, including interleaving display_mode="course" sub-programs among courses.

Changes:

  • Add get_child_positions to derive stable (type, id) -> position mappings from req_tree traversal.
  • Preserve requested course ID ordering in _fetch_courses_by_ids, and stamp explicit position fields onto transformed courses and child programs.
  • Update program loaders to honor explicit position for course and child-program relationships, with new/updated tests covering ordering and gap behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
learning_resources/etl/mitxonline.py Computes child positions from req_tree, preserves course fetch order, and stamps positions onto transformed program children.
learning_resources/etl/mitxonline_test.py Adds unit tests for get_child_positions and ordered _fetch_courses_by_ids; updates program transform expectations.
learning_resources/etl/loaders.py Honors explicit child positions when creating relationships; adjusts pass-2 child-program relationship positioning logic.
learning_resources/etl/loaders_test.py Adds loader tests ensuring explicit course positions are respected and mixed course/sub-program ordering is preserved.

Comment on lines +807 to +820
existing_max = parent_resource.children.aggregate(Max("position")).get(
"position__max"
)
explicit_max = max(
(
cpd["position"]
for cpd in child_programs_data
if cpd.get("position") is not None
),
default=-1,
)
next_position = (
max(existing_max if existing_max is not None else -1, explicit_max) + 1
)
Copy link
Copy Markdown
Member Author

@mbertrand mbertrand May 20, 2026

Choose a reason for hiding this comment

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

I believe this comment is incorrect because of the two-pass design:

In pass 1, load_program calls learning_resource.children.all().delete(), which wipes out all prior children — both PROGRAM_COURSES and PROGRAM_PROGRAMS — and then bulk-creates fresh PROGRAM_COURSES with assigned positions.

Pass 2 only runs after pass 1 has committed. So when it computes existing_max = parent_resource.children.aggregate(Max("position")), the only children present are the freshly-created PROGRAM_COURSES from this run. Stale PROGRAM_PROGRAMS from prior runs are already gone.

Comment thread learning_resources/etl/loaders.py Outdated
@mbertrand mbertrand force-pushed the mb/mitxonline_program_order branch from 40107a4 to 3a01848 Compare May 20, 2026 15:35
@mbertrand mbertrand added the Needs Review An open Pull Request that is ready for review label May 20, 2026
@mbertrand mbertrand force-pushed the mb/mitxonline_program_order branch from 03041df to 27718fd Compare May 20, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants