feat: provide openedx_learning djangoapp and CompetencyTaxonomy model by jesperhodge · Pull Request #712 · openedx/openedx-core

jesperhodge · 2026-08-07T14:39:20Z

AI-first work.

This was coded with human-in-the-loop. I created the initial instructions; Claude Code created a plan, I (Jesper) reviewed the plan and edited as needed; Claude Code implemented, and I reviewed the implementation and tested.

What this does

Open edX already has "taxonomies", which are just collections of tags. This PR adds a way
to mark a taxonomy as holding competencies rather than ordinary tags, so that
competency-based education (CBE) features can later be built on top of it.

Adds a new Django app, openedx_learning, and its first sub-package, cbe. This is a
new home for learning-side features. It follows the same layout as the existing
openedx_content app: one Django app, split into subfolders for readability.
Adds one model, CompetencyTaxonomy. Marking a taxonomy as a competency taxonomy means
creating one of these. It does not copy the taxonomy or replace it; it points at the
existing taxonomy and adds a single extra setting to it.
Adds a small public API so other code can ask "is this taxonomy a competency taxonomy?"
without needing to know how that link is stored.
Wires it all up: app registration, the database migration, a Django admin page, and a
rule that keeps the dependency pointing one way (the CBE code may use the tagging code,
never the reverse).

There is no REST API and no user interface here. This is the groundwork that the follow-up
tickets #641 and #642 are waiting on.

Depends on work in edx-platform

openedx-core is a library, so this app only becomes active once edx-platform lists it in
its settings. That platform-side work is tracked in openedx/openedx-platform#38958.

How to test

This runs entirely inside this repo against a local SQLite file, so you do not need
devstack, tutor, or MySQL. manage.py defaults to the projects.dev settings, which are
already configured to use SQLite at ./dev.db, so there is nothing to configure.

1. Set up a virtualenv (skip if you already have one for this repo):

python3 -m venv .venv
.venv/bin/pip install -r requirements/dev.txt
.venv/bin/pip install -e .

.venv/ is already in .gitignore.

2. Create the database and a login.

.venv/bin/python manage.py migrate
.venv/bin/python manage.py createsuperuser

The first command creates dev.db and applies all migrations, including the new
openedx_learning.0001_initial.

3. Start the server.

.venv/bin/python manage.py runserver

4. Add a competency taxonomy through the admin.

Open http://127.0.0.1:8000/admin/ and log in. You should see a section called
Open edX Core > Learning containing Competency Taxonomies. Click Add, fill in
Name (for example Nursing) and Export id (for example nursing-v1), and save.

You should land back on a list showing the columns Name, Export id, Enabled, and Taxonomy
overrides org.

5. Confirm it wrote to both tables.

sqlite3 dev.db "SELECT t.id, t.name, c.taxonomy_overrides_org
  FROM oel_tagging_taxonomy t
  JOIN openedx_learning_competencytaxonomy c ON c.taxonomy_ptr_id = t.id;"

You should get your row back, something like 1|Nursing|0.

That join returning a result is the thing worth seeing: one save through the admin created
a row in the existing tagging table and a linked row in the new CBE table. That is the
link this PR is really about.

Cleaning up: rm dev.db and re-run step 2 to start over.

openedx-webhooks · 2026-08-07T14:39:28Z

Thanks for the pull request, @jesperhodge!

This repository is currently maintained by @axim-engineering.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

kdmccormick

two stylistic comments, but otherwise this looks good to me.

@ormsbee let me know if you'd like to do a pass before I approve and merge.

kdmccormick · 2026-08-07T17:54:04Z

+# 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"


i think that this is unnecessarily indirect, and as an abstraction it's leaky. you cannot stop callers from accessing taxonomy.competencytaxonomy directly, so renaming the CompetencyTaxonomy model would be a breaking change--we'd best treat it that way rather than pretending that all references to .competencytaxonomy will 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 like taxonomies.select_related("competencytaxonomy"), but when it's been abstracted out to taxonomies.select_related(_COMPETENCY_TAXONOMY_RELATION), it will never be able to do any static analysis.

kdmccormick · 2026-08-07T17:58:00Z

+        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."


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.

