-
Notifications
You must be signed in to change notification settings - Fork 18
Prototype models for Courses and Outline Roots in Learning Core #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for the pull request, @bradenmacdonald! This repository is currently maintained by 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 approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere 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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
560d867 to
cf4a19f
Compare
| class CatalogCourse(models.Model): | ||
| """ | ||
| A catalog course is a collection of course runs. | ||
|
|
||
| So for example, "Stanford Python 101" is a catalog course, and "Stanford | ||
| Python 101 Spring 2025" is a course run of that course. Almost all | ||
| interesting use cases are based around the course run - e.g. enrollment | ||
| happens in a course run, content is authored in a course run, etc. But | ||
| sometimes we need to deal with the related runs of the same course, so this | ||
| model exists for those few times we need a reference to all of them. | ||
|
|
||
| A CatalogCourse is not part of a particular learning package, because | ||
| although we encourage each course's runs to be in the same learning package, | ||
| that's neither a requirement nor always possible. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ormsbee @kdmccormick So do you think I should remove the CatalogCourse model because we don't want it in the authoring domain, and just put org/course_id into the course run model?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My gut says to remove CatalogCourse. Does that make the prototype any more difficult?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm, I don't think it's any more difficult either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these look great as a starting place for the prototype. Thanks!
Are you looking for review for a merge to main? If so, I'd want to open a discussion about whether Course (run) even belongs here. My sense is that we would need to step back and decide where LearningContext goes, and then define Course with that in mind. I'm not even sure authoring is the right app.
But we can table that discussion for later if these are just for the prototype.
|
@kdmccormick No, I was just going to merge these into the lc-course-prototype branch, but I see you've already incorporated them so I can actually just close this for now unless you want to keep discussing it here. I'm kinda leaning toward keeping both |
|
Nope, no preference! |
|
@bradenmacdonald - hi there! Is this still in progress? |
|
@mphilbrick211 We're still deciding how to move forward on this. I'll close it for now, and we can keep it as a reference. |
Sketch of a possible OutlineRoot model, for https://openedx.atlassian.net/wiki/spaces/AC/pages/4612685830/Migrating+Courses+to+Learning+Core
WIP
openedx-learning/openedx_learning/apps/authoring/outline_roots/models.py
Lines 24 to 29 in 7dfcc1e