From c24828b592d8ceddec196fb573e7a457a083ebd0 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 14 Sep 2026 15:45:44 -0700 Subject: [PATCH 1/5] test: import linter config for openedx_catalog --- .importlinter | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.importlinter b/.importlinter index 17dd176f6..d2f1f6e24 100644 --- a/.importlinter +++ b/.importlinter @@ -5,6 +5,7 @@ [importlinter] root_packages = + openedx_catalog openedx_learning openedx_content openedx_tagging @@ -19,17 +20,21 @@ name = "top-level source folders are layered correctly" type = layers layers = # Learning-domain features (currently CBE; Learning Pathways to follow). - # May build on content and tagging. Nothing below may import it: in - # particular, openedx_tagging must never know that CBE exists. + # May build on catalog, content and tagging. Nothing below may import it: + # in particular, openedx_tagging must never know that CBE exists. openedx_learning + # Catalog: defines how content is organized into enrollable courses and + # pathways. May point at content; content must never import it. + openedx_catalog + # Content: authoring-side models and APIs. openedx_content # Tagging is very simple & fundamental. Should probably not depend on any other Django apps. openedx_tagging - # Django utilities. Should not dependend on any of the real apps (above). + # Django utilities. Should not depend on any of the real apps (above). openedx_django_lib # This just an empty shell package, to expose the __version__ number. From ae89eb7f45ab9ab1dea623a8c65c82ff822b2a1c Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 14 Sep 2026 16:06:45 -0700 Subject: [PATCH 2/5] docs: ADR for usage of Open edX Catalog models Co-Authored-By: Claude --- docs/index.rst | 1 + .../decisions/0001-catalog-models-usage.rst | 81 +++++++++++++++ docs/openedx_catalog/decisions/index.rst | 12 +++ docs/openedx_catalog/index.rst | 12 +++ .../0007-pathway-catalog-content-split.rst | 34 +++++-- .../images/pathway-catalog-content.dot | 6 +- .../images/pathway-catalog-content.svg | 98 +++++++++---------- src/openedx_catalog/ARCHITECTURE.md | 5 +- src/openedx_learning/README.rst | 4 +- 9 files changed, 188 insertions(+), 65 deletions(-) create mode 100644 docs/openedx_catalog/decisions/0001-catalog-models-usage.rst create mode 100644 docs/openedx_catalog/decisions/index.rst create mode 100644 docs/openedx_catalog/index.rst diff --git a/docs/index.rst b/docs/index.rst index 8ff88027d..a3eaefb6b 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -16,6 +16,7 @@ openedx_core/decisions/index openedx_content/index + openedx_catalog/index openedx_tagging/index openedx_learning/index diff --git a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst new file mode 100644 index 000000000..8843d939b --- /dev/null +++ b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst @@ -0,0 +1,81 @@ +.. _openedx-catalog-adr-0001: + +1. Role of Catalog's CourseRun and CatalogCourse Models +======================================================= + +Status +------ + +Draft + +Context +------- + +``openedx_catalog`` holds the core models that say which courses exist in an instance: :class:`CatalogCourse` (a set of runs, e.g. "Math 100") and :class:`CourseRun` (one run, e.g. "Math 100 2026Fall"). ``openedx_content`` holds the authored, versioned material itself, grouped into :class:`LearningPackage` instances. This ADR clarifies how the catalog models are meant to be used. + +Until now the direction of the relationship between the two apps has been left open ("TBD" in the catalog architecture diagram), and two proposals have pulled in opposite directions: + +- The proposed `Course Learning Packages ADR`_ gives :class:`CourseRun` a foreign key to :class:`LearningPackage`, which requires the catalog to import content. +- :ref:`openedx-learning-adr-0007` stated that ``openedx_content`` knows about ``openedx_catalog`` and never the reverse, so that Pathway Items can reference course runs directly. + +Meanwhile, ``ContentLibrary`` in ``openedx-platform`` already points at :class:`LearningPackage` from the outside, and ``openedx_learning`` is already layered above ``openedx_content`` in ``.importlinter``. + +Separately, admins can now create catalog courses and course runs before any content exists, and the rest of the system needs a clear rule about what that implies. + +Decisions +--------- + +1. The catalog layers above content +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``openedx_catalog`` may import and hold foreign keys to ``openedx_content``. But ``openedx_content`` must never import ``openedx_catalog``. + +``openedx_learning`` (Pathways, Competency-Based Education, and more) sits above both. + +The resulting order, enforced by the ``src_layering`` contract in ``.importlinter``, is:: + + openedx_learning > openedx_catalog > openedx_content > openedx_tagging + +The general rule behind this ordering is: **a context model points at its content; content never points at contexts.** ``openedx_content`` is generic infrastructure used by courses, libraries, pathways and future context types, and its applets are deliberately ignorant of what a learning package represents. A course run, a library, or a pathway is the thing that knows which package (or which container within a package) holds its content, in exactly the way ``ContentLibrary`` already does. + +This does not contradict the intent of :ref:`openedx-learning-adr-0007`, whose real requirements are that the versioned Pathway definition holds the references to the unversioned catalog objects. Those definition models live in ``openedx_learning``, above the catalog, so they can reference :class:`CourseRun` and content freely. Decision 4 of that ADR has been amended to name ``openedx_learning`` rather than ``openedx_content`` as the side that knows about the catalog. + +2. Catalog entries may be placeholders with no content +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A :class:`CatalogCourse` or :class:`CourseRun` may exist with no content behind it: as a marketing or enrollment placeholder, as a planned future run, or because its content still lives in modulestore. + +The converse guarantee does hold: if a course exists anywhere in the system, it exists as a :class:`CatalogCourse` and :class:`CourseRun` row. + +3. Catalog models are the canonical target for course foreign keys +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +For performance and correctness, any Django model in this repository or in ``openedx-platform`` that needs to reference a course should do so with a foreign key to :class:`CourseRun` (or, rarely, :class:`CatalogCourse`), rather than by storing a course key string or pointing at ``CourseOverview`` (although much existing code does not yet follow this new convention). + +On the other hand, public APIs and events should continue to identify courses by their full string course key and never expose the integer primary keys. + +4. Catalog models stay minimal, unversioned, and extended by related models +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:class:`CatalogCourse` and :class:`CourseRun` carry only identity and a title. They are not versioned, unlike content. Additional course metadata (schedules, grading policy, enrollment options, pricing) will live in dedicated models with a ``ForeignKey`` or ``OneToOneField`` to :class:`CourseRun`, in this app or in others, following the same progressive-enhancement pattern as :ref:`openedx-content-adr-0002`. Whether a given metadata model is versioned, and how, is decided per model and is out of scope here. + +Consequences +------------ + +- Models related to pathway contents will not be added to ``openedx_content`` but rather will live in ``openedx_learning``. This makes sense, as Pathway details are only useful for implementing Pathways, and are not a generic primitive like ``Component`` that is used in multiple contexts. +- Every relationship from the catalog to content is nullable. +- Code must never assume that content, a ``CourseOverview``, or any other related model exists just because a catalog row does. Content-dependent behavior must check for the relationship and degrade gracefully. +- ``openedx_content`` needs no course-, library- or pathway-aware code, and stays reusable by any context type. +- Looking up a run's content is a direct key lookup from the catalog side. Looking up which course/library/etc. a package belongs to is a reverse query (potentially checking multiple tables, e.g. both ``CourseRun`` and ``ContentLibrary``), which is acceptable because it is an uncommon use case. +- Deleting a learning package can never cascade into catalog entries, enrollments, or anything else that hangs off the catalog. +- A :class:`LearningPackage` can be created and populated without yet being associated with a course/library/etc. +- Import Linter will fail any change that makes ``openedx_content`` import ``openedx_catalog``, including a Pathways applet that references :class:`CourseRun` if it is placed inside ``openedx_content``. Such models belong in ``openedx_learning``. + +Rejected Alternatives +--------------------- + +**Peer layering with cross-references.** In this case, we'd state that in general, :class:`LearningPackage` is context agnostic, and catalog models point to :class:`LearningPackage` rather than vice versa, but *within* ``openedx_content`` a new ``PathwayItem`` model allows references to ``CourseRun``. This is probably workable, but lacks the clean separation that we're looking for. It is also a package cycle: ``openedx_catalog`` imports ``openedx_content`` for :class:`LearningPackage` while ``openedx_content`` imports ``openedx_catalog`` for :class:`CourseRun`, which a ``layers`` contract in Import Linter cannot express at all. What's more, ``PathwayItem`` is only useful for the ``pathways`` app, which is presumably optional, so it's not as generic or reusable as the other models offered by ``openedx_content``. + +**Content layers above the catalog.** In this case, ``openedx_content`` would need to hold some mechanism for mapping from :class:`LearningPackage` (or a root container) to :class:`CourseRun` (and presumably to :class:`ContentLibrary`), either hard-coding awareness of "courses", "libraries" and "pathways", or using a polymorphic context registry. This makes the generic content layer aware of one specific context type, and offers no way to treat libraries or pathways the same way without also moving them below content, which is impossible for ``ContentLibrary`` in ``openedx-platform``. + +.. _Course Learning Packages ADR: https://github.com/openedx/openedx-core/pull/812 diff --git a/docs/openedx_catalog/decisions/index.rst b/docs/openedx_catalog/decisions/index.rst new file mode 100644 index 000000000..e221b628b --- /dev/null +++ b/docs/openedx_catalog/decisions/index.rst @@ -0,0 +1,12 @@ +.. _openedx-catalog-decisions-index: + +Decisions +========= + +Architecture Decision Records for the ``openedx_catalog`` app. + +.. toctree:: + :maxdepth: 1 + :glob: + + * diff --git a/docs/openedx_catalog/index.rst b/docs/openedx_catalog/index.rst new file mode 100644 index 000000000..cc407ad0a --- /dev/null +++ b/docs/openedx_catalog/index.rst @@ -0,0 +1,12 @@ +.. _openedx-catalog-index: + +openedx_catalog +=============== + +Django app for the core catalog models (``CatalogCourse``, ``CourseRun``) that define which courses exist in an +instance, independently of whether their content exists yet. + +.. toctree:: + :maxdepth: 1 + + decisions/index diff --git a/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst b/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst index 9f9314726..73e94ed37 100644 --- a/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst +++ b/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst @@ -34,7 +34,8 @@ Decisions 1. A Pathway is split into two parts: - **Catalog Pathway** - the learner-browsable, enrollable thing. It includes the display name, the description - shown in the catalog, SEO metadata, and a **Category**. It is **not versioned**. + shown in the catalog, SEO metadata, and a **Category**. It is **not versioned**. Its definition lives in the + ``openedx_catalog`` app. - **Pathway content** - the definition of the Pathway: its Items and its completion criteria. The content is **versioned**, so that we can always tell what the definition was at any given moment. A version of the Pathway @@ -48,13 +49,18 @@ Decisions 3. In authoring contexts (Studio, Django admin, code, docs), the terminology is always "Pathway", with the Category shown explicitly. Relabelling is a learner-facing concern of the catalog side only. -4. **Dependency direction**: ``openedx_content`` knows about ``openedx_catalog``, never the reverse. This has the - following consequences: +4. **Content implementation**: The models that define pathway content are part of the ``openedx_learning`` package, + which lives above both ``openedx_content`` and ``openedx_catalog``. This has the following consequences: + - The implementation of the Pathway feature is mostly contained within ``openedx_learning`` rather than spread + among ``openedx_catalog``, ``openedx_learning``, and ``openedx_content``. + - Pathway Items/Criteria are treated as part of the Pathways feature, and are not generic content primitives + available for use in non-pathway features. + - Pathway Items can be implemented using ``PublishableEntity``, to get versioning and draft/publish. - Pathway Items may reference ``CourseRun`` entities directly. - - The link from a Catalog Pathway to the Pathway content that implements it lives on the content side. - - Anything that has to tie the two sides together belongs in ``openedx_content``, or in something downstream of - it, but never in ``openedx_catalog``. + - Pathway Items, completion criteria, etc. cannot be used nor referenced in core ``openedx_content`` models such as + ``Component`` (but could be referenced by a generic ``Container`` that allows any ``PublishableEntity`` as its + child, or a specialized ``Container`` subclass defined in ``openedx_learning``, if either were useful). 5. **Enrollment** ties a learner to a Catalog Pathway. Progress is evaluated against the currently published content version, not against a version frozen at enrollment time, so that authoring changes reach learners who @@ -68,8 +74,9 @@ Catalog Pathway Pathway content Display name Pathway Items Category Completion criteria Description References to CourseRuns -SEO metadata Link to the related Catalog Pathway +SEO metadata Enrollment +Link to the Pathway content ============================ =================================== .. Run `dot -Tsvg images/pathway-catalog-content.dot > images/pathway-catalog-content.svg` to regenerate the diagram @@ -86,5 +93,14 @@ Consequences - Because evaluation follows the published version rather than the enrollment-time version, edits to a Pathway apply to learners who are already enrolled, which is what we want, but it means edits need care and re-evaluation. - The unversioned Catalog Pathway can be long-lived even if its content definition is changed significantly over time. -- The dependency direction means a Catalog Pathway cannot, on its own, tell which content implements it. Queries in - that direction start from the content side. + +Changelog +--------- + +2026-09-14: + +* Changed layering so that ``openedx_catalog`` depends on ``openedx_content``, not vice versa. + +2026-09-01: + +* Initial version diff --git a/docs/openedx_learning/decisions/images/pathway-catalog-content.dot b/docs/openedx_learning/decisions/images/pathway-catalog-content.dot index 10bafb89f..a478099cf 100644 --- a/docs/openedx_learning/decisions/images/pathway-catalog-content.dot +++ b/docs/openedx_learning/decisions/images/pathway-catalog-content.dot @@ -16,7 +16,7 @@ digraph pathway_catalog_content { } subgraph cluster_content { - label="openedx_content (versioned)"; + label="openedx_learning (versioned)"; fontsize=10; fontcolor="#4d4d4d"; style=dashed; @@ -33,8 +33,8 @@ digraph pathway_catalog_content { enrollment -> catalog [label="enrolled in"]; enrollment -> v2 [label="progress evaluated\nagainst the currently\npublished version", style=dashed, color="#808080", fontcolor="#4d4d4d"]; - v2 -> catalog [label="implements"]; + v2 -> catalog [label="content", dir=back]; // arrowhead at v2: the Catalog Pathway points at its content v2 -> courserun [label="Items reference"]; - direction [label="dependency direction:\nopenedx_content knows about\nopenedx_catalog, never the reverse", shape=plaintext, fontsize=10, fontcolor="#4d4d4d"]; + direction [label="dependency direction:\nopenedx_learning knows about\nopenedx_catalog, never the reverse", shape=plaintext, fontsize=10, fontcolor="#4d4d4d"]; } diff --git a/docs/openedx_learning/decisions/images/pathway-catalog-content.svg b/docs/openedx_learning/decisions/images/pathway-catalog-content.svg index df9a619a9..0e8811c98 100644 --- a/docs/openedx_learning/decisions/images/pathway-catalog-content.svg +++ b/docs/openedx_learning/decisions/images/pathway-catalog-content.svg @@ -1,115 +1,115 @@ - - - + + pathway_catalog_content - + cluster_catalog - -openedx_catalog (not versioned) + +openedx_catalog (not versioned) cluster_content - -openedx_content (versioned) + +openedx_learning (versioned) catalog - -Catalog Pathway -name, category, -description, SEO + +Catalog Pathway +name, category, +description, SEO courserun - -CourseRun + +CourseRun v1 - -content v1 -Items, criteria + +content v1 +Items, criteria v2 - -content v2 -Items, criteria -(currently published) + +content v2 +Items, criteria +(currently published) v1->v2 - - -revision + + +revision v2->catalog - - -implements + + +content v2->courserun - - -Items reference + + +Items reference user - -User + +User enrollment - -Enrollment + +Enrollment user->enrollment - - -learner + + +learner enrollment->catalog - - -enrolled in + + +enrolled in enrollment->v2 - - -progress evaluated -against the currently -published version + + +progress evaluated +against the currently +published version direction -dependency direction: -openedx_content knows about -openedx_catalog, never the reverse +dependency direction: +openedx_learning knows about +openedx_catalog, never the reverse diff --git a/src/openedx_catalog/ARCHITECTURE.md b/src/openedx_catalog/ARCHITECTURE.md index 6feda9546..5c3ed157f 100644 --- a/src/openedx_catalog/ARCHITECTURE.md +++ b/src/openedx_catalog/ARCHITECTURE.md @@ -15,7 +15,7 @@ flowchart TB Organizations["**edx-organizations** (Organization)"] Enrollments["**platform: enrollments** (CourseEnrollment, CourseEnrollmentAllowed)"] Modes["**platform: course_modes** (CourseMode)"] - Catalog <-. "Direction of this relationship TBD." .-> Content + Catalog -- "References (nullable; see ADR 0001)" --> Content Catalog -- References --> Organizations Enrollments -- References --> Modes Enrollments -- References --> Catalog @@ -24,8 +24,9 @@ flowchart TB style Modes fill:#ccc style Organizations fill:#ccc - Pathways["**openedx_pathways** (Pathway, PathwaySchedule, PathwayEnrollment, PathwayCertificate, etc.)"] + Pathways["**openedx_learning: pathways** (Pathway definition, PathwayItem, PathwayEnrollment, PathwayCertificate, etc. The unversioned CatalogPathway lives in openedx_catalog.)"] Pathways -- References --> Catalog + Pathways -- References --> Content style Pathways fill:#c0ffee,stroke-dasharray: 5 5 diff --git a/src/openedx_learning/README.rst b/src/openedx_learning/README.rst index 33c11ea54..ecceb6b2b 100644 --- a/src/openedx_learning/README.rst +++ b/src/openedx_learning/README.rst @@ -7,5 +7,5 @@ and how they get there. Its sibling ``openedx_content`` holds the material itsel Like ``openedx_content``, it is one Django app split into applets. Its first applet is ``cbe``, for Competency-Based Education; Learning Pathways are expected to follow. -In the layering that ``.importlinter`` enforces, this app sits above ``openedx_content`` -and ``openedx_tagging``. It may build on either of them; neither may import it. +In the layering that ``.importlinter`` enforces, this app sits above ``openedx_catalog``, +``openedx_content`` and ``openedx_tagging``. It may build on any of them; none may import it. From b1752dce2d305f37e0eb5c87c96a0406eaf14c69 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 14 Sep 2026 16:58:37 -0700 Subject: [PATCH 3/5] docs: mention possibility of sharing a LearningPackage --- docs/openedx_catalog/decisions/0001-catalog-models-usage.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst index 8843d939b..4b90cc00f 100644 --- a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst +++ b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst @@ -71,6 +71,11 @@ Consequences - A :class:`LearningPackage` can be created and populated without yet being associated with a course/library/etc. - Import Linter will fail any change that makes ``openedx_content`` import ``openedx_catalog``, including a Pathways applet that references :class:`CourseRun` if it is placed inside ``openedx_content``. Such models belong in ``openedx_learning``. +Sharing a LearningPackage +------------------------- + +This ADR deliberately does not specify exactly how a catalog :class:`CourseRun` maps to a :class:`LearningPackage`. The simplest option is a foreign key from one to the other, which is the tentative plan specified in the proposed `Course Learning Packages ADR`_. However, in the future it may evolve to become e.g. a foreign key from :class:`CourseRun` to a ``CourseRoot`` or ``OutlineRoot`` object *within* a :class:`LearningPackage`; this would allow multiple course runs and even pathways to store their content in a large, combined :class:`LearningPackage`. + Rejected Alternatives --------------------- From 6cda67748dda91bbcc60b0c51c855c24d9e9ee2d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 17 Sep 2026 14:27:52 -0700 Subject: [PATCH 4/5] docs: invert the direction --- .importlinter | 14 +++-- .../decisions/0001-catalog-models-usage.rst | 63 +++++++++---------- .../0007-pathway-catalog-content-split.rst | 33 +++------- .../images/pathway-catalog-content.dot | 2 +- .../images/pathway-catalog-content.svg | 6 +- src/openedx_catalog/ARCHITECTURE.md | 2 +- src/openedx_catalog/README.rst | 2 +- src/openedx_catalog/models/course_run.py | 21 +++---- 8 files changed, 61 insertions(+), 82 deletions(-) diff --git a/.importlinter b/.importlinter index d2f1f6e24..98997d1a2 100644 --- a/.importlinter +++ b/.importlinter @@ -20,17 +20,19 @@ name = "top-level source folders are layered correctly" type = layers layers = # Learning-domain features (currently CBE; Learning Pathways to follow). - # May build on catalog, content and tagging. Nothing below may import it: + # May build on content, catalog and tagging. Nothing below may import it: # in particular, openedx_tagging must never know that CBE exists. openedx_learning - # Catalog: defines how content is organized into enrollable courses and - # pathways. May point at content; content must never import it. - openedx_catalog - - # Content: authoring-side models and APIs. + # Content: authoring-side models and APIs. May reference catalog models + # (e.g. to associate content with a CourseRun). openedx_content + # Catalog: the enrollable things (course runs, catalog courses, pathways). + # Not aware of content; must never import openedx_content. See + # docs/openedx_catalog/decisions/0001-catalog-models-usage.rst + openedx_catalog + # Tagging is very simple & fundamental. Should probably not depend on any other Django apps. openedx_tagging diff --git a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst index 4b90cc00f..7eae69612 100644 --- a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst +++ b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst @@ -1,7 +1,7 @@ .. _openedx-catalog-adr-0001: -1. Role of Catalog's CourseRun and CatalogCourse Models -======================================================= +1. Role of Catalog +================== Status ------ @@ -11,41 +11,44 @@ Draft Context ------- -``openedx_catalog`` holds the core models that say which courses exist in an instance: :class:`CatalogCourse` (a set of runs, e.g. "Math 100") and :class:`CourseRun` (one run, e.g. "Math 100 2026Fall"). ``openedx_content`` holds the authored, versioned material itself, grouped into :class:`LearningPackage` instances. This ADR clarifies how the catalog models are meant to be used. +``openedx_catalog`` holds the core models for tracking enrollable things (and eventually, enrollments as well). -Until now the direction of the relationship between the two apps has been left open ("TBD" in the catalog architecture diagram), and two proposals have pulled in opposite directions: +Specifically, its main models are: -- The proposed `Course Learning Packages ADR`_ gives :class:`CourseRun` a foreign key to :class:`LearningPackage`, which requires the catalog to import content. -- :ref:`openedx-learning-adr-0007` stated that ``openedx_content`` knows about ``openedx_catalog`` and never the reverse, so that Pathway Items can reference course runs directly. +- :class:`CourseRun` (one course run, e.g. "Math 100 2026Fall"). +- :class:`CatalogCourse` (a set of course runs, e.g. "Math 100") +- :class:`CatalogPathway` (a pathway that learners can enroll in) +- :class:`PathwayCategory` (learner-facing label for pathway types, e.g. "Masters Degree") +- :class:`PathwayEnrollment` (tracks enrollment into pathways) -Meanwhile, ``ContentLibrary`` in ``openedx-platform`` already points at :class:`LearningPackage` from the outside, and ``openedx_learning`` is already layered above ``openedx_content`` in ``.importlinter``. +This ADR clarifies how the catalog models are meant to be used. -Separately, admins can now create catalog courses and course runs before any content exists, and the rest of the system needs a clear rule about what that implies. +``openedx_content`` holds the authored, versioned material itself, grouped into :class:`LearningPackage` instances. Until now the direction of the relationship between the two apps has been left open. Decisions --------- -1. The catalog layers above content -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +1. Catalog entries may be placeholders with no content +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -``openedx_catalog`` may import and hold foreign keys to ``openedx_content``. But ``openedx_content`` must never import ``openedx_catalog``. +A :class:`CatalogCourse` or :class:`CourseRun` may exist with no content behind it: as a marketing or enrollment placeholder, as a planned future run, or because its content still lives in modulestore. -``openedx_learning`` (Pathways, Competency-Based Education, and more) sits above both. +The converse guarantee does hold: if a course exists anywhere in the system, it exists as a :class:`CatalogCourse` and :class:`CourseRun` row. -The resulting order, enforced by the ``src_layering`` contract in ``.importlinter``, is:: +2. The catalog app is not aware of content +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - openedx_learning > openedx_catalog > openedx_content > openedx_tagging +``openedx_content`` will have a table(s) for tracking the relationship between a :class:`CourseRun` and its content. But the catalog app itself is not aware of content, and does not maintain any relationship between enrollable things (course runs, pathways) and their content. -The general rule behind this ordering is: **a context model points at its content; content never points at contexts.** ``openedx_content`` is generic infrastructure used by courses, libraries, pathways and future context types, and its applets are deliberately ignorant of what a learning package represents. A course run, a library, or a pathway is the thing that knows which package (or which container within a package) holds its content, in exactly the way ``ContentLibrary`` already does. +``openedx_content`` may import and hold foreign keys to ``openedx_catalog``. But ``openedx_catalog`` must never import ``openedx_content``. -This does not contradict the intent of :ref:`openedx-learning-adr-0007`, whose real requirements are that the versioned Pathway definition holds the references to the unversioned catalog objects. Those definition models live in ``openedx_learning``, above the catalog, so they can reference :class:`CourseRun` and content freely. Decision 4 of that ADR has been amended to name ``openedx_learning`` rather than ``openedx_content`` as the side that knows about the catalog. +``openedx_learning`` (Pathways, Competency-Based Education, and more) and other parts of the platform sit above both. -2. Catalog entries may be placeholders with no content -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The resulting order, enforced by the ``src_layering`` contract in ``.importlinter``, is:: -A :class:`CatalogCourse` or :class:`CourseRun` may exist with no content behind it: as a marketing or enrollment placeholder, as a planned future run, or because its content still lives in modulestore. + openedx_learning > openedx_content > openedx_catalog > openedx_tagging -The converse guarantee does hold: if a course exists anywhere in the system, it exists as a :class:`CatalogCourse` and :class:`CourseRun` row. +The general principle behind this is that changes in how content is represented should not require changes to the catalog app. For example, if we were to change from associating each course run with a :class:`LearningPackage` to associating each course run with an ``OutlineRoot`` in a :class:`LearningPackage` that contains multiple runs, that should not require changes to the catalog app, which would be the case if we used foreign keys from :class:`CourseRun` to :class:`LearningPackage` within the catalog app. 3. Catalog models are the canonical target for course foreign keys ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -57,30 +60,20 @@ On the other hand, public APIs and events should continue to identify courses by 4. Catalog models stay minimal, unversioned, and extended by related models ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -:class:`CatalogCourse` and :class:`CourseRun` carry only identity and a title. They are not versioned, unlike content. Additional course metadata (schedules, grading policy, enrollment options, pricing) will live in dedicated models with a ``ForeignKey`` or ``OneToOneField`` to :class:`CourseRun`, in this app or in others, following the same progressive-enhancement pattern as :ref:`openedx-content-adr-0002`. Whether a given metadata model is versioned, and how, is decided per model and is out of scope here. +Catalog's models like :class:`CatalogCourse` and :class:`CourseRun` carry only identity and a title. They are not versioned, unlike content. Additional course metadata (schedules, grading policy, enrollment options, pricing) will live in dedicated models with a ``ForeignKey`` or ``OneToOneField`` to :class:`CourseRun`, in this app or in others, following the same progressive-enhancement pattern as :ref:`openedx-content-adr-0002`. Whether a given metadata model is versioned, and how, is decided per model and is out of scope here. Consequences ------------ -- Models related to pathway contents will not be added to ``openedx_content`` but rather will live in ``openedx_learning``. This makes sense, as Pathway details are only useful for implementing Pathways, and are not a generic primitive like ``Component`` that is used in multiple contexts. -- Every relationship from the catalog to content is nullable. +- ``openedx_content`` will need a table(s) that associates course content with catalog's course runs, and which ensures that no more than one content outline can be associated with the same :class:`CourseRun`. - Code must never assume that content, a ``CourseOverview``, or any other related model exists just because a catalog row does. Content-dependent behavior must check for the relationship and degrade gracefully. -- ``openedx_content`` needs no course-, library- or pathway-aware code, and stays reusable by any context type. -- Looking up a run's content is a direct key lookup from the catalog side. Looking up which course/library/etc. a package belongs to is a reverse query (potentially checking multiple tables, e.g. both ``CourseRun`` and ``ContentLibrary``), which is acceptable because it is an uncommon use case. - Deleting a learning package can never cascade into catalog entries, enrollments, or anything else that hangs off the catalog. -- A :class:`LearningPackage` can be created and populated without yet being associated with a course/library/etc. -- Import Linter will fail any change that makes ``openedx_content`` import ``openedx_catalog``, including a Pathways applet that references :class:`CourseRun` if it is placed inside ``openedx_content``. Such models belong in ``openedx_learning``. - -Sharing a LearningPackage -------------------------- - -This ADR deliberately does not specify exactly how a catalog :class:`CourseRun` maps to a :class:`LearningPackage`. The simplest option is a foreign key from one to the other, which is the tentative plan specified in the proposed `Course Learning Packages ADR`_. However, in the future it may evolve to become e.g. a foreign key from :class:`CourseRun` to a ``CourseRoot`` or ``OutlineRoot`` object *within* a :class:`LearningPackage`; this would allow multiple course runs and even pathways to store their content in a large, combined :class:`LearningPackage`. +- A :class:`LearningPackage` can still be created and populated without yet being associated with a course/library/etc. +- Import Linter will fail any change that makes ``openedx_catalog`` import ``openedx_content``. Rejected Alternatives --------------------- **Peer layering with cross-references.** In this case, we'd state that in general, :class:`LearningPackage` is context agnostic, and catalog models point to :class:`LearningPackage` rather than vice versa, but *within* ``openedx_content`` a new ``PathwayItem`` model allows references to ``CourseRun``. This is probably workable, but lacks the clean separation that we're looking for. It is also a package cycle: ``openedx_catalog`` imports ``openedx_content`` for :class:`LearningPackage` while ``openedx_content`` imports ``openedx_catalog`` for :class:`CourseRun`, which a ``layers`` contract in Import Linter cannot express at all. What's more, ``PathwayItem`` is only useful for the ``pathways`` app, which is presumably optional, so it's not as generic or reusable as the other models offered by ``openedx_content``. -**Content layers above the catalog.** In this case, ``openedx_content`` would need to hold some mechanism for mapping from :class:`LearningPackage` (or a root container) to :class:`CourseRun` (and presumably to :class:`ContentLibrary`), either hard-coding awareness of "courses", "libraries" and "pathways", or using a polymorphic context registry. This makes the generic content layer aware of one specific context type, and offers no way to treat libraries or pathways the same way without also moving them below content, which is impossible for ``ContentLibrary`` in ``openedx-platform``. - -.. _Course Learning Packages ADR: https://github.com/openedx/openedx-core/pull/812 +**Catalog layers above the content.** In this case, ``openedx_catalog`` would hold a foreign key from :class:`CourseRun` to :class:`LearningPackage`, but any refactors to how content is stored (e.g. relationship to ``OutlineRoot`` instead of ``LearningPackage``) would require changing this foreign key, which shouldn't be the case. diff --git a/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst b/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst index 73e94ed37..ecb9966a0 100644 --- a/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst +++ b/docs/openedx_learning/decisions/0007-pathway-catalog-content-split.rst @@ -49,18 +49,13 @@ Decisions 3. In authoring contexts (Studio, Django admin, code, docs), the terminology is always "Pathway", with the Category shown explicitly. Relabelling is a learner-facing concern of the catalog side only. -4. **Content implementation**: The models that define pathway content are part of the ``openedx_learning`` package, - which lives above both ``openedx_content`` and ``openedx_catalog``. This has the following consequences: - - - The implementation of the Pathway feature is mostly contained within ``openedx_learning`` rather than spread - among ``openedx_catalog``, ``openedx_learning``, and ``openedx_content``. - - Pathway Items/Criteria are treated as part of the Pathways feature, and are not generic content primitives - available for use in non-pathway features. - - Pathway Items can be implemented using ``PublishableEntity``, to get versioning and draft/publish. +4. **Dependency direction**: ``openedx_content`` knows about ``openedx_catalog``, never the reverse. This has the + following consequences: + - Pathway Items may reference ``CourseRun`` entities directly. - - Pathway Items, completion criteria, etc. cannot be used nor referenced in core ``openedx_content`` models such as - ``Component`` (but could be referenced by a generic ``Container`` that allows any ``PublishableEntity`` as its - child, or a specialized ``Container`` subclass defined in ``openedx_learning``, if either were useful). + - The link from a Catalog Pathway to the Pathway content that implements it lives on the content side. + - Anything that has to tie the two sides together belongs in ``openedx_content``, or in something downstream of + it, but never in ``openedx_catalog``. 5. **Enrollment** ties a learner to a Catalog Pathway. Progress is evaluated against the currently published content version, not against a version frozen at enrollment time, so that authoring changes reach learners who @@ -74,9 +69,8 @@ Catalog Pathway Pathway content Display name Pathway Items Category Completion criteria Description References to CourseRuns -SEO metadata +SEO metadata Link to the related Catalog Pathway Enrollment -Link to the Pathway content ============================ =================================== .. Run `dot -Tsvg images/pathway-catalog-content.dot > images/pathway-catalog-content.svg` to regenerate the diagram @@ -93,14 +87,5 @@ Consequences - Because evaluation follows the published version rather than the enrollment-time version, edits to a Pathway apply to learners who are already enrolled, which is what we want, but it means edits need care and re-evaluation. - The unversioned Catalog Pathway can be long-lived even if its content definition is changed significantly over time. - -Changelog ---------- - -2026-09-14: - -* Changed layering so that ``openedx_catalog`` depends on ``openedx_content``, not vice versa. - -2026-09-01: - -* Initial version +- The dependency direction means a Catalog Pathway cannot, on its own, tell which content implements it. Queries in + that direction start from the content side. diff --git a/docs/openedx_learning/decisions/images/pathway-catalog-content.dot b/docs/openedx_learning/decisions/images/pathway-catalog-content.dot index a478099cf..e560c5bef 100644 --- a/docs/openedx_learning/decisions/images/pathway-catalog-content.dot +++ b/docs/openedx_learning/decisions/images/pathway-catalog-content.dot @@ -33,7 +33,7 @@ digraph pathway_catalog_content { enrollment -> catalog [label="enrolled in"]; enrollment -> v2 [label="progress evaluated\nagainst the currently\npublished version", style=dashed, color="#808080", fontcolor="#4d4d4d"]; - v2 -> catalog [label="content", dir=back]; // arrowhead at v2: the Catalog Pathway points at its content + v2 -> catalog [label="implements"]; v2 -> courserun [label="Items reference"]; direction [label="dependency direction:\nopenedx_learning knows about\nopenedx_catalog, never the reverse", shape=plaintext, fontsize=10, fontcolor="#4d4d4d"]; diff --git a/docs/openedx_learning/decisions/images/pathway-catalog-content.svg b/docs/openedx_learning/decisions/images/pathway-catalog-content.svg index 0e8811c98..9f499e2c3 100644 --- a/docs/openedx_learning/decisions/images/pathway-catalog-content.svg +++ b/docs/openedx_learning/decisions/images/pathway-catalog-content.svg @@ -58,9 +58,9 @@ v2->catalog - - -content + + +implements diff --git a/src/openedx_catalog/ARCHITECTURE.md b/src/openedx_catalog/ARCHITECTURE.md index 5c3ed157f..60c768f6c 100644 --- a/src/openedx_catalog/ARCHITECTURE.md +++ b/src/openedx_catalog/ARCHITECTURE.md @@ -15,7 +15,7 @@ flowchart TB Organizations["**edx-organizations** (Organization)"] Enrollments["**platform: enrollments** (CourseEnrollment, CourseEnrollmentAllowed)"] Modes["**platform: course_modes** (CourseMode)"] - Catalog -- "References (nullable; see ADR 0001)" --> Content + Content -- "References (see ADR 0001)" --> Catalog Catalog -- References --> Organizations Enrollments -- References --> Modes Enrollments -- References --> Catalog diff --git a/src/openedx_catalog/README.rst b/src/openedx_catalog/README.rst index e47e32eab..5729928db 100644 --- a/src/openedx_catalog/README.rst +++ b/src/openedx_catalog/README.rst @@ -14,7 +14,7 @@ The existing ``CourseOverview`` model in ``openedx-platform`` is derived from va 1. Provide a core model to represent each course, for foreign key purposes. 2. To allow provisioning placeholder courses before any content even exists. 3. To be much simpler and more performant than ``CourseOverview`` was (far fewer fields generally, fewer legacy fields, integer primary key). -4. Perhaps to provide a transition mechanism, a pointer than can point either to modulestore or openedx_content, as we transition content storage. +4. To be independent of how and where content is stored: the catalog is not aware of content, and the mapping from a course run to its content (in modulestore or in ``openedx_content``) is maintained outside this app. See `ADR 0001 <../../docs/openedx_catalog/decisions/0001-catalog-models-usage.rst>`__. Architecture ------------ diff --git a/src/openedx_catalog/models/course_run.py b/src/openedx_catalog/models/course_run.py index cf8a34ba5..64337301d 100644 --- a/src/openedx_catalog/models/course_run.py +++ b/src/openedx_catalog/models/course_run.py @@ -75,9 +75,10 @@ class CourseRun(models.Model): this catalog app or other apps. They should either be versioned using `PublishableEntity` or use the `HistoricalRecords()` history from `django-simple-history` to preserve a record of all changes. - - In the future, there will be a relationship to Learning Package. Several - course runs from the same catalog course may be stored in the same - learning package. + - This app is deliberately not aware of content. The mapping from a course + run to its content (a Learning Package and/or an OutlineRoot container in + `openedx_content`) is maintained on the `openedx_content` side, never as a + field on this model. See docs/openedx_catalog/decisions/0001. """ CourseRunID = NewType("CourseRunID", int) @@ -168,14 +169,12 @@ def course_code(self) -> str: # Do we want mix in SoftDeletableModel from django-model-utils to make courses soft deletable? - # In the future, either this model or CatalogCourse will have: - # learning_package = models.ForeignKey(LearningPackage) - - # In the future, this model will likely have a relationship to the - # OutlineRoot which would be an `openedx_content` `Container` instance that - # holds the conten tree (Sections, Subsections, Units, etc.). For now, if - # the content exists, it will be in modulestore instead (you can get the - # `SplitModulestoreCourseIndex` using TODO: define API method). + # 🛑 Do not add a relationship to LearningPackage, OutlineRoot, or any other + # `openedx_content` model here. The catalog app is not aware of content; + # `openedx_content` holds the table that maps a CourseRun to its content + # (see docs/openedx_catalog/decisions/0001). For now, if the content + # exists, it will be in modulestore instead (you can get the + # `SplitModulestoreCourseIndex` using TODO: define API method). def clean(self): """Defaults and validation of model fields""" From 831128d12bb609ea847382dfd1a28633aacb111e Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 18 Sep 2026 12:18:06 -0700 Subject: [PATCH 5/5] docs: mention possibility of a join app layered above --- docs/openedx_catalog/decisions/0001-catalog-models-usage.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst index 7eae69612..3fa6ba934 100644 --- a/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst +++ b/docs/openedx_catalog/decisions/0001-catalog-models-usage.rst @@ -74,6 +74,8 @@ Consequences Rejected Alternatives --------------------- +**Another app joins content and catalog.** In this case, ``catalog`` and ``content`` would be wholly independent, prohibited from referencing each other. Another app, like ``openedx_learning``, ``openedx_courses``, or ``cms.contentstore`` would be layered on top and hold the records that associate each catalog with each course. This is a perfectly viable option, and is currently how content libraries are implemented. However, for now it seems simpler and more useful to put the mapping into the ``content`` app directly. + **Peer layering with cross-references.** In this case, we'd state that in general, :class:`LearningPackage` is context agnostic, and catalog models point to :class:`LearningPackage` rather than vice versa, but *within* ``openedx_content`` a new ``PathwayItem`` model allows references to ``CourseRun``. This is probably workable, but lacks the clean separation that we're looking for. It is also a package cycle: ``openedx_catalog`` imports ``openedx_content`` for :class:`LearningPackage` while ``openedx_content`` imports ``openedx_catalog`` for :class:`CourseRun`, which a ``layers`` contract in Import Linter cannot express at all. What's more, ``PathwayItem`` is only useful for the ``pathways`` app, which is presumably optional, so it's not as generic or reusable as the other models offered by ``openedx_content``. **Catalog layers above the content.** In this case, ``openedx_catalog`` would hold a foreign key from :class:`CourseRun` to :class:`LearningPackage`, but any refactors to how content is stored (e.g. relationship to ``OutlineRoot`` instead of ``LearningPackage``) would require changing this foreign key, which shouldn't be the case.