We do not have a task or epic that would implement this since it's out of scope. Epic 6 is what most closely relates to it though, but we don't currently have Epic level Github Issues, so that's a handful of issues. The one most closely related is probably #631 though.

If it's out of scope for Willow, then can we omit the field for now @mgwozdz-unicon ?

I think it would be fine to omit the field for now.

ormsbee · 2026-08-07T19:10:03Z

@kdmccormick: No need to block on me. Thank you.

mgwozdz-unicon · 2026-08-10T15:40:31Z

A couple of things from going through this:

READMEs. Every other top-level Django app in this repo (openedx_content, openedx_tagging, openedx_catalog, openedx_django_lib) has a README.rst in its own directory; openedx_learning doesn't yet. Given that, I'd add at least one for the app itself, and preferably a second one for the cbe applet too (that tier's less consistent across the repo, 3 of 8 applets have one, but still seems worth it for a brand-new applet).

test_select_competency_taxonomies_avoids_n_plus_1 comment. The test's comment ties its explanation of the pk__in filter to one specific named seed, the "Language" taxonomy from oel_tagging's data migration. #654 removes or converts that exact row, so the comment would end up referencing something that no longer exists. Wording it more generally now, e.g. guarding against "any taxonomies seeded outside this test" rather than naming "Language" specifically, keeps the comment accurate regardless of what happens to that seed data later, without anyone needing to circle back and touch it.

A couple smaller things Claude surfaced while I was reviewing this:

Question (src/openedx_learning/applets/cbe/api.py, is_competency_taxonomy): behavior is untested for an unsaved instance (pk is None). Accessing the reverse one-to-one descriptor on an unsaved parent raises a plain ValueError in Django, which hasattr() won't catch, so that would propagate uncaught instead of returning False. Probably a non-issue if no caller ever passes an unsaved instance, just flagging it as an assumption that's currently implicit rather than stated or guarded.

Nit (src/openedx_learning/admin.py:5): wildcard-imports from applets/cbe/admin.py, which doesn't declare an __all__, unlike this PR's models.py/api.py aggregators, which wildcard-import from modules that do declare one. Low risk, just inconsistent with the pattern the rest of the PR sets up.

jesperhodge · 2026-08-12T16:14:01Z

@mgwozdz-unicon thanks for the review! I made changes to address all your comments.

One exception:
"Question (src/openedx_learning/applets/cbe/api.py, is_competency_taxonomy): behavior is untested for an unsaved instance (pk is None). Accessing the reverse one-to-one descriptor on an unsaved parent raises a plain ValueError in Django, which hasattr() won't catch, so that would propagate uncaught instead of returning False. Probably a non-issue if no caller ever passes an unsaved instance, just flagging it as an assumption that's currently implicit rather than stated or guarded."

Claude figured out that this is not correct - hasattr() evaluates to False for this, and Claude added a test that shows that this works, so I think that's resolved as well.

I wonder why Claude surfaced this erroneous point. Could you share what model and effort level you ran it with, and what your prompt was? Did Claude use the /code-review skill for this?

mgwozdz-unicon

Thanks for addressing all of these. I've reviewed it again and this looks good to me.

On your question: Sonnet 5, high reasoning effort. I didn't use the /code-review skill here, my prompt included a specific list of questions plus an open "anything else worth flagging?", and that didn't happen to trigger it to use the skill (though I've found on my other projects that it often does), but I didn't force it to use the skill afterward either. This particular point came out of that open-ended last question rather than a targeted look at that one method. Going forward I'll make a point of invoking /code-review explicitly for these.

It's called out separately from my own points since it came straight from Claude as a flag to double-check rather than a verified finding. This time it was wrong on the specific exception type, but right that it was worth a test rather than an assumption.

bradenmacdonald

From a quick look, LGTM! I didn't do a thorough review.

jesperhodge · 2026-08-25T14:15:07Z

@kdmccormick bump - I addressed your comments, could you check this again?

jesperhodge · 2026-08-26T19:13:41Z

Hi @ormsbee it seems Kyle is out of office. Could you take this over? I fixed everything mentioned in Kyle's comments.

ormsbee · 2026-08-26T20:00:50Z

@jesperhodge: @kdmccormick is back today, and plans to review this PR.

kdmccormick

two small nits but if you agree with those, then good to go 👍🏻

kdmccormick · 2026-08-26T20:33:33Z

+
+# 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"


again, this is strangely indirect. just access .competencytaxonomy like app code would. if we messed up, then there'll be an AttributeError.

kdmccormick · 2026-08-26T20:36:46Z

@@ -0,0 +1,11 @@
+Learning App


Please bump the version to 1.3.0 https://github.com/openedx/openedx-core/blob/main/src/openedx_core/__init__.py

Going forward: do a patch bump for every bugfix, a minor bump for every backwards-compatible feature, and a major bump for every breaking change. (We'll soon have tooling set up to do this automatically based on conventional commits -- we're testing that out on other repos first).

ormsbee · 2026-08-27T14:04:48Z

@jesperhodge: I see that you've made the requested changes. Please resolve the conflicts, and I'll merge. If you could also squash your commits and give a revised commit message, that would be appreciated (otherwise, I just have to take a stab at it).

Thank you.

This PR adds a way to mark a taxonomy as holding competencies rather than ordinary tags, so that competency-based education (CBE) features can later be built on top of it. * Adds a new Django app, openedx_learning, and its first sub-package, cbe. This is a new home for learning-side features. It follows the same layout as the existing openedx_content app: one Django app, split into subfolders for readability. * Adds one model, CompetencyTaxonomy. Marking a taxonomy as a competency taxonomy means creating one of these. It does not copy the taxonomy or replace it; it points at the existing taxonomy and adds a single extra setting to it. * Adds a small public API so other code can ask 'is this taxonomy a competency taxonomy?' without needing to know how that link is stored. * Wires it all up: app registration, the database migration, a Django admin page, and a rule that keeps the dependency pointing one way (the CBE code may use the tagging code, never the reverse).

jesperhodge · 2026-08-27T17:23:47Z

@ormsbee thanks a lot; it's squashed. If you wouldn't mind could you just ping me and @anton Lezhneu in slack when it's merged? A single "ping" word is enough

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 7, 2026

openedx-webhooks added this to Contributions Aug 7, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Aug 7, 2026

jesperhodge marked this pull request as ready for review August 7, 2026 15:54

jesperhodge requested review from kdmccormick, mgwozdz-unicon and ormsbee and removed request for kdmccormick and ormsbee August 7, 2026 15:55

kdmccormick requested changes Aug 7, 2026

View reviewed changes

mphilbrick211 moved this from Needs Triage to In Eng Review in Contributions Aug 7, 2026

jesperhodge requested a review from kdmccormick August 12, 2026 16:14

mgwozdz-unicon approved these changes Aug 12, 2026

View reviewed changes

mgwozdz-unicon requested review from bradenmacdonald and ormsbee August 21, 2026 22:42

bradenmacdonald reviewed Aug 24, 2026

View reviewed changes

kdmccormick approved these changes Aug 26, 2026

View reviewed changes

jesperhodge force-pushed the jesperhodge/feat--640-cbe-app-foundation branch from 78c605f to 2ff6cb0 Compare August 27, 2026 17:08

jesperhodge force-pushed the jesperhodge/feat--640-cbe-app-foundation branch from 2ff6cb0 to d4b259d Compare August 27, 2026 17:23

ormsbee merged commit c0733f1 into openedx:main Aug 27, 2026
7 checks passed

github-project-automation Bot moved this from In Eng Review to Done in Contributions Aug 27, 2026

Uh oh!

Conversation

jesperhodge commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

AI-first work.

Related links

What this does

Depends on work in edx-platform

How to test

Uh oh!

openedx-webhooks commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormsbee commented Aug 7, 2026

Uh oh!

mgwozdz-unicon commented Aug 10, 2026

Uh oh!

jesperhodge commented Aug 12, 2026

Uh oh!

mgwozdz-unicon left a comment

Choose a reason for hiding this comment

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

jesperhodge commented Aug 25, 2026

Uh oh!

jesperhodge commented Aug 26, 2026

Uh oh!

ormsbee commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kdmccormick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ormsbee commented Aug 27, 2026

Uh oh!

jesperhodge commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jesperhodge commented Aug 7, 2026 •

edited

Loading

openedx-webhooks commented Aug 7, 2026 •

edited

Loading

ormsbee commented Aug 26, 2026 •

edited

Loading