feat: add GET competency criteria-groups endpoint by ufedaseyeuconsultant · Pull Request #878 · openedx/openedx-core

ufedaseyeuconsultant · 2026-10-16T20:40:48Z

Description

Adds a read endpoint that returns a competency's CompetencyCriteriaGroup tree (root, course-level group, leaf) and the CompetencyCriterion rows under it, as two flat lists for the client to rebuild:

GET /api/cbe/v1/competencies/<int:tag_id>/criteria-groups/

Studio's Manage & Apply Competencies flow needs this endpoint to render a competency's existing groups, criteria, and tagged subsections from persisted backend state, so the next write (adding or removing an association) acts on what the database holds rather than on state tracked in the client.

The caller must be authenticated and must pass oel_tagging.view_taxonomy on the tag's taxonomy. openedx-platform overrides that rule with its org-aware check. A caller who fails it gets a 403. An unknown tag, or a tag that is not on a CompetencyTaxonomy, is a 404.
Only groups whose whole ancestor chain is non-archived are returned, with the non-archived criteria under them, so every parent_id and group_id in the response names a returned row. Pruning happens in memory after one groups query; the criteria query then covers only the kept groups.
Groups are ordered by parent (roots first), then ordering, then id. Criteria are ordered by group, then id. The order is stable across databases.
An empty tree, where the tag exists but nothing has been authored under it yet, is a valid 200 with two empty arrays.

Public API additions, all marked UNSTABLE: get_competency_criteria_tree(tag), which takes the resolved Tag, and the CompetencyCriteriaTree dataclass it returns, both in openedx_learning.applets.cbe.api. The REST layer adds CompetencyCriteriaGroupSerializer and CompetencyCriterionReadSerializer in v1/serializers.py and CompetencyReadPermission in v1/permissions.py.

Which roles this affects: Course Authors and Platform Administrators, through Studio's competency-management UI (read-only in this PR); Developers, as a new API to build against (#672 and #683 consume it next).

Supporting information

Implements [BE] GET Competency Criteria & Groups #681.
Builds on [BE] Implement CBE core data models (CompetencyTaxonomy, criteria, learner status) #613 (CompetencyTaxonomy), [BE] Create CompetencyCriteria #665 (the CompetencyCriterion and CompetencyCriteriaGroup models and the create-criterion endpoint), and 716 archived field cc group models #857 (the archived columns), all already on main.
ADR: docs/openedx_learning/decisions/0002-competency-criteria-model.rst, Decision 5, for the indexes this read path relies on.

Testing instructions

