From af3249916beadd62639af67fdaff12130861c4f7 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Fri, 7 Aug 2026 10:38:29 -0400 Subject: [PATCH 1/4] feat: provide openedx_learning djangoapp and CompetencyTaxonomy model --- .importlinter | 9 ++- projects/dev.py | 1 + src/openedx_learning/__init__.py | 5 ++ src/openedx_learning/admin.py | 6 ++ src/openedx_learning/api.py | 6 ++ src/openedx_learning/applets/__init__.py | 3 + src/openedx_learning/applets/cbe/__init__.py | 3 + src/openedx_learning/applets/cbe/admin.py | 17 +++++ src/openedx_learning/applets/cbe/api.py | 38 ++++++++++ src/openedx_learning/applets/cbe/models.py | 52 ++++++++++++++ src/openedx_learning/apps.py | 15 ++++ .../migrations/0001_initial.py | 28 ++++++++ src/openedx_learning/migrations/__init__.py | 0 src/openedx_learning/models.py | 7 ++ src/openedx_learning/models_api.py | 9 +++ src/openedx_learning/py.typed | 0 test_settings.py | 1 + tests/openedx_learning/__init__.py | 0 tests/openedx_learning/applets/__init__.py | 0 .../openedx_learning/applets/cbe/__init__.py | 0 .../openedx_learning/applets/cbe/test_api.py | 50 ++++++++++++++ .../applets/cbe/test_models.py | 69 +++++++++++++++++++ 22 files changed, 317 insertions(+), 2 deletions(-) create mode 100644 src/openedx_learning/__init__.py create mode 100644 src/openedx_learning/admin.py create mode 100644 src/openedx_learning/api.py create mode 100644 src/openedx_learning/applets/__init__.py create mode 100644 src/openedx_learning/applets/cbe/__init__.py create mode 100644 src/openedx_learning/applets/cbe/admin.py create mode 100644 src/openedx_learning/applets/cbe/api.py create mode 100644 src/openedx_learning/applets/cbe/models.py create mode 100644 src/openedx_learning/apps.py create mode 100644 src/openedx_learning/migrations/0001_initial.py create mode 100644 src/openedx_learning/migrations/__init__.py create mode 100644 src/openedx_learning/models.py create mode 100644 src/openedx_learning/models_api.py create mode 100644 src/openedx_learning/py.typed create mode 100644 tests/openedx_learning/__init__.py create mode 100644 tests/openedx_learning/applets/__init__.py create mode 100644 tests/openedx_learning/applets/cbe/__init__.py create mode 100644 tests/openedx_learning/applets/cbe/test_api.py create mode 100644 tests/openedx_learning/applets/cbe/test_models.py diff --git a/.importlinter b/.importlinter index 5b90864cd..17dd176f6 100644 --- a/.importlinter +++ b/.importlinter @@ -5,6 +5,7 @@ [importlinter] root_packages = + openedx_learning openedx_content openedx_tagging openedx_django_lib @@ -17,8 +18,12 @@ root_packages = name = "top-level source folders are layered correctly" type = layers layers = - # Content is currently the highest-level thing in this repo. - # Over time, we may add apps "above" or "below" this. + # 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. + openedx_learning + + # Content: authoring-side models and APIs. openedx_content # Tagging is very simple & fundamental. Should probably not depend on any other Django apps. diff --git a/projects/dev.py b/projects/dev.py index 277a78598..28348acab 100644 --- a/projects/dev.py +++ b/projects/dev.py @@ -39,6 +39,7 @@ # Our Apps "openedx_catalog", + "openedx_learning", "openedx_tagging", "openedx_content", *openedx_content_backcompat_apps_to_install(), diff --git a/src/openedx_learning/__init__.py b/src/openedx_learning/__init__.py new file mode 100644 index 000000000..341b39932 --- /dev/null +++ b/src/openedx_learning/__init__.py @@ -0,0 +1,5 @@ +""" +Learning-domain features for Open edX Core. + +Currently one applet, cbe, holding the Competency-Based Education models. +""" diff --git a/src/openedx_learning/admin.py b/src/openedx_learning/admin.py new file mode 100644 index 000000000..e065c4048 --- /dev/null +++ b/src/openedx_learning/admin.py @@ -0,0 +1,6 @@ +""" +This module aggregates all applet Django Admin modules. +""" +# pylint: disable=wildcard-import + +from .applets.cbe.admin import * diff --git a/src/openedx_learning/api.py b/src/openedx_learning/api.py new file mode 100644 index 000000000..789a315f3 --- /dev/null +++ b/src/openedx_learning/api.py @@ -0,0 +1,6 @@ +""" +This is the public API for learning-domain features in Open edX Core. +""" +# This wildcard import is okay because the applet api module declares __all__. +# pylint: disable=wildcard-import +from .applets.cbe.api import * diff --git a/src/openedx_learning/applets/__init__.py b/src/openedx_learning/applets/__init__.py new file mode 100644 index 000000000..1879029d5 --- /dev/null +++ b/src/openedx_learning/applets/__init__.py @@ -0,0 +1,3 @@ +""" +The applets that make up the openedx_learning Django app. +""" diff --git a/src/openedx_learning/applets/cbe/__init__.py b/src/openedx_learning/applets/cbe/__init__.py new file mode 100644 index 000000000..850f1f3b1 --- /dev/null +++ b/src/openedx_learning/applets/cbe/__init__.py @@ -0,0 +1,3 @@ +""" +The CBE (Competency-Based Education) applet. +""" diff --git a/src/openedx_learning/applets/cbe/admin.py b/src/openedx_learning/applets/cbe/admin.py new file mode 100644 index 000000000..da7d3ca2b --- /dev/null +++ b/src/openedx_learning/applets/cbe/admin.py @@ -0,0 +1,17 @@ +""" +Django Admin pages for CBE models. +""" +from django.contrib import admin + +from .models import CompetencyTaxonomy + + +class CompetencyTaxonomyAdmin(admin.ModelAdmin): + """ + The CompetencyTaxonomy model admin. + """ + list_display = ["name", "export_id", "enabled", "taxonomy_overrides_org"] + list_filter = ["enabled", "taxonomy_overrides_org"] + + +admin.site.register(CompetencyTaxonomy, CompetencyTaxonomyAdmin) diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py new file mode 100644 index 000000000..2fede2c3d --- /dev/null +++ b/src/openedx_learning/applets/cbe/api.py @@ -0,0 +1,38 @@ +""" +Public API for Competency-Based Education (CBE). +""" +from __future__ import annotations + +from django.db.models import QuerySet + +from openedx_tagging.models import Taxonomy + +__all__ = [ + "is_competency_taxonomy", + "select_competency_taxonomies", +] + +# The accessor Django generates for the multi-table-inheritance link from Taxonomy to +# CompetencyTaxonomy. Deliberately private: callers use the functions below rather than +# spelling this out, so a model rename is a one-line change here and nowhere else. +_COMPETENCY_TAXONOMY_RELATION = "competencytaxonomy" + + +def is_competency_taxonomy(taxonomy: Taxonomy) -> bool: + """ + Return True if ``taxonomy`` is competency-enabled, i.e. has a CompetencyTaxonomy row. + + Costs one query per call unless ``taxonomy`` came from a queryset passed through + :func:`select_competency_taxonomies`. + """ + return hasattr(taxonomy, _COMPETENCY_TAXONOMY_RELATION) + + +def select_competency_taxonomies(taxonomies: QuerySet[Taxonomy]) -> QuerySet[Taxonomy]: + """ + Return ``taxonomies`` with each CompetencyTaxonomy row joined in. + + Pair this with :func:`is_competency_taxonomy` when checking more than one taxonomy, + so the check costs no additional query per row. + """ + return taxonomies.select_related(_COMPETENCY_TAXONOMY_RELATION) diff --git a/src/openedx_learning/applets/cbe/models.py b/src/openedx_learning/applets/cbe/models.py new file mode 100644 index 000000000..f89e2af4c --- /dev/null +++ b/src/openedx_learning/applets/cbe/models.py @@ -0,0 +1,52 @@ +""" +Models for Competency-Based Education (CBE). +""" +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from openedx_tagging.models import Taxonomy + +__all__ = [ + "CompetencyTaxonomy", +] + + +class CompetencyTaxonomy(Taxonomy): + """ + Marks a Taxonomy as competency-enabled, so CBE features apply to its tags. + + A taxonomy listed in this table: + + - can be displayed in the competency criteria association view. + - can be displayed in the competency progress tracking views. + - can also be displayed in the existing generic taxonomy views. + - constrains its associated content objects to those supported for progress + tracking, and to ones that could logically be used to demonstrate mastery of + the competency (for example, associating both a course and one assignment + within that same course would be ambiguous). + + A taxonomy *not* listed here: + + - is only displayed in the existing generic taxonomy views. + - is not displayed in competency criteria association views. + - is not displayed in competency progress tracking views. + - has no competency-specific constraints on its associated content objects. + + Creating a competency taxonomy creates both the parent ``Taxonomy`` row and this + row in one transaction; deleting either row removes both. + + .. no_pii: + """ + + taxonomy_overrides_org = models.BooleanField( + default=False, + help_text=_( + "When both an organization-scoped and a taxonomy-scoped rule profile " + "could apply to a criterion, this decides which one is assigned: false " + "assigns the organization's, true assigns this taxonomy's." + ), + ) + + class Meta: + verbose_name = "Competency Taxonomy" + verbose_name_plural = "Competency Taxonomies" diff --git a/src/openedx_learning/apps.py b/src/openedx_learning/apps.py new file mode 100644 index 000000000..070504eba --- /dev/null +++ b/src/openedx_learning/apps.py @@ -0,0 +1,15 @@ +""" +App Config for our umbrella openedx_learning app. +""" +from django.apps import AppConfig + + +class LearningConfig(AppConfig): + """ + Initialization for all applets must happen in here. + """ + + name = "openedx_learning" + verbose_name = "Open edX Core > Learning" + default_auto_field = "django.db.models.BigAutoField" + label = "openedx_learning" diff --git a/src/openedx_learning/migrations/0001_initial.py b/src/openedx_learning/migrations/0001_initial.py new file mode 100644 index 000000000..29e51fd1f --- /dev/null +++ b/src/openedx_learning/migrations/0001_initial.py @@ -0,0 +1,28 @@ +# Generated by Django 5.2.16 on 2026-08-06 19:25 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('oel_tagging', '0020_tag_depth_and_lineage'), + ] + + operations = [ + migrations.CreateModel( + name='CompetencyTaxonomy', + fields=[ + ('taxonomy_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='oel_tagging.taxonomy')), + ('taxonomy_overrides_org', models.BooleanField(default=False, help_text="When both an organization-scoped and a taxonomy-scoped rule profile could apply to a criterion, this decides which one is assigned: false assigns the organization's, true assigns this taxonomy's.")), + ], + options={ + 'verbose_name': 'Competency Taxonomy', + 'verbose_name_plural': 'Competency Taxonomies', + }, + bases=('oel_tagging.taxonomy',), + ), + ] diff --git a/src/openedx_learning/migrations/__init__.py b/src/openedx_learning/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_learning/models.py b/src/openedx_learning/models.py new file mode 100644 index 000000000..c008b9905 --- /dev/null +++ b/src/openedx_learning/models.py @@ -0,0 +1,7 @@ +""" +This module aggregates all applet model modules. +""" + +# pylint: disable=wildcard-import + +from .applets.cbe.models import * diff --git a/src/openedx_learning/models_api.py b/src/openedx_learning/models_api.py new file mode 100644 index 000000000..4f02b4f98 --- /dev/null +++ b/src/openedx_learning/models_api.py @@ -0,0 +1,9 @@ +""" +Models that we want callers to extend or make foreign keys to. + +This is also the stable import point for the model class itself, for callers that +need to create competency taxonomies directly. +""" + +# pylint: disable=unused-import +from .models import CompetencyTaxonomy diff --git a/src/openedx_learning/py.typed b/src/openedx_learning/py.typed new file mode 100644 index 000000000..e69de29bb diff --git a/test_settings.py b/test_settings.py index e1e250134..be0a84904 100644 --- a/test_settings.py +++ b/test_settings.py @@ -58,6 +58,7 @@ def root(*args): "openedx_tagging", "openedx_content", "openedx_catalog", + "openedx_learning", *openedx_content_backcompat_apps_to_install(), # Apps with models that are only used for testing "tests.test_django_app", diff --git a/tests/openedx_learning/__init__.py b/tests/openedx_learning/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_learning/applets/__init__.py b/tests/openedx_learning/applets/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_learning/applets/cbe/__init__.py b/tests/openedx_learning/applets/cbe/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py new file mode 100644 index 000000000..ae5ec93bc --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -0,0 +1,50 @@ +""" +Tests for the CBE public API surface (openedx_learning.api). +""" +import pytest + +from openedx_learning.api import is_competency_taxonomy, select_competency_taxonomies +from openedx_learning.models import CompetencyTaxonomy +from openedx_tagging.models import Taxonomy + +pytestmark = pytest.mark.django_db + + +def test_is_competency_taxonomy() -> None: + """ + is_competency_taxonomy() is True for a competency taxonomy, False for a plain one. + """ + competency = CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + plain = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + + assert is_competency_taxonomy(Taxonomy.objects.get(pk=competency.pk)) is True + assert is_competency_taxonomy(plain) is False + + +def test_is_competency_taxonomy_on_child_instance_directly() -> None: + """ + is_competency_taxonomy() also returns True when handed a CompetencyTaxonomy + instance directly, not just a parent Taxonomy fetched from the DB. + """ + competency = CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + assert is_competency_taxonomy(competency) is True + + +def test_select_competency_taxonomies_avoids_n_plus_1(django_assert_num_queries) -> None: + """ + select_competency_taxonomies() joins the CompetencyTaxonomy row in, so checking + is_competency_taxonomy() on every row in the queryset costs one query, not N+1. + """ + competency1 = CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + competency2 = CompetencyTaxonomy.objects.create(name="Welding", export_id="welding-v1") + plain = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + # Scoped to just these three: unfiltered Taxonomy.objects.all() also picks up the + # system-seeded "Language" taxonomy from oel_tagging's data migration, which would + # make the True/False counts below depend on incidental fixture data. + taxonomies = Taxonomy.objects.filter(pk__in=[competency1.pk, competency2.pk, plain.pk]) + + with django_assert_num_queries(1): + results = [is_competency_taxonomy(t) for t in select_competency_taxonomies(taxonomies)] + + assert results.count(True) == 2 + assert results.count(False) == 1 diff --git a/tests/openedx_learning/applets/cbe/test_models.py b/tests/openedx_learning/applets/cbe/test_models.py new file mode 100644 index 000000000..95eb195f8 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_models.py @@ -0,0 +1,69 @@ +""" +Tests for the CompetencyTaxonomy model. +""" +import pytest + +from openedx_learning.models import CompetencyTaxonomy +from openedx_tagging.models import Taxonomy + +pytestmark = pytest.mark.django_db + +# The default MTI reverse accessor. django-stubs cannot see dynamically added +# accessors, so these tests reach it by name; that name is the ADR-0013 contract. +RELATION = "competencytaxonomy" + + +@pytest.fixture(name="competency_taxonomy") +def _competency_taxonomy() -> CompetencyTaxonomy: + """Create a CompetencyTaxonomy for use in these tests.""" + return CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + + +def test_create_writes_both_rows(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + Creating a CompetencyTaxonomy writes both the parent Taxonomy row and the child row. + """ + assert Taxonomy.objects.filter(pk=competency_taxonomy.pk).exists() + assert CompetencyTaxonomy.objects.filter(pk=competency_taxonomy.pk).exists() + + +def test_taxonomy_overrides_org_defaults_false(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + taxonomy_overrides_org defaults to False. + """ + assert competency_taxonomy.taxonomy_overrides_org is False + + +def test_mti_round_trip(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + The MTI relationship works in both directions: the child reads the parent's + fields directly, and the parent reaches the child via the default accessor. + """ + assert competency_taxonomy.name == "Nursing" + parent = Taxonomy.objects.get(pk=competency_taxonomy.pk) + assert getattr(parent, RELATION) == competency_taxonomy + + +def test_plain_taxonomy_has_no_competencytaxonomy() -> None: + """ + A plain Taxonomy (no CompetencyTaxonomy row) raises RelatedObjectDoesNotExist. + """ + plain = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + # Django builds the accessor's RelatedObjectDoesNotExist as a subclass of the child + # model's DoesNotExist, so catching that names no dynamically added attribute. + with pytest.raises(CompetencyTaxonomy.DoesNotExist): + getattr(plain, RELATION) + + +def test_delete_cascades_both_directions() -> None: + """ + Deleting the parent Taxonomy removes the CompetencyTaxonomy row, and deleting + the child removes the parent row too. + """ + ct1 = CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + Taxonomy.objects.get(pk=ct1.pk).delete() + assert not CompetencyTaxonomy.objects.filter(pk=ct1.pk).exists() + + ct2 = CompetencyTaxonomy.objects.create(name="Welding", export_id="welding-v1") + ct2.delete() + assert not Taxonomy.objects.filter(pk=ct2.pk).exists() From 060ea6956eeddd294cea4702c77fbb7936d78df3 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 10 Aug 2026 16:32:34 -0400 Subject: [PATCH 2/4] docs: adjust ADR as needed --- .../0013-competency-taxonomy-detection.rst | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/docs/openedx_tagging/decisions/0013-competency-taxonomy-detection.rst b/docs/openedx_tagging/decisions/0013-competency-taxonomy-detection.rst index 645fe92a4..a057d9038 100644 --- a/docs/openedx_tagging/decisions/0013-competency-taxonomy-detection.rst +++ b/docs/openedx_tagging/decisions/0013-competency-taxonomy-detection.rst @@ -30,7 +30,7 @@ Decision Report a taxonomy's type entirely within **openedx-platform**, using the existing relation between ``Taxonomy`` and ``CompetencyTaxonomy`` established in ADR 0002, without adding any -field, method, or enum value to ``openedx_tagging`` or the CBE app: +field, method, or enum value to ``openedx_tagging``: .. image:: images/CompetencyTypeDetection.png :alt: Studio calls openedx-platform's serializer, which delegates to oel_tagging's pure @@ -41,11 +41,18 @@ field, method, or enum value to ``openedx_tagging`` or the CBE app: - openedx-platform's REST layer adds a read-only ``taxonomy_type`` value to its taxonomy serializer, computed by checking whether a related ``CompetencyTaxonomy`` row exists for that ``Taxonomy``: ``"competency"`` if so, ``"tags"`` otherwise. +- It performs that check through the CBE app's public API, + ``openedx_learning.api.is_competency_taxonomy()``, rather than naming the relation itself. + The relation name is a Django-generated default derived from the model's class name, so + spelling it in openedx-platform would let a rename upstream break Studio with nothing + failing in either repository's tests. - That same layer's queryset fetches the related ``CompetencyTaxonomy`` row alongside the - ``Taxonomy`` list, so the check costs no extra query per row. -- ``openedx_tagging``'s ``Taxonomy`` model, its base ``TaxonomySerializer``, and the CBE app - stay fully unaware of each other for this purpose: no new field, no new enum value, no - import. + ``Taxonomy`` list, using the companion ``select_competency_taxonomies()``, so the check + costs no extra query per row. +- ``openedx_tagging``'s ``Taxonomy`` model and its base ``TaxonomySerializer`` gain nothing + for this purpose: no new field, no new enum value, no import. That constraint is on + ``openedx_tagging`` alone. The CBE app owns this relation, so exposing it through the CBE + app's own public API is expected rather than avoided. - No creation-time wiring is needed to keep this accurate: ADR 0002 Decision 1 already creates the ``CompetencyTaxonomy`` row in the same transaction as its parent ``Taxonomy`` row, so the existence check can never drift out of sync the way a separately-stored @@ -53,10 +60,10 @@ field, method, or enum value to ``openedx_tagging`` or the CBE app: **Known trade-off.** A future third taxonomy type needs another hardcoded branch in openedx-platform's shared serializer, the same cost a field-based approach would have -avoided with a one-line enum addition. Accepted because keeping ``openedx_tagging`` and the -CBE app free of any competency-specific reference, even an inert stored value, was judged -more valuable than that extensibility, particularly given the project's move away from -system-defined taxonomies, which makes a third taxonomy flavor unlikely soon. +avoided with a one-line enum addition. Accepted because keeping ``openedx_tagging`` free of +any competency-specific reference, even an inert stored value, was judged more valuable +than that extensibility, particularly given the project's move away from system-defined +taxonomies, which makes a third taxonomy flavor unlikely soon. Rejected Alternatives ---------------------- @@ -69,9 +76,8 @@ A ``TaxonomyType(models.TextChoices)`` field (``TAGS``/``COMPETENCY``) added dir transaction as ADR 0002 Decision 1's existing lifecycle rule. Although this requires no per-request check and was more extensible for a hypothetical third taxonomy flavor, it still named a CBE-specific concept, a ``COMPETENCY`` enum value, directly in -``openedx_tagging``'s own schema and public API. Keeping ``openedx_tagging`` and the CBE -app fully free of any competency-specific reference, even an inert one, is worth the lost -extensibility. +``openedx_tagging``'s own schema and public API. Keeping ``openedx_tagging`` fully free of +any competency-specific reference, even an inert one, is worth the lost extensibility. Check for a related ``CompetencyTaxonomy`` row directly inside ``openedx_tagging`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From af5d5175afd11a4701b6c9d7030df711636b55b9 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 12 Aug 2026 10:42:49 -0400 Subject: [PATCH 3/4] refactor: address review feedback on CompetencyTaxonomy Drop taxonomy_overrides_org. It belongs to the rule-profile assignment mechanism in openedx_learning ADR 0002 Decision 4, which does not exist yet, so no code path reads it and the admin exposed an editable toggle nothing honored. Removing it after release would need a DEPR cycle on a public model attribute; adding it back is one AddField in the change that gives it meaning. Edited 0001_initial in place since the app is unreleased. ADR 0002 keeps describing the eventual design. Use the "competencytaxonomy" literal directly instead of a private constant. Callers can reach taxonomy.competencytaxonomy regardless, so the indirection hid nothing while blinding mypy, which can analyze a literal passed to select_related but never a name. Declare __all__ in the cbe admin module, matching the models and api modules the aggregators wildcard-import from. Pin is_competency_taxonomy() returning False for an unsaved taxonomy. Django makes RelatedObjectDoesNotExist an AttributeError subclass so hasattr() catches it, which is worth a test rather than an assumption. Stop naming the seeded "Language" taxonomy in the N+1 test's comment, since #654 removes that row. Co-Authored-By: Claude Opus 5 (1M context) --- src/openedx_learning/applets/cbe/admin.py | 8 ++++++-- src/openedx_learning/applets/cbe/api.py | 13 +++++-------- src/openedx_learning/applets/cbe/models.py | 12 ------------ src/openedx_learning/migrations/0001_initial.py | 1 - tests/openedx_learning/applets/cbe/test_api.py | 17 ++++++++++++++--- .../openedx_learning/applets/cbe/test_models.py | 7 ------- 6 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/openedx_learning/applets/cbe/admin.py b/src/openedx_learning/applets/cbe/admin.py index da7d3ca2b..71f7cf583 100644 --- a/src/openedx_learning/applets/cbe/admin.py +++ b/src/openedx_learning/applets/cbe/admin.py @@ -5,13 +5,17 @@ from .models import CompetencyTaxonomy +__all__ = [ + "CompetencyTaxonomyAdmin", +] + class CompetencyTaxonomyAdmin(admin.ModelAdmin): """ The CompetencyTaxonomy model admin. """ - list_display = ["name", "export_id", "enabled", "taxonomy_overrides_org"] - list_filter = ["enabled", "taxonomy_overrides_org"] + list_display = ["name", "export_id", "enabled"] + list_filter = ["enabled"] admin.site.register(CompetencyTaxonomy, CompetencyTaxonomyAdmin) diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index 2fede2c3d..8fc4861dc 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -12,20 +12,17 @@ "select_competency_taxonomies", ] -# The accessor Django generates for the multi-table-inheritance link from Taxonomy to -# CompetencyTaxonomy. Deliberately private: callers use the functions below rather than -# spelling this out, so a model rename is a one-line change here and nowhere else. -_COMPETENCY_TAXONOMY_RELATION = "competencytaxonomy" - def is_competency_taxonomy(taxonomy: Taxonomy) -> bool: """ Return True if ``taxonomy`` is competency-enabled, i.e. has a CompetencyTaxonomy row. Costs one query per call unless ``taxonomy`` came from a queryset passed through - :func:`select_competency_taxonomies`. + :func:`select_competency_taxonomies`. Returns False for an unsaved ``taxonomy``. """ - return hasattr(taxonomy, _COMPETENCY_TAXONOMY_RELATION) + # "competencytaxonomy" is the accessor Django generates for the multi-table-inheritance + # link from Taxonomy to CompetencyTaxonomy. + return hasattr(taxonomy, "competencytaxonomy") def select_competency_taxonomies(taxonomies: QuerySet[Taxonomy]) -> QuerySet[Taxonomy]: @@ -35,4 +32,4 @@ def select_competency_taxonomies(taxonomies: QuerySet[Taxonomy]) -> QuerySet[Tax Pair this with :func:`is_competency_taxonomy` when checking more than one taxonomy, so the check costs no additional query per row. """ - return taxonomies.select_related(_COMPETENCY_TAXONOMY_RELATION) + return taxonomies.select_related("competencytaxonomy") diff --git a/src/openedx_learning/applets/cbe/models.py b/src/openedx_learning/applets/cbe/models.py index f89e2af4c..7cbd8cb3a 100644 --- a/src/openedx_learning/applets/cbe/models.py +++ b/src/openedx_learning/applets/cbe/models.py @@ -1,9 +1,6 @@ """ Models for Competency-Based Education (CBE). """ -from django.db import models -from django.utils.translation import gettext_lazy as _ - from openedx_tagging.models import Taxonomy __all__ = [ @@ -38,15 +35,6 @@ class CompetencyTaxonomy(Taxonomy): .. no_pii: """ - taxonomy_overrides_org = models.BooleanField( - default=False, - help_text=_( - "When both an organization-scoped and a taxonomy-scoped rule profile " - "could apply to a criterion, this decides which one is assigned: false " - "assigns the organization's, true assigns this taxonomy's." - ), - ) - class Meta: verbose_name = "Competency Taxonomy" verbose_name_plural = "Competency Taxonomies" diff --git a/src/openedx_learning/migrations/0001_initial.py b/src/openedx_learning/migrations/0001_initial.py index 29e51fd1f..6cea73464 100644 --- a/src/openedx_learning/migrations/0001_initial.py +++ b/src/openedx_learning/migrations/0001_initial.py @@ -17,7 +17,6 @@ class Migration(migrations.Migration): name='CompetencyTaxonomy', fields=[ ('taxonomy_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='oel_tagging.taxonomy')), - ('taxonomy_overrides_org', models.BooleanField(default=False, help_text="When both an organization-scoped and a taxonomy-scoped rule profile could apply to a criterion, this decides which one is assigned: false assigns the organization's, true assigns this taxonomy's.")), ], options={ 'verbose_name': 'Competency Taxonomy', diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py index ae5ec93bc..8ebe712bf 100644 --- a/tests/openedx_learning/applets/cbe/test_api.py +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -30,6 +30,17 @@ def test_is_competency_taxonomy_on_child_instance_directly() -> None: assert is_competency_taxonomy(competency) is True +def test_is_competency_taxonomy_on_unsaved_instance() -> None: + """ + is_competency_taxonomy() returns False for an unsaved Taxonomy, rather than raising. + """ + # pk is None, so the reverse one-to-one descriptor short-circuits and raises + # RelatedObjectDoesNotExist, which Django defines as an AttributeError subclass + # precisely so hasattr() catches it here instead of propagating. + unsaved = Taxonomy(name="Unsaved", export_id="unsaved-v1") + assert is_competency_taxonomy(unsaved) is False + + def test_select_competency_taxonomies_avoids_n_plus_1(django_assert_num_queries) -> None: """ select_competency_taxonomies() joins the CompetencyTaxonomy row in, so checking @@ -38,9 +49,9 @@ def test_select_competency_taxonomies_avoids_n_plus_1(django_assert_num_queries) competency1 = CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") competency2 = CompetencyTaxonomy.objects.create(name="Welding", export_id="welding-v1") plain = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") - # Scoped to just these three: unfiltered Taxonomy.objects.all() also picks up the - # system-seeded "Language" taxonomy from oel_tagging's data migration, which would - # make the True/False counts below depend on incidental fixture data. + # Scoped to just these three: unfiltered Taxonomy.objects.all() would also pick up + # any taxonomies seeded outside this test, which would make the True/False counts + # below depend on incidental fixture data. taxonomies = Taxonomy.objects.filter(pk__in=[competency1.pk, competency2.pk, plain.pk]) with django_assert_num_queries(1): diff --git a/tests/openedx_learning/applets/cbe/test_models.py b/tests/openedx_learning/applets/cbe/test_models.py index 95eb195f8..2354d1d3d 100644 --- a/tests/openedx_learning/applets/cbe/test_models.py +++ b/tests/openedx_learning/applets/cbe/test_models.py @@ -27,13 +27,6 @@ def test_create_writes_both_rows(competency_taxonomy: CompetencyTaxonomy) -> Non assert CompetencyTaxonomy.objects.filter(pk=competency_taxonomy.pk).exists() -def test_taxonomy_overrides_org_defaults_false(competency_taxonomy: CompetencyTaxonomy) -> None: - """ - taxonomy_overrides_org defaults to False. - """ - assert competency_taxonomy.taxonomy_overrides_org is False - - def test_mti_round_trip(competency_taxonomy: CompetencyTaxonomy) -> None: """ The MTI relationship works in both directions: the child reads the parent's From e04d85c4d7dc5f4a0e30fc0eba74d6a2ba3294d2 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 12 Aug 2026 10:43:00 -0400 Subject: [PATCH 4/4] docs: add READMEs for openedx_learning and the cbe applet Every other top-level app in this repo has one. The applet README covers what a competency taxonomy is, the shared primary key it has with Taxonomy, the two API functions to check it, and why the check lives here rather than in openedx_tagging. Co-Authored-By: Claude Opus 5 (1M context) --- src/openedx_learning/README.rst | 11 +++++++++++ src/openedx_learning/applets/cbe/README.rst | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/openedx_learning/README.rst create mode 100644 src/openedx_learning/applets/cbe/README.rst diff --git a/src/openedx_learning/README.rst b/src/openedx_learning/README.rst new file mode 100644 index 000000000..33c11ea54 --- /dev/null +++ b/src/openedx_learning/README.rst @@ -0,0 +1,11 @@ +Learning App +============ + +The ``openedx_learning`` app holds models and APIs for what learners are meant to achieve +and how they get there. Its sibling ``openedx_content`` holds the material itself. + +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. diff --git a/src/openedx_learning/applets/cbe/README.rst b/src/openedx_learning/applets/cbe/README.rst new file mode 100644 index 000000000..869474767 --- /dev/null +++ b/src/openedx_learning/applets/cbe/README.rst @@ -0,0 +1,21 @@ +Competency-Based Education Applet +================================= + +The ``cbe`` applet models learner mastery of competencies. A competency is a tag; a +competency taxonomy is a taxonomy whose tags are competencies rather than ordinary labels. + +``CompetencyTaxonomy`` marks a taxonomy as one of those. It is a Django multi-table +inheritance subclass of ``openedx_tagging``'s ``Taxonomy``, so the two share a primary key: +a competency taxonomy is the same taxonomy, not a copy of one. Creating one writes both +rows in a single transaction, and deleting either row removes both. + +Ask whether a taxonomy is competency-enabled through ``openedx_learning.api``, not by +reaching for ``taxonomy.competencytaxonomy``. ``is_competency_taxonomy()`` answers for one +taxonomy; put a queryset through ``select_competency_taxonomies()`` first when checking a +list, so the whole list costs one query instead of one per row. Keeping the check here is +what lets ``openedx_tagging`` stay a generic tagging library that never learns CBE exists. + +The criteria, rule profile, and learner status tables this applet still needs are designed +in ``docs/openedx_learning/decisions/``. ``openedx_tagging`` ADR 0013 covers how +``openedx-platform`` calls ``is_competency_taxonomy()`` to report a taxonomy's type to +Studio.