-
Notifications
You must be signed in to change notification settings - Fork 27
feat: provide openedx_learning djangoapp and CompetencyTaxonomy model #712
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
Open
jesperhodge
wants to merge
1
commit into
openedx:main
Choose a base branch
from
jesperhodge:jesperhodge/feat--640-cbe-app-foundation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| """ | ||
| Learning-domain features for Open edX Core. | ||
|
|
||
| Currently one applet, cbe, holding the Competency-Based Education models. | ||
| """ |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """ | ||
| This module aggregates all applet Django Admin modules. | ||
| """ | ||
| # pylint: disable=wildcard-import | ||
|
|
||
| from .applets.cbe.admin import * |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 * |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| """ | ||
| The applets that make up the openedx_learning Django app. | ||
| """ |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| """ | ||
| The CBE (Competency-Based Education) applet. | ||
| """ |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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." | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you add a note stating that this isn't yet used anywhere? bonus points if you add a comment linking to a task or epic issue that would implement it. |
||
| ), | ||
| ) | ||
|
|
||
| class Meta: | ||
| verbose_name = "Competency Taxonomy" | ||
| verbose_name_plural = "Competency Taxonomies" | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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',), | ||
| ), | ||
| ] |
Empty file.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| """ | ||
| This module aggregates all applet model modules. | ||
| """ | ||
|
|
||
| # pylint: disable=wildcard-import | ||
|
|
||
| from .applets.cbe.models import * |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Empty file.
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
Empty file.
Empty file.
Empty file.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() |
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.
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 that this is unnecessarily indirect, and as an abstraction it's leaky. you cannot stop callers from accessing
taxonomy.competencytaxonomydirectly, so renaming the CompetencyTaxonomy model would be a breaking change--we'd best treat it that way rather than pretending that all references to.competencytaxonomywill go through the two functions below.I would just use the
"competencytaxonomy"string literal in the function below, as is idiomatic in django code. mypy is sometimes actually smart enough to do type checking on expressions liketaxonomies.select_related("competencytaxonomy"), but when it's been abstracted out totaxonomies.select_related(_COMPETENCY_TAXONOMY_RELATION), it will never be able to do any static analysis.