From 968bdfeeecdc9326c07e5f56b6209d68a16e1ac2 Mon Sep 17 00:00:00 2001 From: ufedaseyeuconsultant Date: Mon, 26 Oct 2026 23:41:44 +0300 Subject: [PATCH] feat: add GET competency criteria-groups endpoint (#681) Adds GET /api/cbe/v1/competencies//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 --- src/openedx_learning/applets/cbe/api.py | 60 ++++ .../applets/cbe/rest_api/v1/permissions.py | 17 ++ .../applets/cbe/rest_api/v1/serializers.py | 46 ++- .../applets/cbe/rest_api/v1/urls.py | 5 + .../applets/cbe/rest_api/v1/views.py | 36 ++- src/openedx_learning/applets/cbe/rules.py | 5 +- .../openedx_learning/applets/cbe/test_api.py | 280 ++++++++++++++++++ .../applets/cbe/test_views.py | 246 +++++++++++++++ 8 files changed, 691 insertions(+), 4 deletions(-) diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index c4df8d3db..87552fee7 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -4,6 +4,7 @@ from __future__ import annotations import logging +from dataclasses import dataclass from datetime import datetime, timezone from decimal import Decimal from typing import Any, Sequence @@ -35,12 +36,14 @@ from .rule_payloads import evaluate_rule __all__ = [ + "CompetencyCriteriaTree", "CompetencyCriterionArchivedError", "GradedObjectScore", "associate_competency_criterion", "bulk_update_competency_criteria", "create_competency_criterion", "create_competency_taxonomy", + "get_competency_criteria_tree", "get_competency_rule_profiles", "is_competency_taxonomy", "record_graded_object_statuses", @@ -143,6 +146,63 @@ def resolve_competency_tag(tag_id: int) -> Tag: return tag +@dataclass(frozen=True, kw_only=True, slots=True) +class CompetencyCriteriaTree: + """ + One competency's non-archived CompetencyCriteriaGroup tree and CompetencyCriterion leaves. + + UNSTABLE: the criteria-authoring family is incomplete, so this shape may change without a + deprecation cycle. ``groups`` and ``criteria`` are flat lists: each group carries its + ``parent_id`` and each criterion its ``group_id``, and both always name a group in ``groups``. + """ + + groups: list[CompetencyCriteriaGroup] + criteria: list[CompetencyCriterion] + + +def get_competency_criteria_tree(tag: Tag) -> CompetencyCriteriaTree: + """ + Return the non-archived CompetencyCriteriaGroup tree and CompetencyCriterion leaves for ``tag``. + + A group is returned only if it and every ancestor up to its root are non-archived, and a + criterion only if it and its group are returned, so the result always forms one tree: no + returned ``parent_id`` or ``group_id`` names a row that was left out. Groups come back + root first, then ordered by ``(parent_id, ordering, id)``; criteria by ``(group_id, id)``. + + UNSTABLE: the criteria-authoring family is incomplete, so its group update and archive + behavior may change this function's shape without a deprecation cycle. + """ + groups = list( + CompetencyCriteriaGroup.objects.filter(tag=tag, archived=False) + .select_related("course") + .order_by(F("parent_id").asc(nulls_first=True), "ordering", "id") + ) + + # Walk down from the roots instead of relying on row order: ordering by parent_id does not + # guarantee a parent comes before its child once groups can be moved, since a parent may + # then have a larger id than its child. + children_by_parent_id: dict[int | None, list[int]] = {} + for group in groups: + children_by_parent_id.setdefault(group.parent_id, []).append(group.id) + kept_group_ids: set[int] = set() + pending = list(children_by_parent_id.get(None, [])) + while pending: + group_id = pending.pop() + kept_group_ids.add(group_id) + pending.extend(children_by_parent_id.get(group_id, [])) + + criteria = list( + CompetencyCriterion.objects.filter(group_id__in=kept_group_ids, archived=False) + .select_related("object_tag") + .order_by("group_id", "id") + ) + + return CompetencyCriteriaTree( + groups=[group for group in groups if group.id in kept_group_ids], + criteria=criteria, + ) + + def _create_leaf_group( tag: Tag, course_run: CourseRun, diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py index 2c3cf186f..fd221972c 100644 --- a/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py +++ b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py @@ -1,6 +1,7 @@ """ Permissions for the CBE REST API, v1. """ +from rest_framework import permissions from rest_framework.permissions import DjangoObjectPermissions @@ -17,3 +18,19 @@ class CompetencyRuleProfilePermissions(DjangoObjectPermissions): "OPTIONS": [], "HEAD": ["%(app_label)s.view_%(model_name)s"], } + + +class CompetencyReadPermission(permissions.BasePermission): + """ + A user may read a competency's criteria tree if they can view the tag's taxonomy. + + Authentication comes from ``IsAuthenticated`` on the view, so this class only does the + object-level check, which the view runs through ``check_object_permissions()``. That check + asks ``oel_tagging.view_taxonomy`` because openedx-platform overrides that rule with its + org-aware one and sets ``oel_tagging.view_tag`` to always allow. + """ + + 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.""" + # has_perm(..., None) would fall back to a global rule, so a tag without a taxonomy is refused. + return obj.taxonomy is not None and request.user.has_perm("oel_tagging.view_taxonomy", obj.taxonomy) diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py index 22e029e9c..0c700c1fa 100644 --- a/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py +++ b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py @@ -8,7 +8,7 @@ from opaque_keys.edx.keys import UsageKey from rest_framework import serializers -from ...models import CompetencyCriterion, CompetencyRuleProfile, LogicOperator +from ...models import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, LogicOperator class CompetencyRuleProfileSerializer(serializers.ModelSerializer): @@ -119,3 +119,47 @@ def validate(self, attrs: dict) -> dict: if unrecognized: raise serializers.ValidationError({key: "This field is not recognized." for key in unrecognized}) return attrs + + +class CompetencyCriteriaGroupSerializer(serializers.ModelSerializer): + """ + Read-only representation of a CompetencyCriteriaGroup, for the criteria-tree read endpoint. + + UNSTABLE: the criteria-authoring family is incomplete, so this shape may change without a + deprecation cycle. + + course_key uses a CharField with a dotted source instead of SlugRelatedField, since + CourseRun.course_key is an opaque-keys field that the JSON renderer can't serialize directly; + CharField.to_representation() calls str() on it instead, and returns None cleanly when + `course` itself is null. + """ + + course_key = serializers.CharField(source="course.course_key", read_only=True, allow_null=True) + + class Meta: + model = CompetencyCriteriaGroup + fields = ["id", "parent_id", "tag_id", "course_key", "name", "ordering", "logic_operator"] + + +class CompetencyCriterionReadSerializer(serializers.ModelSerializer): + """ + Read-only representation of a CompetencyCriterion, for the criteria-tree read endpoint. + + UNSTABLE: the criteria-authoring family is incomplete, so this shape may change without a + deprecation cycle. + + A separate serializer from CompetencyCriterionSerializer (#665's create-request serializer), + not an added field on it: that serializer's `object_id` is write_only, and giving it a dotted + source to double as this endpoint's output would silently break the create endpoint, since a + dotted source on a writable field changes where to_internal_value() places the value in + validated_data. + """ + + object_id = serializers.CharField(source="object_tag.object_id", read_only=True) + + class Meta: + model = CompetencyCriterion + fields = [ + "id", "group_id", "object_tag_id", "object_id", + "rule_profile_id", "rule_type_override", "rule_payload_override", + ] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/urls.py b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py index 0862262d5..658cab5e0 100644 --- a/src/openedx_learning/applets/cbe/rest_api/v1/urls.py +++ b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py @@ -22,4 +22,9 @@ views.CompetencyCriterionBulkUpdateView.as_view(), name="criterion-bulk-update", ), + path( + "competencies//criteria-groups/", + views.CompetencyCriteriaTreeView.as_view(), + name="criteria-tree", + ), ] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/views.py b/src/openedx_learning/applets/cbe/rest_api/v1/views.py index e576108b9..a232cfd83 100644 --- a/src/openedx_learning/applets/cbe/rest_api/v1/views.py +++ b/src/openedx_learning/applets/cbe/rest_api/v1/views.py @@ -24,14 +24,17 @@ CompetencyCriterionArchivedError, associate_competency_criterion, bulk_update_competency_criteria, + get_competency_criteria_tree, get_competency_rule_profiles, resolve_competency_tag, ) from ...models import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile from ..paginators import CompetencyRuleProfilePagination -from .permissions import CompetencyRuleProfilePermissions +from .permissions import CompetencyReadPermission, CompetencyRuleProfilePermissions from .serializers import ( + CompetencyCriteriaGroupSerializer, CompetencyCriterionBulkUpdateSerializer, + CompetencyCriterionReadSerializer, CompetencyCriterionSerializer, CompetencyRuleProfileSerializer, ) @@ -164,3 +167,34 @@ def patch(self, request, group_id: int): except CompetencyCriterionArchivedError as exc: raise CompetencyCriteriaConflict(str(exc)) from exc return Response(CompetencyCriterionSerializer(criteria, many=True).data, status=status.HTTP_200_OK) + + +class CompetencyCriteriaTreeView(generics.GenericAPIView): + """ + GET-only. Return a competency's CompetencyCriteriaGroup tree and CompetencyCriterion leaves. + + UNSTABLE: the criteria-authoring family is incomplete, so this response shape may change + without a deprecation cycle. + + Only groups whose whole ancestor chain is non-archived are returned, with their non-archived + criteria; see ``get_competency_criteria_tree``. Returned as two flat lists (groups, criteria) + for client-side reconstruction: each group carries its parent_id, and each criterion its + 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. + """ + + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = [IsAuthenticated, CompetencyReadPermission] + + def get(self, request, tag_id): + """Return the competency's criteria tree; 404 if ``tag_id`` is not a competency tag.""" + try: + tag = resolve_competency_tag(tag_id) + except Tag.DoesNotExist as exc: + raise NotFound(str(exc)) from exc + self.check_object_permissions(request, tag) + tree = get_competency_criteria_tree(tag) + return Response({ + "groups": CompetencyCriteriaGroupSerializer(tree.groups, many=True).data, + "criteria": CompetencyCriterionReadSerializer(tree.criteria, many=True).data, + }) diff --git a/src/openedx_learning/applets/cbe/rules.py b/src/openedx_learning/applets/cbe/rules.py index f5d536bf0..ba2932acc 100644 --- a/src/openedx_learning/applets/cbe/rules.py +++ b/src/openedx_learning/applets/cbe/rules.py @@ -28,8 +28,9 @@ def can_view_competency_rule_profile( Course authors, not only platform staff, need to read rule profiles, so the collection and the system default are open to anyone signed in. A taxonomy-scoped profile is as visible as its taxonomy, which is the gate the criteria tree endpoint uses too, through - ``oel_tagging.view_tag``; 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. The signed-in requirement is added where the permission is registered, below, so ``has_perm`` refuses an anonymous caller whatever view sits in front of it. This predicate alone does not diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py index 8f014fa9d..d1ef776b9 100644 --- a/tests/openedx_learning/applets/cbe/test_api.py +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -21,6 +21,7 @@ bulk_update_competency_criteria, create_competency_criterion, create_competency_taxonomy, + get_competency_criteria_tree, get_competency_rule_profiles, is_competency_taxonomy, resolve_supplied_leaf_group, @@ -1015,3 +1016,282 @@ def _fail_on_second_save(self, *args, **kwargs) -> None: monkeypatch.undo() assert len(saves) == 2 assert snapshot(batch) == before + + +# ============================================================================================== +# get_competency_criteria_tree +# ============================================================================================== + + +def test_get_competency_criteria_tree_is_empty_when_no_groups_exist_yet(tag: Tag) -> None: + """A competency with no CompetencyCriteriaGroup rows at all returns two empty lists.""" + tree = get_competency_criteria_tree(tag) + + assert not tree.groups + assert not tree.criteria + + +def test_get_competency_criteria_tree_returns_the_full_hierarchy_and_its_leaf_criterion( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A root, its course-level child, and a leaf under it, plus the leaf's one criterion, all come back.""" + leaf = _create_leaf_group(tag, course_run) + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + criterion = CompetencyCriterion.objects.create( + group=leaf, object_tag=object_tag, rule_profile=default_rule_profile, + ) + + tree = get_competency_criteria_tree(tag) + + returned_groups_by_id = {group.id: group for group in tree.groups} + assert set(returned_groups_by_id) == {root.id, course_level.id, leaf.id} + assert returned_groups_by_id[root.id].parent_id is None + assert returned_groups_by_id[root.id].course_id is None + assert returned_groups_by_id[course_level.id].parent_id == root.id + assert returned_groups_by_id[leaf.id].parent_id == course_level.id + assert [c.id for c in tree.criteria] == [criterion.id] + assert tree.criteria[0].group_id == leaf.id + + +def test_get_competency_criteria_tree_excludes_an_archived_group_and_its_archived_criterion( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """An archived leaf group and an archived criterion under it are both left out.""" + live_leaf = _create_leaf_group(tag, course_run) + live_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "live"), taxonomy=tag.taxonomy, tag=tag, + ) + live_criterion = CompetencyCriterion.objects.create( + group=live_leaf, object_tag=live_object_tag, rule_profile=default_rule_profile, + ) + archived_leaf = _create_leaf_group(tag, course_run) + archived_leaf.archived = True + archived_leaf.save() + archived_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "archived"), taxonomy=tag.taxonomy, tag=tag, + ) + archived_criterion = CompetencyCriterion.objects.create( + group=archived_leaf, object_tag=archived_object_tag, rule_profile=default_rule_profile, archived=True, + ) + + tree = get_competency_criteria_tree(tag) + + returned_group_ids = {group.id for group in tree.groups} + returned_criterion_ids = {criterion.id for criterion in tree.criteria} + assert live_leaf.id in returned_group_ids + assert archived_leaf.id not in returned_group_ids + assert live_criterion.id in returned_criterion_ids + assert archived_criterion.id not in returned_criterion_ids + + +def test_get_competency_criteria_tree_combines_groups_and_criteria_from_every_course_unscoped( + tag: Tag, course_run: CourseRun, organization: Organization, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Two course-level groups under one root, each with its own leaf and criterion, both come back together.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + leaf1 = _create_leaf_group(tag, course_run) + leaf2 = _create_leaf_group(tag, other_course_run) + object_tag1 = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + object_tag2 = ObjectTag.objects.create( + object_id=usage_key(other_course_run, "p1"), taxonomy=tag.taxonomy, tag=tag, + ) + criterion1 = CompetencyCriterion.objects.create( + group=leaf1, object_tag=object_tag1, rule_profile=default_rule_profile, + ) + criterion2 = CompetencyCriterion.objects.create( + group=leaf2, object_tag=object_tag2, rule_profile=default_rule_profile, + ) + + course_level1 = leaf1.parent + course_level2 = leaf2.parent + assert course_level1 is not None + assert course_level2 is not None + root1 = course_level1.parent + root2 = course_level2.parent + assert root1 is not None + assert root2 is not None + + tree = get_competency_criteria_tree(tag) + + assert course_level1.id != course_level2.id, "the two leaves should sit under different course-level groups" + assert root1.id == root2.id, "both course-level groups should share one root" + assert {group.id for group in tree.groups} == { + root1.id, course_level1.id, leaf1.id, course_level2.id, leaf2.id, + } + assert {criterion.id for criterion in tree.criteria} == {criterion1.id, criterion2.id} + + +def test_get_competency_criteria_tree_prunes_rows_under_an_archived_ancestor( + tag: Tag, course_run: CourseRun, organization: Organization, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Archiving only a course-level group removes its still-active leaf and criterion, so no parent_id dangles.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + pruned_leaf = _create_leaf_group(tag, course_run) + pruned_course_level = pruned_leaf.parent + assert pruned_course_level is not None + pruned_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "pruned"), taxonomy=tag.taxonomy, tag=tag, + ) + CompetencyCriterion.objects.create( + group=pruned_leaf, object_tag=pruned_object_tag, rule_profile=default_rule_profile, + ) + kept_leaf = _create_leaf_group(tag, other_course_run) + kept_object_tag = ObjectTag.objects.create( + object_id=usage_key(other_course_run, "kept"), taxonomy=tag.taxonomy, tag=tag, + ) + kept_criterion = CompetencyCriterion.objects.create( + group=kept_leaf, object_tag=kept_object_tag, rule_profile=default_rule_profile, + ) + kept_course_level = kept_leaf.parent + assert kept_course_level is not None + root = kept_course_level.parent + assert root is not None + pruned_course_level.archived = True + pruned_course_level.save() + + tree = get_competency_criteria_tree(tag) + + assert {group.id for group in tree.groups} == {root.id, kept_course_level.id, kept_leaf.id} + assert [criterion.id for criterion in tree.criteria] == [kept_criterion.id] + + +def test_get_competency_criteria_tree_returns_nothing_under_an_archived_root( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """An archived root hides its still-active course-level group, leaf, and criterion.""" + leaf = _create_leaf_group(tag, course_run) + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + CompetencyCriterion.objects.create(group=leaf, object_tag=object_tag, rule_profile=default_rule_profile) + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + root.archived = True + root.save() + + tree = get_competency_criteria_tree(tag) + + assert not tree.groups + assert not tree.criteria + + +def test_get_competency_criteria_tree_prunes_two_levels_below_an_archived_group( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Archiving a course-level group also hides an active leaf below it and that leaf's active child group.""" + leaf = _create_leaf_group(tag, course_run) + child_group = CompetencyCriteriaGroup.objects.create(tag=tag, parent=leaf, course_id=leaf.course_id) + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + CompetencyCriterion.objects.create(group=child_group, object_tag=object_tag, rule_profile=default_rule_profile) + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + course_level.archived = True + course_level.save() + + tree = get_competency_criteria_tree(tag) + + assert {group.id for group in tree.groups} == {root.id} + assert not tree.criteria + + +def test_get_competency_criteria_tree_keeps_a_group_moved_under_a_later_created_parent( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A group under a parent with a larger id, itself under a later group, is kept: pruning ignores id order.""" + leaf = _create_leaf_group(tag, course_run) + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + group_a = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root) + group_b = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root) + group_c = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root) + # A sits under B and B under C, so ordering by parent_id puts A before B: a single pass in + # that order would judge A before its parent B was kept. + group_b.parent = group_c + group_b.save() + group_a.parent = group_b + group_a.save() + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + criterion = CompetencyCriterion.objects.create( + group=group_a, object_tag=object_tag, rule_profile=default_rule_profile, + ) + + tree = get_competency_criteria_tree(tag) + + assert {group_a.id, group_b.id, group_c.id} <= {group.id for group in tree.groups} + assert [c.id for c in tree.criteria] == [criterion.id] + + +def test_get_competency_criteria_tree_orders_criteria_by_group_and_id( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Criteria come back by ``(group_id, id)`` whatever order they were created in.""" + first_group = _create_leaf_group(tag, course_run) + second_group = _create_leaf_group(tag, course_run) + created = [] + for group, name in ((second_group, "a"), (first_group, "b"), (second_group, "c"), (first_group, "d")): + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, name), taxonomy=tag.taxonomy, tag=tag) + created.append( + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + ) + + tree = get_competency_criteria_tree(tag) + + expected = sorted(created, key=lambda c: (c.group_id, c.id)) + assert [c.id for c in tree.criteria] == [c.id for c in expected] + assert [c.group_id for c in tree.criteria] == [first_group.id, first_group.id, second_group.id, second_group.id] + + +def test_get_competency_criteria_tree_orders_groups_by_parent_ordering_and_id( + tag: Tag, course_run: CourseRun, +) -> None: + """The root comes first, then siblings under one parent by ``ordering`` and then by ``id``.""" + leaf = _create_leaf_group(tag, course_run) + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + # Created out of ordering sequence; two share an ordering value so id breaks the tie. + late = CompetencyCriteriaGroup.objects.create(tag=tag, parent=course_level, ordering=5) + early = CompetencyCriteriaGroup.objects.create(tag=tag, parent=course_level, ordering=1) + tied_first = CompetencyCriteriaGroup.objects.create(tag=tag, parent=course_level, ordering=3) + tied_second = CompetencyCriteriaGroup.objects.create(tag=tag, parent=course_level, ordering=3) + leaf.ordering = 4 + leaf.save() + + tree = get_competency_criteria_tree(tag) + + assert tree.groups[0].id == root.id + siblings = [group.id for group in tree.groups if group.parent_id == course_level.id] + assert siblings == [early.id, tied_first.id, tied_second.id, leaf.id, late.id] + + +def test_get_competency_criteria_tree_costs_a_bounded_number_of_queries( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, django_assert_num_queries, +) -> None: + """ + Reading several groups and criteria costs a fixed two queries, not one per row. + + Pins the select_related("course")/select_related("object_tag") calls in + get_competency_criteria_tree(): dropping either would still pass every other test in this + module (the rows returned would be identical), since select_related only changes how many + queries fetch them, not which rows come back. + """ + leaf = _create_leaf_group(tag, course_run) + for i in range(3): + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, f"p{i}"), taxonomy=tag.taxonomy, tag=tag) + CompetencyCriterion.objects.create(group=leaf, object_tag=object_tag, rule_profile=default_rule_profile) + + with django_assert_num_queries(2): + tree = get_competency_criteria_tree(tag) + for group in tree.groups: + _ = group.course # accessing the select_related'd relation must not add a query + for criterion in tree.criteria: + _ = criterion.object_tag diff --git a/tests/openedx_learning/applets/cbe/test_views.py b/tests/openedx_learning/applets/cbe/test_views.py index bc1798e35..0320b1fea 100644 --- a/tests/openedx_learning/applets/cbe/test_views.py +++ b/tests/openedx_learning/applets/cbe/test_views.py @@ -1098,3 +1098,249 @@ def test_bulk_update_offers_only_patch(method: str, user_client: APIClient, leaf response = getattr(user_client, method)(bulk_update_url(leaf.id), {}, format="json") assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + +# ============================================================================================== +# CompetencyCriteriaTreeView (#681) +# ============================================================================================== + + +def criteria_tree_url(tag_id: int) -> str: + """Return the criteria-tree endpoint's path for `tag_id`, resolved through the URL name.""" + return reverse("cbe:criteria-tree", kwargs={"tag_id": tag_id}) + + +def test_full_tree_response_shape( + user_client: APIClient, + tag: Tag, + course_run: CourseRun, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """A permitted caller reads the full tree: every group and the leaf's criterion, complete enough to act on.""" + leaf = _create_leaf_group(tag, course_run) + object_id = usage_key(course_run, "p1") + object_tag = ObjectTag.objects.create(object_id=object_id, taxonomy=tag.taxonomy, tag=tag) + criterion = CompetencyCriterion.objects.create( + group=leaf, object_tag=object_tag, rule_profile=default_rule_profile, + ) + + course_level = leaf.parent + assert course_level is not None + root = course_level.parent + assert root is not None + + response = user_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_200_OK + groups_by_id = {group["id"]: group for group in response.data["groups"]} + assert set(groups_by_id) == {leaf.id, course_level.id, root.id} + assert set(groups_by_id[leaf.id]) == { + "id", "parent_id", "tag_id", "course_key", "name", "ordering", "logic_operator", + } + assert groups_by_id[root.id]["parent_id"] is None + assert groups_by_id[root.id]["course_key"] is None + assert groups_by_id[course_level.id]["parent_id"] == root.id + assert groups_by_id[leaf.id]["parent_id"] == course_level.id + assert len(response.data["criteria"]) == 1 + criterion_data = response.data["criteria"][0] + assert criterion_data["id"] == criterion.id + assert criterion_data["group_id"] == leaf.id + assert criterion_data["object_tag_id"] == object_tag.id + assert criterion_data["object_id"] == object_id + assert criterion_data["rule_profile_id"] == default_rule_profile.id + assert criterion_data["rule_type_override"] is None + assert criterion_data["rule_payload_override"] is None + + +def test_full_tree_combines_multiple_courses_in_one_unscoped_response( + user_client: APIClient, + tag: Tag, + course_run: CourseRun, + organization: Organization, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + A GET request for one competency returns every course's groups and criteria together, + with no course_id or date-window scoping parameter to narrow the request. + """ + other_course_run = make_course_run(organization, "Python200", "Fall2026") + leaf1 = _create_leaf_group(tag, course_run) + leaf2 = _create_leaf_group(tag, other_course_run) + object_tag1 = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + object_tag2 = ObjectTag.objects.create( + object_id=usage_key(other_course_run, "p1"), taxonomy=tag.taxonomy, tag=tag, + ) + criterion1 = CompetencyCriterion.objects.create( + group=leaf1, object_tag=object_tag1, rule_profile=default_rule_profile, + ) + criterion2 = CompetencyCriterion.objects.create( + group=leaf2, object_tag=object_tag2, rule_profile=default_rule_profile, + ) + course_level1 = leaf1.parent + course_level2 = leaf2.parent + assert course_level1 is not None + assert course_level2 is not None + root = course_level1.parent + assert root is not None + assert course_level2.parent_id == root.id + + response = user_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_200_OK + returned_group_ids = {group["id"] for group in response.data["groups"]} + assert returned_group_ids == {root.id, course_level1.id, leaf1.id, course_level2.id, leaf2.id} + assert {c["id"] for c in response.data["criteria"]} == {criterion1.id, criterion2.id} + + +def test_tree_response_never_references_a_group_it_left_out( + user_client: APIClient, + tag: Tag, + course_run: CourseRun, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """With only a course-level group archived, every parent_id and group_id still names a returned group.""" + leaf = _create_leaf_group(tag, course_run) + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + CompetencyCriterion.objects.create(group=leaf, object_tag=object_tag, rule_profile=default_rule_profile) + sibling_leaf = _create_leaf_group(tag, course_run) + sibling_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "p2"), taxonomy=tag.taxonomy, tag=tag, + ) + kept_criterion = CompetencyCriterion.objects.create( + group=sibling_leaf, object_tag=sibling_object_tag, rule_profile=default_rule_profile, + ) + course_level = leaf.parent + assert course_level is not None + # Both leaves share one course-level group, so detach the sibling under a kept parent. + kept_course_level = sibling_leaf.parent + assert kept_course_level is not None + assert kept_course_level.id == course_level.id + root = course_level.parent + assert root is not None + sibling_leaf.parent = root + sibling_leaf.save() + course_level.archived = True + course_level.save() + + response = user_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_200_OK + assert [c["id"] for c in response.data["criteria"]] == [kept_criterion.id] + returned_ids = {group["id"] for group in response.data["groups"]} + assert leaf.id not in returned_ids + assert all(group["parent_id"] is None or group["parent_id"] in returned_ids for group in response.data["groups"]) + assert all(criterion["group_id"] in returned_ids for criterion in response.data["criteria"]) + + +@pytest.mark.parametrize("method", ["post", "put", "patch", "delete"]) +def test_criteria_tree_offers_only_get(method: str, user_client: APIClient, tag: Tag) -> None: + """Every method other than GET is refused as not allowed.""" + response = getattr(user_client, method)(criteria_tree_url(tag.id), {}, format="json") + + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED + + +def test_empty_tree_is_200_with_two_empty_arrays(user_client: APIClient, tag: Tag) -> None: + """A tag with no CompetencyCriteriaGroup rows yet is a valid 200 with two empty arrays, not a 404.""" + response = user_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_200_OK + assert response.data["groups"] == [] + assert response.data["criteria"] == [] + + +def test_archived_group_and_criterion_are_excluded( + user_client: APIClient, + tag: Tag, + course_run: CourseRun, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """An archived leaf group and an archived criterion under it are left out of the response.""" + live_leaf = _create_leaf_group(tag, course_run) + live_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "live"), taxonomy=tag.taxonomy, tag=tag, + ) + live_criterion = CompetencyCriterion.objects.create( + group=live_leaf, object_tag=live_object_tag, rule_profile=default_rule_profile, + ) + archived_leaf = _create_leaf_group(tag, course_run) + archived_leaf.archived = True + archived_leaf.save() + archived_object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "archived"), taxonomy=tag.taxonomy, tag=tag, + ) + CompetencyCriterion.objects.create( + group=archived_leaf, object_tag=archived_object_tag, rule_profile=default_rule_profile, archived=True, + ) + + response = user_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_200_OK + returned_group_ids = {group["id"] for group in response.data["groups"]} + assert live_leaf.id in returned_group_ids + assert archived_leaf.id not in returned_group_ids + assert [c["id"] for c in response.data["criteria"]] == [live_criterion.id] + + +def test_unknown_tag_id_404s(user_client: APIClient) -> None: + """An unresolvable tag_id 404s.""" + response = user_client.get(criteria_tree_url(999999)) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_tag_not_on_a_competency_taxonomy_404s(user_client: APIClient) -> None: + """A tag_id that resolves to a real Tag, but not one on a CompetencyTaxonomy, 404s.""" + plain_taxonomy = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Not A Competency") + + response = user_client.get(criteria_tree_url(plain_tag.id)) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_no_view_access_to_the_taxonomy_is_403(api_client: APIClient, user, tag: Tag) -> None: + """A non-staff caller may not read the tree of a competency on a disabled taxonomy.""" + taxonomy = tag.taxonomy + assert taxonomy is not None + taxonomy.enabled = False + taxonomy.save() + api_client.force_authenticate(user=user) + + response = api_client.get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_anonymous_request_is_401_and_discloses_nothing( + tag: Tag, + course_run: CourseRun, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """An unauthenticated caller gets 401 and none of the tree, even for a competency with a criterion.""" + leaf = _create_leaf_group(tag, course_run) + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, "p1"), taxonomy=tag.taxonomy, tag=tag) + CompetencyCriterion.objects.create(group=leaf, object_tag=object_tag, rule_profile=default_rule_profile) + + response = APIClient().get(criteria_tree_url(tag.id)) + + assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert "groups" not in response.data + assert "criteria" not in response.data + + +def test_view_checks_the_view_taxonomy_rule_not_view_tag(user_client: APIClient, tag: Tag) -> None: + """The object check follows oel_tagging.view_taxonomy, which openedx-platform overrides, not view_tag.""" + view_tag = "oel_tagging.view_tag" + view_taxonomy = "oel_tagging.view_taxonomy" + original_view_tag = rules.permissions.permissions[view_tag] + original_view_taxonomy = rules.permissions.permissions[view_taxonomy] + rules.set_perm(view_tag, rules.always_allow) + rules.set_perm(view_taxonomy, rules.always_deny) + try: + response = user_client.get(criteria_tree_url(tag.id)) + finally: + rules.set_perm(view_tag, original_view_tag) + rules.set_perm(view_taxonomy, original_view_taxonomy) + + assert response.status_code == status.HTTP_403_FORBIDDEN