Apply migrations: python manage.py migrate openedx_learning.
Create a competency taxonomy and tag, and attach at least one criterion under a course run, through POST /api/cbe/v1/competencies/<tag_id>/criteria/ or openedx_learning.api.associate_competency_criterion(...).
GET /api/cbe/v1/competencies/<tag_id>/criteria-groups/ as a user who can view the taxonomy. Expect 200 with:
- groups: the root (parent_id and course_key both null), the course-level group, and the leaf, each with the correct parent_id.
- criteria: the criterion, with group_id matching the leaf, and id, object_tag_id, object_id (the tagged subsection's usage key), rule_profile_id, rule_type_override, and rule_payload_override.
Set archived=True on the course-level group and repeat step 3. Expect the course-level group, the leaf under it, and the criterion to be absent; the root stays.
GET the same URL for a tag_id that does not exist, and for one that exists but is not on a CompetencyTaxonomy. Expect 404 in both cases.
GET as an anonymous user (expect 401) and as a signed-in user without view access to the taxonomy (expect 403).
Automated: pytest tests/openedx_learning/applets/cbe/ -o addopts="".

Other information

No backward-incompatible changes: this PR adds one endpoint and its API function, and no migrations.
Out of scope, tracked separately: write behavior on this resource ([BE] Delete Competency Criterion #674 and [BE] Build endpoint for removing a Competency Criteria Group #675 delete or archive a criterion or group), course-run scoping ([BE] Add filtering to GET Competency Criteria & Groups #682, built on top of this branch), and a deletable convenience flag, which is deferred until an ADR settles the mechanism.
Rebased version of feat: add GET competency criteria-groups endpoint alezconsultant/openedx-core#3

🤖 Generated with Claude Code

openedx-webhooks · 2026-10-16T20:40:56Z

Thanks for the pull request, @ufedaseyeuconsultant!

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.

tbain

Prior Unicon Review was performed here; alezconsultant#3

Light code review of this rebase + Codex review revealed no new issues and no substantive changes with regard to the previous review vs. the current active review against the updated target.

ormsbee

Thanks for this. Claude ran the CBE suite on 5ea5745 for me (344 tests, all passing) along with the probes I mention inline.

My main concern is the permission gate, with a few other minor concerns and questions inline. Once those are addressed, please feel free to squash and update the commit message, and I'll merge.

Thank you.

Claude drafted the first pass of this review and ran the CBE suite (344 tests) and the probes on 5ea5745 for me. The anonymous-request probe is written out inline so you can reproduce it.

ormsbee · 2026-10-26T16:00:57Z

+    CompetencyCriterionCreateView: a read has no target object to tag, so there's nothing for
+    that predicate to check here. has_object_permission only, no has_permission override: the
+    base class's always-True default is correct since the real check is object-level, run
+    manually by the view via check_object_permissions().


This reasoning skips authentication. BasePermission.has_permission() returning True means an anonymous request reaches get(), resolves the tag, and then can_view_taxonomy says anyone can view an enabled taxonomy, so check_object_permissions() passes for AnonymousUser as well. Claude ran this on the branch with the tag/course_run fixtures, one leaf, and one criterion:

APIClient().get(reverse("cbe:criteria-tree", kwargs={"tag_id": tag.id}))

ANON tree GET (enabled taxonomy): 200 {'groups': [{'id': 1, 'parent_id': None, 'name': 'Writing Poetry (root)', ...}, {'id': 2, 'parent_id': 1, 'course_key': 'course-v1:Org1+Python100+Fall2026', ...}, {'id': 3, 'parent_id': 2, ...}], 'criteria': [{'id': 1, 'group_id': 3, 'object_id': 'block-v1:Org1+Python100+Fall2026+type@sequential+block@s1', ...}]} ANON rule_profiles GET: 401 Authentication credentials were not provided. ANON criterion-create POST: 401 Authentication credentials were not provided.

So this is the one endpoint in the applet that hands course keys and subsection usage keys to a caller with no session at all. TaxonomyTagsView, the precedent #681 cites, sits behind a DjangoObjectPermissions subclass, and DjangoModelPermissions.has_permission() refuses an unauthenticated user before anything else runs. CompetencyRuleProfilePermissions right above gets that for free, and the two write views use IsAuthenticated explicitly.

Please either add IsAuthenticated to the view's permission_classes or give this class a has_permission() that checks request.user.is_authenticated, update this docstring to match, and add an anonymous test next to test_no_view_access_to_the_taxonomy_is_403.

Fixed: IsAuthenticated is on the view, the docstring now says so, and there is an anonymous test.

ormsbee · 2026-10-26T16:00:58Z

+
+    def has_object_permission(self, request, view, obj) -> bool:
+        """Return True if the requesting user may view the taxonomy that ``obj`` (a Tag) belongs to."""
+        return rules.has_perm("oel_tagging.view_tag", request.user, obj)


The other half of the problem: oel_tagging.view_tag isn't the rule that runs in Studio. openedx-platform replaces it with rules.always_allow (content_tagging/rules.py#L384), so once this is mounted under cms/urls.py this line is True for every user and every taxonomy, disabled ones included. test_no_view_access_to_the_taxonomy_is_403 passes only because this repo's own can_view_tag is what test_settings registers.

The rule platform does override with something org-aware is oel_tagging.view_taxonomy (#L377), which is why can_view_competency_rule_profile in cbe/rules.py asks for view_taxonomy rather than calling the predicate (its docstring spells this out). Please check oel_tagging.view_taxonomy against obj.taxonomy here instead; resolve_competency_tag() already guarantees it's set.

This seems like a longer term issue of having permissions overrides being defined in openedx-platform. I get that we basically have to for certain things today, but we can hopefully pull more of the real permissions logic into openedx-core over time.

Switched to oel_tagging.view_taxonomy on obj.taxonomy. Agreed on the longer-term point about moving the real permission logic into openedx-core.

ormsbee · 2026-10-26T16:00:58Z

+        CompetencyCriteriaGroup.objects.filter(tag_id=tag_id, archived=False).select_related("course")
+    )
+    criteria = list(
+        CompetencyCriterion.objects.filter(group__in=groups, archived=False).select_related("object_tag")


Question: should a read prune everything under an archived group, rather than filtering row by row? bulk_update_competency_criteria() treats an archived ancestor as closing the whole subtree ("An archived ancestor counts too"), and #675 archives a subtree as a unit, but this query only looks at each row's own flag. Claude archived just the course-level group on the branch and left its leaf and criterion alone, and the response was:

group 1 parent_id None group 3 parent_id 2 (2 is archived, and not in the response) criterion 1 group_id 3

So the client gets a group whose parent_id points at nothing, plus a criterion under it. I think the response should always be something the client can rebuild without special cases, which means walking down from the roots after the first query and dropping whatever isn't reachable (it's all in memory already, so no extra queries). If you're not doing this because we're leaning on #675's guarantee that subtrees are archived whole, that's okay with me too. I'm just reflexively paranoid about allowing illegal states to exist, even if it's meant to be a temporary state during implementation.

Done in memory after the first query, as you suggested; details in the top-level reply.

ormsbee · 2026-10-26T16:00:58Z

+    criteria: list[CompetencyCriterion]
+
+
+def get_competency_criteria_tree(tag_id: int) -> CompetencyCriteriaTree:


[question] Should this take the resolved Tag instead of an id? The view already has it from resolve_competency_tag(), and as written an unknown or non-competency tag_id from a Python-API caller returns CompetencyCriteriaTree(groups=[], criteria=[]), which is exactly what "nothing authored yet" looks like (Claude checked with get_competency_criteria_tree(999999)). associate_competency_criterion() resolves its own tag_id so an in-process caller gets Tag.DoesNotExist; this one is the odd one out. Taking the Tag gets the same guarantee without a second lookup. Not blocking.

ormsbee · 2026-10-26T16:00:58Z

+    behavior may change this function's shape without a deprecation cycle.
+    """
+    groups = list(
+        CompetencyCriteriaGroup.objects.filter(tag_id=tag_id, archived=False).select_related("course")


Nit: Please give both querysets an order_by() so the response is stable across databases. get_competency_rule_profiles() makes the same point in its docstring, and #682 will need it for pagination anyway. ("parent_id", "ordering", "id") for groups would also hand the client its siblings in evaluation order.

Added for both querysets, with groups in (parent_id, ordering, id) order and NULLs first.

ormsbee · 2026-10-26T16:00:58Z

+
+    class Meta:
+        model = CompetencyCriteriaGroup
+        fields = ["id", "parent_id", "tag_id", "course_key", "name", "ordering", "logic_operator", "archived"]


My understanding is that archived is always false in this response, since the API function filters on it, and the criterion serializer below leaves it out entirely. If that's correct, please drop it so a client doesn't learn to read a field that can never change.

ufedaseyeuconsultant · 2026-10-26T21:01:27Z

@ormsbee
Thanks for review.

Authentication. The view now lists IsAuthenticated before CompetencyReadPermission, the same as the two write views, so an anonymous request gets 401 before the tag is resolved. A test next to test_no_view_access_to_the_taxonomy_is_403 makes the anonymous request and asserts 401 with no tree in the body.
The rule that runs in Studio. The object check now asks oel_tagging.view_taxonomy on the tag's taxonomy, for the reason you gave and the one can_view_competency_rule_profile already documents. A test sets view_tag to always allow and view_taxonomy to always deny and asserts 403, so it fails if the check goes back to view_tag. The check also refuses a tag with no taxonomy, since has_perm(..., None) would fall back to a global rule. The can_view_competency_rule_profile docstring no longer claims this endpoint uses view_tag.
Pruning. Yes, the response is now always one tree. After the groups query, a walk down from the non-archived roots keeps only groups whose whole ancestor chain is non-archived, and the criteria query covers only those groups. It adds no query. It walks a parent-to-children map rather than relying on row order, because ordering by parent_id does not put a parent before its child once a group can be moved under a group with a larger id; a test re-parents a chain to pin that. Tests cover an archived root (nothing returned), an archived group two levels above active rows, and, over REST, that every parent_id and group_id in the response names a returned row.
Tag instead of id. get_competency_criteria_tree(tag) now takes the resolved Tag; the view passes the one it already has, so there is no second lookup, and an in-process caller cannot get an empty tree for a bad id.
data.py. CompetencyCriteriaTree moved there (frozen, keyword-only, slotted, like GradedObjectScore) and is still exported from openedx_learning.api.
Order. Groups are ordered by parent_id with NULLs first, then ordering, then id; criteria by group_id, then id. NULLs first is explicit because MySQL and PostgreSQL place NULLs differently by default, so the root comes first on every database.
archived on group rows. Dropped; a test asserts the exact set of keys on a group row.

Also from a final pass: the two read serializers and the view now carry the UNSTABLE note, the view docstring describes the pruning, and a test asserts the other HTTP methods return 405.

One thing worth knowing for Studio: under the platform's can_view_taxonomy, a taxonomy with no TaxonomyOrg rows is readable only by taxonomy admins.

ormsbee

Thank you for the update. I have two tiny docstring formatting suggestions, and a request to undo a previous request of mine (the data.py move).

Please also bring the PR description in line with the commit message: the scope note still describes the inherited #665 history, and the "Other information" section says this PR adds the archived column, which landed in #857. Then I'll merge.

Thank you.

Claude drafted this review and re-ran the CBE suite (357 tests), makemigrations --check, and the quality checks (pycodestyle, isort, pylint, mypy, import-linter) on 039bd9e for me, along with the anonymous-request and archived-group probes from last round, plus a check that a parent cycle with no root is dropped rather than looped on.

ormsbee · 2026-10-27T20:40:53Z

 from dataclasses import dataclass
 from decimal import Decimal

+from .models import CompetencyCriteriaGroup, CompetencyCriterion


Sorry, this is completely my fault. Please put this and CompetencyCriteriaTree back in the api.py module. I hadn't put it together that the dataclasses have models in them, and we've tried to keep data.py free of model dependencies. OEP-49 was written before we started passing models back through api.py on a regular basis, and maybe it needs some amending, but that's not something I should bog this PR down with.

ormsbee · 2026-10-27T20:40:53Z

+    group_id. An empty tree (the tag exists
+    but nothing has been authored under it yet) is a valid 200 with two empty arrays; a 404 is
+    reserved for tag resolution failure only.


Suggested change

group_id. An empty tree (the tag exists

but nothing has been authored under it yet) is a valid 200 with two empty arrays; a 404 is

reserved for tag resolution failure only.

group_id. An empty tree (the tag exists but nothing has been authored under it yet) is a

valid 200 with two empty arrays; a 404 is reserved for tag resolution failure only.

ormsbee · 2026-10-27T20:45:15Z

+    ``oel_tagging.view_taxonomy`` on the tag's taxonomy; a profile on a disabled taxonomy stays admin-only. Course- and
    organization-scoped profiles are refused to everyone until #645 decides who may see them.


Suggested change

``oel_tagging.view_taxonomy`` on the tag's taxonomy; a profile on a disabled taxonomy stays admin-only. Course- and

organization-scoped profiles are refused to everyone until #645 decides who may see them.

``oel_tagging.view_taxonomy`` on the tag's taxonomy; a profile on a disabled taxonomy stays

admin-only. Course- and organization-scoped profiles are refused to everyone until #645

decides who may see them.

Adds GET /api/cbe/v1/competencies/<tag_id>/criteria-groups/, which returns a competency's criteria tree as two flat lists, groups and criteria, for the client to rebuild. - The caller must be authenticated and must pass oel_tagging.view_taxonomy on the tag's taxonomy, the rule openedx-platform overrides with its org-aware check. An unknown or non-competency tag is a 404. - Only groups whose whole ancestor chain is non-archived are returned, with the non-archived criteria under them, so every parent_id and group_id in the response names a returned row. Pruning happens in memory after one groups query; the criteria query covers only the kept groups. - Groups are ordered by parent (roots first), ordering, then id, and criteria by group, then id, so the response is stable across databases. - get_competency_criteria_tree(tag) takes the resolved Tag and returns CompetencyCriteriaTree, both in api.py and both UNSTABLE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

openedx-webhooks added this to Contributions Oct 16, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Oct 16, 2026

tbain approved these changes Oct 19, 2026

View reviewed changes

mgwozdz-unicon requested a review from ormsbee October 20, 2026 13:57

alezconsultant mentioned this pull request Oct 20, 2026

feat: add blocked-courses endpoint for a competency alezconsultant/openedx-core#7

Closed

ufedaseyeuconsultant force-pushed the feat--681-get-competency-criteria-and-groups branch from 84b92cd to 653cd7b Compare October 22, 2026 15:11

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Oct 22, 2026

ufedaseyeuconsultant force-pushed the feat--681-get-competency-criteria-and-groups branch from 653cd7b to 5ea5745 Compare October 26, 2026 14:02

ormsbee requested changes Oct 26, 2026

View reviewed changes

ufedaseyeuconsultant force-pushed the feat--681-get-competency-criteria-and-groups branch from 5ea5745 to 039bd9e Compare October 26, 2026 21:00

ufedaseyeuconsultant requested a review from ormsbee October 26, 2026 21:01

ormsbee requested changes Oct 27, 2026

View reviewed changes

ufedaseyeuconsultant force-pushed the feat--681-get-competency-criteria-and-groups branch from 039bd9e to 968bdfe Compare October 27, 2026 22:27

ufedaseyeuconsultant requested a review from ormsbee October 27, 2026 22:29

ormsbee approved these changes Oct 27, 2026

View reviewed changes

ormsbee merged commit 2da991f into openedx:main Oct 27, 2026
7 checks passed

github-project-automation Bot moved this from Ready for Review to Done in Contributions Oct 27, 2026

alezconsultant mentioned this pull request Oct 28, 2026

Alezconsultant/796 blocked courses #915

Open

ormsbee mentioned this pull request Oct 29, 2026

feat: add course-run scoping to the criteria-groups endpoint (#682) #911

Merged

dvarenikqaconsultant mentioned this pull request Oct 30, 2026

[BE] GET Competency Criteria & Groups #681

Closed

		criteria: list[CompetencyCriterion]


		def get_competency_criteria_tree(tag_id: int) -> CompetencyCriteriaTree:

		``oel_tagging.view_taxonomy`` on the tag's taxonomy; a profile on a disabled taxonomy stays admin-only. Course- and
		organization-scoped profiles are refused to everyone until #645 decides who may see them.

Conversation

ufedaseyeuconsultant commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Other information

Uh oh!

openedx-webhooks commented Oct 16, 2026

Uh oh!

tbain left a comment

Choose a reason for hiding this comment

Uh oh!

ormsbee 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ufedaseyeuconsultant commented Oct 26, 2026

Uh oh!

ormsbee 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!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ufedaseyeuconsultant commented Oct 16, 2026 •

edited

Loading