ADR for openedx_catalog models (layering openedx_content above catalog) [FC-0138] - #818
bradenmacdonald wants to merge 5 commits into
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
|
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. |
|
@ormsbee @kdmccormick @ChrisChV and especially @Agrendalath, please let me know your thoughts on this. |
|
thanks for writing this up. "Catalog layers over content" has always felt intuitive to me as well. that said, applying this same direction to Pathways means that:
it feels weird that CatalogCourses and CourseRuns would be in openedx_catalog, but CatalogPathways will be in openedx_learning. whereas if we flip the relationship so that openedx_catalog is at the bottom, then everything can point "down" to openedx_catalog. but there are other questions to address if we flip the relationship, so I am not sure yet. a question that's been on my mind that might guide us a bit: where do you think the LearningContext and Usage tables would go? |
| style Organizations fill:#ccc | ||
|
|
||
| Pathways["<a href='https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5148147732/Brief+Modular+Content+Delivery+-+Platform+Strategy'>**openedx_pathways**</a> (Pathway, PathwaySchedule, PathwayEnrollment, PathwayCertificate, etc.)"] | ||
| Pathways["<a href='https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5148147732/Brief+Modular+Content+Delivery+-+Platform+Strategy'>**openedx_learning: pathways**</a> (Pathway definition, PathwayItem, PathwayEnrollment, PathwayCertificate, etc. The unversioned CatalogPathway lives in openedx_catalog.)"] |
There was a problem hiding this comment.
Nit: I believe PathwayEnrollment should be tied to the CatalogPathway.
There was a problem hiding this comment.
I've mention this below, but I've been unclear on where enrollments will live.
|
@bradenmacdonald, I like this idea! I already implemented these changes in #819.
We could point the content to the |
In some ways, it could also make sense, if we think of I think we've always planned for Pathways to be a first-class thing within the catalog, and I'm also completely fine with that approach. To make it work, we can use the "FK to PublishableEntity" approach @Agrendalath is suggesting in #819 or create a join table in
We haven't yet had any need for a concrete Usage tables are a mapping of IDs to content, and so they feel like part of the content to me. I'd put them in Related to that, @Agrendalath's PR #819 reminds me that we have to decide where the enrollment tables should go (for now just pathways, but long-term we may move course enrollment as well). Since |
c62eb50 to
6cda677
Compare
|
OK everyone, based on the discussion at the Core Arch Working Group this week, we've found @ormsbee's argument pretty compelling, and I've inverted the proposal here. It's now consistent with the existing pathways ADR, and says content layers above catalog. |
|
Looks good to me 👍 |
Based on the discussion in #812, we need to clarify whether the new catalog app depends on the content app, or vice versa.
See the new ADR itself for all the context: 1. Role of Catalog’s CourseRun and CatalogCourse Models
This PR also revises a Pathways ADR, 7. Pathways: Split Between Catalog and ContentPrivate ref: FAL-4383