Add best_run_id to REST/search API results #2696
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OpenAPI ChangesShow/hide 208 changes: 0 error, 0 warning, 208 infoUnexpected changes? Ensure your branch is up-to-date with |
mbertrand
commented
Nov 12, 2025
| def best_run(self) -> Optional["LearningResourceRun"]: | ||
| """Returns the most current/upcoming enrollable run for the learning resource""" | ||
| published_runs = self.runs.filter(published=True) | ||
| if hasattr(self, "_published_runs"): |
Member
Author
There was a problem hiding this comment.
The inclusion of best_run_id in the serializer introduced lots of n+1 query errors, which is why this property needed to be refactored.
mbertrand
force-pushed
the
mb/next_start_date
branch
from
0f4a863 to
396d00d
Compare
shanbady
approved these changes
Nov 13, 2025
Contributor
shanbady
left a comment
shanbady
left a comment
There was a problem hiding this comment.
Approving 👍 refactoring here looks good and worked as described.
There are two corner cases which are good to be aware of but probably warrants a separate discussion+PR:
- Our contentfile loader has a bunch of logic that tries to pick a "best run" - currently seems to just grab the first published run which is potentially causing issues
- We have these "test_mode" courses which may not have any published runs but should still fallback to something (maybe the first run with contentfiles if it has multiple unpublished runs)
shanbady
added
Waiting on author
and removed
Needs Review
Member
Author
|
I will create a separate issue for the 2 things you mentioned above, I don't think they should be addressed in this PR. |
mbertrand
force-pushed
the
mb/next_start_date
branch
from
396d00d to
5280d25
Compare
mbertrand
changed the title
learning_resources/models.py
Outdated
Comment on lines
577
to
578
| # If no current enrollable run found, find the next upcoming run | ||
| upcoming_runs = [ |
Contributor
There was a problem hiding this comment.
In the past, I've understood "published" to mean "enrollable". Is that no longer true?
Member
Author
There was a problem hiding this comment.
Never was - all OCW courses are published but not enrollable for example, You can access all the course material but there is no enrollment possible. Ditto for OpenLearningLibrary,
Contributor
There was a problem hiding this comment.
Thanks. That makes sense.
Contributor
There was a problem hiding this comment.
Wouldn't OCW and OLL be included in enrollable_runs array via this filter?
not run.enrollment_start
and run.start_date
and run.start_date <= now
)
Member
Author
There was a problem hiding this comment.
No, run.start_date is always None for OCW and OLL
ChristopherChudzicki
approved these changes
Nov 19, 2025
learning_resources/models.py
Outdated
| if upcoming_runs: | ||
| return min(upcoming_runs, key=lambda r: r.start_date) | ||
|
|
||
| # If current_run is still null, return the run with the latest start date |
Contributor
There was a problem hiding this comment.
current_run ... typo?
learning_resources/models.py
Outdated
Comment on lines
577
to
578
| # If no current enrollable run found, find the next upcoming run | ||
| upcoming_runs = [ |
Contributor
There was a problem hiding this comment.
Wouldn't OCW and OLL be included in enrollable_runs array via this filter?
not run.enrollment_start
and run.start_date
and run.start_date <= now
)
mbertrand
force-pushed
the
mb/next_start_date
branch
from
542ba39 to
7365408
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Backend part of https://github.com/mitodl/hq/issues/9206
Description (What does it do?)
best_run_idfield to learning resource serializers and search indexHow can this be tested?
./manage.py recreate_index --all./manage.py backpopulate_ocw_data --course-name 7-05-general-biochemistry-spring-2020best_run_idequal to the id of the course's one run.best_run_idvalue equal to the id of thebest_run.When this gets to RC/production, the search index will need to be recreated to include the new
best_run_idfield.