Repository navigation
feat: add GET competency criteria-groups endpoint - #3
Closed
ufedaseyeuconsultant wants to merge 47 commits into
Conversation
Modernize openedx-core to uv + pyproject.toml (PEP 621/735) + python-semantic-release. Part of openedx/public-engineering#506 (tracked in openedx/public-engineering#516). - Replace setup.py/setup.cfg with pyproject.toml (PEP 621 static metadata), version derived from git tags via setuptools-scm. - Switch from pip-compile to uv with PEP 735 dependency groups; commit uv.lock. - Update tox.ini to use tox-uv with uv-venv-lock-runner. - Update CI to use astral-sh/setup-uv; SHA-pin all actions. - Add python-semantic-release + release.yml, replacing the old token-based pypi-publish.yml with OIDC trusted publishing. - src/ layout not touched -- this repo already used it, so no change was needed here. - Delete setup.py, setup.cfg, .coveragerc, requirements/ (all .in/.txt files, constraints.txt, private.readme), .github/workflows/pypi-publish.yml. Addresses salman2013's PR review feedback: - Keep the CI job name as tests, matching the pre-migration name (deferring the per-toxenv rename to avoid breaking required-status-check matching). - __version__ falls back to "unknown" on PackageNotFoundError instead of leaving it unset, with the except branch marked pragma: no cover since the package is always installed in CI. - Remove the legacy, unused .readthedocs.yml (superseded by .readthedocs.yaml). - Drop an unintended fetch-depth: 0 on the checkout step -- a leftover side effect of removing the separate setup-python step, not a deliberate choice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dropped during merge
…ernize-python-tooling # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/quality.txt # requirements/test.txt
…penedx#803) Add taxonomy_type as a write-only ChoiceField on TaxonomySerializer ("tags" default, "competency" via a new TaxonomyType enum). TaxonomyView.perform_create() and create_import() discard it after validation: openedx_tagging must never import openedx_learning, so it can't act on it. create_import()'s taxonomy-creation step is extracted into an overridable _create_taxonomy_for_import() hook for subclasses. Add create_competency_taxonomy() to the CBE applet: creates the Taxonomy and linked CompetencyTaxonomy row in one transaction, via save_base(raw=True) since save() would re-save Taxonomy with unpopulated field values. Add CompetencyTaxonomyView(TaxonomyView), overriding perform_create() and _create_taxonomy_for_import() to dispatch to create_competency_taxonomy() when taxonomy_type="competency". Lives here since this is the layer that can see both openedx_tagging and openedx_learning. See openedx#628 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mad/modernize-python-tooling # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/pip-tools.txt # requirements/quality.txt # requirements/test.txt # src/openedx_core/__init__.py
…e job Matches openedx/sample-plugin's current release.yml; the custom OPENEDX_SEMANTIC_RELEASE_GITHUB_TOKEN secret isn't needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…openedx#818) AI note: Claude slightly assisted with this, though most of it was written by me.
tbain
suggested changes
Sep 25, 2026
There was a problem hiding this comment.
- Bug: Codex points out that the serialization is changing the names of values unexpectedly; it states:
The response field names do not match the contract consumed by tickets openedx#672/openedx#681. serializers.py:104 emits tag_id, while the contract requires oel_tagging_tag_id; serializers.py:125-126 emits group_id, object_tag_id, and rule_profile_id instead of competency_criteria_group_id, oel_tagging_objecttag_id, and competency_rule_profile_id. After camel-casing, the frontend receives groupId rather than the expected competencyCriteriaGroupId, so it cannot associate criteria with groups or resolve their rule metadata. The new tests assert these incorrect names at test_views.py:676-679, masking the contract break.
Is the resolution for these naming changes resolved elsewhere and this is expected, or is this a genuine bug and we need to adjust these names to match expected names for these attributes?
-
Improvement: Comments in src/openedx_learning/applets/cbe/rest_api/v1/serializers.py seem, perhaps, overly wordy and I'm not sure they convey anything of great value - maybe we can cut these down or outright delete them? Unless this is the standard for these sorts of files?
-
Improvement: Comment in src/openedx_learning/applets/cbe/rest_api/v1/views.py line 136 is unnecessary.
Raw AI output:
Details
## VerdictRequest changes.
Findings
- [P1] Bug — response field names do not match the agreed API contract.
serializers.py:104emitstag_id, while the contract requiresoel_tagging_tag_id.serializers.py:125-126emitsgroup_id,object_tag_id, andrule_profile_idinstead ofcompetency_criteria_group_id,oel_tagging_objecttag_id, andcompetency_rule_profile_id. After camel-casing, the frontend receivesgroupIdrather thancompetencyCriteriaGroupId, so it cannot associate criteria with groups or resolve their rule metadata. The tests assert these incorrect names attest_views.py:676-679, masking the contract break.
Checks
git diff --check: passed- Python compilation of changed files: passed
- Focused pytest run: could not start because
openedx_eventsis missing from the environment - PR description includes AI attribution and manual testing steps
Makefile now assumes an already-synced/activated local env; uv run stays in CI workflow steps only. See openedx/ccx-keys#190 (review comment r4097501942). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ernize-python-tooling # Conflicts: # requirements/base.txt # requirements/ci.txt # requirements/dev.txt # requirements/doc.txt # requirements/quality.txt # requirements/test.txt
This was referenced Sep 29, 2026
Author
|
@tbain 2 and 3 done Also including detailed investigation of ClaudeChecked this against the real downstream consumer rather than against the ticket's own pseudocode: fork AShatsila/frontend-app-authoring, branch ashatsila/672-manage-and-apply-competencies (commit 9f76732e), the actual implementation for openedx#672.src/taxonomy/competency-management/data/types.ts defines CompetencyCriteriaGroupsResponse, explicitly commented as "openedx#681's documented response shape", and it matches the current implementation field-for-field: CompetencyCriteriaGroup reads tagId, parentId, courseKey; CompetencyCriterion reads groupId, ruleProfileId. Not oelTaggingTagId, competencyCriteriaGroupId, or competencyRuleProfileId. object_tag_id isn't even modeled on this GET response; the frontend only reads objectId. This isn't just a type declaration nobody consumes. The actual usage site confirms it: CompetencyAssociationsContext.tsx:322 reads criterion.groupId when writing focus after a create call, not criterion.competencyCriteriaGroupId. There is a real documentation gap here, but it's not in this code: ticket openedx#672's own "Implementation specifics" section states that fields read back from openedx#681 are competencyCriteriaGroupId-style, which matches openedx#681's own stale AC text but not what either side actually built. Both openedx#681's AC and openedx#672's description appear to have been written before implementation, against the ticket's original pseudocode, and neither was updated once the two sides converged on the shorter names during actual implementation. Given that, I'm leaving serializers.py and the tests as they are: renaming to the long form would break the real, already-built openedx#672 integration rather than fix a contract mismatch. I'll follow up separately on getting openedx#681's and openedx#672's written text corrected so this doesn't mislead the next reviewer. |
ufedaseyeuconsultant
force-pushed
the
feat--681-get-competency-criteria-and-groups
branch
from
September 29, 2026 17:27
d3e8a7e to
46df87f
Compare
… published deps The last real release (1.4.0) published Django, edx-drf-extensions, and edx-organizations with no version bounds at all, since pre-migration setup.py read requirements/base.in (unpinned) rather than base.txt or constraints.txt. The pyproject.toml dependencies list introduced by the uv migration added bounds that never existed in published metadata -- most importantly Django<6.0, which would block openedx-platform (or any other consumer) from resolving Django 6 until openedx-core cuts a new release. The edx-drf-extensions>=10.7.0 lower bound also didn't match history: it came from a stale requirements/base.txt snapshot on this branch at the time pyproject.toml was authored. The actual last-released pin (v1.4.0 tag) was 10.8.0, and the latest pre-migration pin on main (before requirements/base.txt was deleted by the migration) was 10.9.0. Neither matches 10.7.0, and none of these pins were ever part of published Requires-Dist in the first place. Restore all three to unbounded, matching PyPI's published metadata for 1.4.0. Django<6.0 is still enforced for this repo's own lockfile/test resolution via [tool.uv].constraint-dependencies, which only affects this repo and does not propagate to consumers. uv.lock regenerated; resolved package set is unchanged (specifiers only). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tox.ini's envlist listed a lint-imports entry with no matching [testenv:lint-imports] section, so it silently fell back to the generic [testenv] and just re-ran pytest instead of linting imports. This also meant Makefile's `make quality` (which delegates to `tox -e quality`) never actually ran import-linter locally. Delete the bogus envlist entry and instead run `lint-imports` as a step in [testenv:quality], right after isort. import-linter is already pulled in via the quality dependency-group (through test -> test-base), so no new dependency is needed. .github/workflows/lint-imports.yml is intentionally left in place: it is currently a required branch-protection status check on main. Removing it now would leave that required check permanently stuck at "Expected -- Waiting for status" on this PR's head commit, deadlocking the PR. Per feanil, that workflow should only be removed once quality's lint-imports step is confirmed working and he lifts the branch-protection requirement on "Lint Python Imports" (since tox -e quality is already a required check and will cover the same ground). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- ci.yml: added --locked to uv sync/uv run tox steps. Verified uv sync --locked --group ci passes cleanly. - pyproject.toml: the comment next to the unbounded Django/etc. dependencies pointed at [tool.uv].constraint-dependencies as the place to add a repo-specific bound -- that block is tool-generated output (edx_lint write_uv_constraints), not an editing target. Corrected to point at [tool.edx_lint].uv_constraints instead, and noted Django<6.0 is already covered globally via edx-lint's common_constraints.txt (confirmed directly), not something this repo added itself. Per feanil's review on openedx#704: discussion_r4147327735, r4147327755. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
tbain
approved these changes
Sep 30, 2026
tbain
left a comment
tbain
left a comment
There was a problem hiding this comment.
Approved from me, and Codex has no further comments or corrections
…odernize-python-tooling feat: modernize Python tooling to uv + pyproject.toml + semantic-release
lint-imports already runs as part of the quality tox env in ci.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#800) Add the CBE criteria models: CompetencyRuleProfile, CompetencyCriteriaGroup and CompetencyCriterion (the criteria tree's AND/OR node and leaf), the rule payload contract and its validation, and CompetencyTaxonomy.taxonomy_overrides_org. Delete behavior cascades from tags, groups and object tags. Register django-simple-history, layer openedx_catalog beside openedx_content in the import contracts, and record the scope_code and on_delete decisions in ADR-0002. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Every tag now always carries an external identifier (the Competency ID shown on the Competency Management page), whether an institution assigns one or the system generates it from the tag's name. Previously a tag only got an external_id via file import; one created through the taxonomy editor or the tagging API had none at all. Tag.save() generates a candidate from the tag's value when external_id is empty, resolving collisions with a casefold-based per-taxonomy uniqueness check and a numeric suffix. Taxonomy.add_tag() rejects a caller-supplied duplicate (exact or case-different) with ValueError instead of letting it reach the database as an IntegrityError. A new migration backfills every existing null or empty-string external_id, in capped in-memory and SQL batches, without overwriting institution-assigned ones, then makes the column NOT NULL; rollback only loosens that constraint and leaves generated data in place. A stale pre-upgrade export whose id collides with another tag's real external_id still fails safely on re-import, tripping a co-triggered rename action's existing duplicate-value check; this behavior and its narrower residual limitation are documented in ADR 0012. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…emove-lint-imports-workflow build: remove redundant lint-imports workflow
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 9.0.0 to 10.2.0. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@c771a70...c18668a) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.2.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [python-semantic-release/python-semantic-release](https://github.com/python-semantic-release/python-semantic-release) from 10.6.2 to 10.7.0. - [Release notes](https://github.com/python-semantic-release/python-semantic-release/releases) - [Changelog](https://github.com/python-semantic-release/python-semantic-release/blob/master/CHANGELOG.rst) - [Commits](python-semantic-release/python-semantic-release@9a026e9...b700dbe) --- updated-dependencies: - dependency-name: python-semantic-release/python-semantic-release dependency-version: 10.7.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Add two of openedx#642's four learner-status models. CompetencyMasteryStatus is the mastery-rank lookup table, seeded with ids 10, 20 and 30 so the database can compare ranks directly. StudentCompetencyStatus holds one row per learner and competency tag, updated in place, and a check constraint keeps AttemptedNotDemonstrated out of it. Deleting a user deletes their status rows, and a tag with status rows cannot be deleted. Part of openedx#642. Co-authored-by: Jesper Hodge <jhodge@unicon.net>
…/python-semantic-release/python-semantic-release-10.7.0 build(deps): bump python-semantic-release/python-semantic-release from 10.6.2 to 10.7.0
…/astral-sh/setup-uv-10.2.0 build(deps): bump astral-sh/setup-uv from 9.0.0 to 10.2.0
Add the last two of openedx#642's four learner-status models. StudentCompetencyCriterionStatus holds a learner's mastery status for one leaf criterion, and StudentCompetencyCriteriaGroupStatus holds it for one criteria group. Each holds one row per learner and criterion, or per learner and group, updated in place, and accepts all three status values. Deleting a user deletes their status rows, and a criterion, group, object tag, tag or taxonomy with status rows beneath it cannot be deleted. Closes openedx#642. Co-authored-by: Jesper Hodge <jhodge@unicon.net>
A taxonomy re-import row can change an existing tag's external_id in place by setting a previous_id column alongside its new id: when previous_id matches an existing tag's external_id and the row's id differs, the import renames that tag instead of deleting it and creating a new one, preserving its primary key, its associations, and its import/export identity (see ADR 0010). Renames within one import are order-independent, including a swap or longer cycle where two or more tags trade external_id values in the same file: any tag whose current external_id is another row's rename target is staged through a temporary placeholder before the row claiming that id executes, since (taxonomy, external_id) is a per-statement unique constraint. A rename or parent update resolves its target tag by primary key rather than re-resolving by external_id at execute time, so a tag already renamed earlier in the same import is still matched correctly. This staging covers external_id only; a row that also tries to take the other tag's current value in the same swap is still rejected, since value carries the same per-taxonomy uniqueness without the same staging treatment. The import plan rejects, before any action executes: two or more rows sharing the same previous_id; two or more rows resolving to the same final external_id; a parent_id referencing a parent's stale pre-rename external_id; and a rename targeting a tag a replace-mode delete sweep is removing in the same import. Conversely, a replace-mode delete sweep now correctly treats a rename row's id as the tag's desired new external_id, not proof that the tag currently holding that id should survive, so a rename can reuse an external_id the same import's delete sweep is freeing up. Test coverage runs the rename round-trip, replace-mode, and idempotent re-import scenarios through the CSV parser as well as JSON, matching the path Studio's import wizard actually uses. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…edx#853) django-simple-history bakes the SIMPLE_HISTORY_DATE_INDEX setting's value into the db_index it generates for every HistoricalRecords() model's history_date field. This library's test settings never set it, so its committed migrations recorded db_index=True (the django-simple-history default) for CompetencyCriteriaGroup, CompetencyCriterion, and CompetencyRuleProfile's historical tables. openedx-platform, the primary consumer these models are built for, sets SIMPLE_HISTORY_DATE_INDEX = False globally in openedx/envs/common.py, so installing this library there makes the live model state disagree with the shipped migrations: Django detects a missing migration for history_date on every one of these three tables the moment openedx-platform's own test suite loads them, failing its MigrationTests.test_migrations_are_in_sync check. Sets the same value in test_settings.py and dev.py so this library's own migrations match what its actual consumer produces, and adds the migration that `makemigrations` generates as a result. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…terion (openedx#716) Adds archived = models.BooleanField(default=False) to both models, in one migration, per ADR 0002 Decision 7's archive-only retirement rule. No archived_at companion field: django-simple-history (ADR 0003) already tracks both models, covering the "when" without a separate timestamp. Both models' existing `.. no_pii:` class-level docstring annotation already covers the new field, matching CompetencyRuleProfile.archived's existing non-PII precedent; no separate registry change needed. This ticket makes no behavior changes: nothing here sets, reads, or filters on the field. That is downstream tickets' scope (openedx#674, openedx#675, openedx#681). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…edx#861) openedx-platform's own import-linter config already treats openedx_catalog (and openedx_learning) as an isolated app, reachable from outside only through its api/models_api/data/tests modules, which is how a foreign-key import into openedx_catalog.models (instead of models_api) got caught on the platform's CI and not here: this repo's own .importlinter only layers top-level packages, it says nothing about which modules within a package may be reached from outside it. Adds an equivalent "isolated_apps" contract type, scoped to openedx_catalog only for now; extending it to openedx_content, openedx_learning, or openedx_tagging would surface pre-existing violations (confirmed by testing the broader scope and reverting), out of scope for this change. IsolatedAppsContract itself is ported verbatim from openedx-platform's openedx.testing.importlinter.isolated_apps_contract (this library can't depend on openedx-platform), with one change: self.isolated_apps and self.allowed_modules are copied through list() before iterating, working around a pylint false positive (E1133) caused by pylint not understanding that Contract._populate_fields() replaces these class-level Field descriptors with their parsed list values at the instance level. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 10.2.0 to 10.3.0. - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@c18668a...1c37ad0) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 8.0.1 to 8.0.2. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@3e5f45b...9000827) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: 8.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 7.0.1 to 7.0.2. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@043fb46...cf430e0) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: 7.0.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Adds GET api/cbe/v1/rule_profiles/, a paginated, read-only collection
of every non-archived CompetencyRuleProfile, and the REST scaffolding
that later CBE endpoints will share: the applets/cbe/rest_api package,
openedx_learning/urls.py, and openedx_learning/rules.py. Each row
reports id, scope_type (derived from the scope columns), rule_type,
rule_payload and archived; scope_code and the scope foreign keys stay
internal. The endpoint is UNSTABLE until the rest of the rule profile
API lands.
Reading is gated on a new openedx_learning.view_competencyruleprofile
permission, registered as rules.is_authenticated &
can_view_competency_rule_profile. Any signed-in user may list profiles
and read the system default. A taxonomy-scoped profile is as visible
as its taxonomy, checked through has_perm("oel_tagging.view_taxonomy")
so that openedx-platform's org-aware override applies. Course- and
organization-scoped profiles are refused to everyone for now (openedx#645).
The list does not yet filter rows per profile; that has to change when
the first scoped profile is created.
A consuming project includes openedx_learning.urls under a prefix of
its choice (openedx-platform uses api/) and needs
rules.apps.AutodiscoverRulesConfig in INSTALLED_APPS and
rules.permissions.ObjectPermissionBackend in AUTHENTICATION_BACKENDS,
or every caller is refused. See src/openedx_learning/README.rst.
Closes openedx#773.
Co-authored-by: javoconsultant <jmeliev_consultant@flex.unicon.net>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs: ADR proposals for improved static/media asset handling * docs: fix forward reference to ADR that doesn't exist yet * docs: minor typos * docs: blob_... -> media_file_namespace, dedup at org level, link other ADR * docs: forward ref to next ADR * docs: mention the future option for flex packages that combine runs * docs: edit to clarify/simplify some of the rejected alternatives * docs: mention idea of using collections to separate runs within LP * docs: update since catalog -> content layering is pending decision * docs: CourseContent holds the relationship * docs: typo * docs: clarifications around using org code as media file namespace * docs: guiding principle: no 1:1 assumptions between contexts and packages * docs: clarify wording in ADR 0012 Co-authored-by: Kyle D McCormick <kyle@axim.org> * docs: suggestions from review --------- Co-authored-by: Kyle D McCormick <kyle@axim.org>
…/actions/upload-artifact-7.0.2 build(deps): bump actions/upload-artifact from 7.0.1 to 7.0.2
…/astral-sh/setup-uv-10.3.0 build(deps): bump astral-sh/setup-uv from 10.2.0 to 10.3.0
…/actions/download-artifact-8.0.2 build(deps): bump actions/download-artifact from 8.0.1 to 8.0.2
Add POST /api/cbe/v1/competencies/<int:tag_id>/criteria/, associating a gradeable subsection with a competency. When group_id is supplied, the CompetencyCriterion is attached to that existing leaf CompetencyCriteriaGroup. When it is omitted, the root and course-level groups for this competency and course are reused or created, and a new leaf is always created under them. Add to applets/cbe/api.py, all marked UNSTABLE: resolve_competency_tag(), resolve_supplied_leaf_group(), create_competency_criterion(), and associate_competency_criterion() as the single entry point the view calls. Group creation is the private _create_leaf_group(), so groups are only ever created behind the duplicate-association check. The API raises Django ValidationError and model DoesNotExist exceptions; the view translates them into 400 and 404 responses, and a 404 carries the exception's message, so a missing tag, a tag outside a competency taxonomy, and a missing group can be told apart. Group creation and the duplicate-association check run inside one transaction that first locks the competency's Tag row with select_for_update(), so concurrent requests for the same competency are serialized. MySQL has no partial unique indexes, so the one-root-per-tag rule cannot be backed by a constraint and relies on that lock. Add migration 0015: a UniqueConstraint on CompetencyCriteriaGroup (tag, course, parent) that backstops course-level group creation. The root and course-level lookups match archived rows deliberately, so an archived container is never replaced by a second row in its slot. When a request supplies no rule fields, the criterion is assigned the live rule profile scoped to the competency's taxonomy if one exists, and the seeded system-default profile otherwise. It is never left with all three rule fields null, because CompetencyCriterion's profile_xor_override_check constraint forbids that. A request that supplies a profile together with an override, only one of the two overrides, or an archived or unknown profile is rejected with a 400 before anything is written. Related to openedx#665 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a read endpoint returning a competency's full CompetencyCriteriaGroup tree (root, course-level, leaf) and CompetencyCriterion leaves under it, as two flat lists, so Studio's Manage & Apply Competencies flow can render existing associations from persisted state instead of tracking them client-side. GET /api/cbe/v1/competencies/<int:tag_id>/criteria-groups/ resolves the competency tag (reusing openedx#665's resolve_competency_tag(), 404 if missing or not on a CompetencyTaxonomy), checks read access via the new CompetencyReadPermission (oel_tagging.view_tag, permissive for any enabled taxonomy), and returns every non-archived group and criterion via the new get_competency_criteria_tree(). An empty tree is a valid 200 with two empty arrays, not a 404. Filtering on archived relies on the archived field openedx#716 already added to both models. Three deviations from the ticket's literal spec, found while verifying against the actual adopted models rather than the ticket's speculative pseudocode: the real FK attribute names are tag/group/object_tag/rule_profile (not the oel_tagging_tag_id-style names the ticket assumed); course_key uses a plain CharField with a dotted source instead of SlugRelatedField, since CourseRun.course_key is an opaque-keys field that SlugRelatedField can't render to JSON; and the criterion read shape is a new CompetencyCriterionReadSerializer rather than an added field on the existing create-request serializer, whose own object_id field is write_only and would have broken under a dotted source. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…x#681) Tightens the two new read-serializer docstrings (CompetencyCriteriaGroupSerializer, CompetencyCriterionReadSerializer) per PR review, keeping the non-obvious "why" (opaque-keys course_key serialization, dotted-source object_id write/read split) without the surrounding restatement. Drops CompetencyCriteriaTreeView.get()'s docstring, which only restated its three-line body. Field names on both serializers (tag_id, group_id, object_tag_id, rule_profile_id) are left unchanged: verified against the real openedx#672 frontend consumer (AShatsila/frontend-app-authoring@ashatsila/672-manage-and-apply-competencies), whose types.ts and actual read sites use exactly these short names, not the oel_tagging_tag_id-style names openedx#681's own AC text assumed. That AC text and both sides converged on the shorter contract; renaming here would break the real integration rather than fix a mismatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The other criteria-authoring entry points carry the UNSTABLE note, because group update and archive are still to come. Without it this read would be a stable public API that can only change through DEPR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#681) The tree view's docstring said each criterion carries competency_criteria_group_id, but the serializer returns group_id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ufedaseyeuconsultant
force-pushed
the
feat--681-get-competency-criteria-and-groups
branch
from
October 16, 2026 20:38
46df87f to
84b92cd
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Adds a read endpoint that returns a competency's full
CompetencyCriteriaGrouptree (root,course-level, leaf) and
CompetencyCriterionleaves under it, as two flat lists:Studio's Manage & Apply Competencies authoring flow needs this to render a competency's
existing groups, criteria, and already-tagged subsections from persisted backend state,
instead of tracking or guessing at that state client-side, so the next write action (add or
remove an association) acts against ground truth.
An empty tree (the tag exists, nothing authored yet) is a valid
200with two empty arrays,not a
404. A404means thetag_iddoesn't exist or isn't on aCompetencyTaxonomy; a403means the caller can't view that taxonomy.Which roles this affects: Course Author and Platform Administrator, via Studio's
competency-management UI (read-only in this PR); Developer, as a new API surface to build
against (openedx#672, openedx#683 consume it next).
Scope note for reviewers: the
CompetencyCriteriaGroup/CompetencyRuleProfile/CompetencyCriterionmodels, migrations, and the create-criterion endpoint (openedx#665) that thisPR builds on haven't merged into
mainon their own yet, so this branch carries that fullfoundation along with the new read endpoint. The commits from
a6f3b04through5293666are that inherited work, brought in from the branch where openedx#665 was developed;
d3e8a7eisthis PR's own addition. That inherited history includes its own authors' fixes for two real
issues (a duplicate, redundant migration chain that briefly built the same schema twice, and
a handful of tests pinned to stale behavior) found and corrected independently on that branch
before it was merged in here; both were re-verified as part of stabilizing this branch, and
the full suite is green.
Public API additions:
get_competency_criteria_tree(tag_id)inopenedx_learning.applets.cbe.api,CompetencyCriteriaGroupSerializerandCompetencyCriterionReadSerializerin the CBE REST API'sv1/serializers.py, andCompetencyReadPermissioninv1/permissions.py. No breaking changes; this whole surface isnew.
Supporting information
CompetencyTaxonomy) and [BE] Create CompetencyCriteria openedx/openedx-core#665 (CompetencyCriterion/CompetencyCriteriaGroupmodels and the create-criterion endpoint), both carried in thisbranch per the scope note above.
docs/openedx_learning/decisions/0002-competency-criteria-model.rst, Decision 5, forthe indexes this read path relies on.
Testing instructions
python manage.py migrate openedx_learning.(via
POST /api/cbe/v1/competencies/<tag_id>/criteria/from [BE] Create CompetencyCriteria openedx/openedx-core#665, or directly through thePython API:
openedx_learning.api.associate_competency_criterion(...)).GET /api/cbe/v1/competencies/<tag_id>/criteria-groups/as a user who can view thetaxonomy. Expect
200with:groups: the root (parent_idandcourse_keybothnull), the course-level group,and the leaf, each with the correct
parent_id.criteria: the one criterion, withgroup_idmatching the leaf,id,object_tag_id,object_id(the tagged subsection's usage key),rule_profile_id,rule_type_override,rule_payload_override.archived=True) and repeat step 3: confirm bothare excluded from the response.
GETthe same URL for atag_idthat doesn't exist, and for one that exists but isn't ona
CompetencyTaxonomy: expect404in both cases.GETas a user without view access to the taxonomy: expect403.pytest tests/openedx_learning/applets/cbe/.Other information
No backward-incompatible changes; this PR only adds new fields, a new endpoint, and a new
boolean
archivedcolumn (defaultFalse) on two models that had none of this data before.Depends on [BE] Create CompetencyCriteria openedx/openedx-core#665 not yet having a separate, merged PR — see the scope note in Description.
Out of scope, tracked separately: any write behavior on this new resource ([BE] Create CompetencyCriteria openedx/openedx-core#665/[BE] Delete Competency Criterion openedx/openedx-core#674/[BE] Build endpoint for removing a Competency Criteria Group openedx/openedx-core#675
own that), course-run scoping and pagination for large result sets ([BE] Add filtering to GET Competency Criteria & Groups openedx/openedx-core#682, already built as
a follow-up on top of this branch), and a
deletableconvenience flag (deferred, no ADRyet settles the mechanism).
🤖 Generated with Claude Code