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

src/openedx_catalog/api_impl/courses.py

@@ -3,6 +3,7 @@
 """
 
 import logging
+from collections.abc import Iterable
 from typing import overload
 
 from opaque_keys.edx.keys import CourseKey
@@ -19,6 +20,7 @@
     "update_catalog_course",
     "delete_catalog_course",
     "get_course_run",
+    "get_course_run_ids",
     "sync_course_run_details",
     "create_course_run_for_modulestore_course_with",
     "delete_course_run",
@@ -116,6 +118,26 @@ def get_course_run(course_key: CourseKey) -> CourseRun:
     return CourseRun.objects.get(course_key__exact=course_key)
 
 
+def get_course_run_ids(course_keys: Iterable[CourseKey]) -> dict[CourseKey, int]:
+    """
+    Return a {course_key: CourseRun.id} mapping for the keys that resolve to a CourseRun.
+
+    A key with no matching CourseRun is absent from the result.
+
+    ⚠️ Does not check permissions or visibility rules.
+    """
+    course_keys = list(course_keys)
+    if not course_keys:
+        return {}
+    # course_key's accessor is typed CourseKey | None regardless of null=False (opaque_keys'
+    # own stub), so filter here instead of asserting past mypy.
+    return {
+        key: run_id
+        for key, run_id in CourseRun.objects.filter(course_key__in=course_keys).values_list("course_key", "id")
+        if key is not None
+    }
+
+
 def sync_course_run_details(
     course_key: CourseKey,
     *,

src/openedx_learning/applets/cbe/api.py

@@ -15,9 +15,9 @@
 from django.db.transaction import TransactionManagementError
 from django.utils.translation import gettext_lazy as _
 from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
 
-from openedx_catalog.api import get_course_run
+from openedx_catalog.api import get_course_run, get_course_run_ids
 from openedx_catalog.models_api import CourseRun
 from openedx_tagging.api import create_taxonomy, get_ancestor_tags, get_descendant_tags, get_object_tags, tag_object
 from openedx_tagging.models import ObjectTag, Tag, Taxonomy
@@ -167,52 +167,110 @@ class CompetencyCriteriaTree:
     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``.
+
+    ``criteria_count`` is ``len(criteria)``. ``total_criteria_count`` counts the criteria under
+    every reachable group before any course scoping.
     """
 
     groups: list[CompetencyCriteriaGroup]
     criteria: list[CompetencyCriterion]
+    criteria_count: int
+    total_criteria_count: int
 
 
-def get_competency_criteria_tree(tag: Tag) -> CompetencyCriteriaTree:
+def get_competency_criteria_tree(tag: Tag, course_keys: list[CourseKey] | None = None) -> 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.
+
+    A group is reachable only if it and every ancestor up to its root are non-archived. A
+    criterion is returned only if it is non-archived and its group is returned, so the result
+    always forms one tree: no returned ``parent_id`` or ``group_id`` names a row that was left out.
+
+    ``course_keys`` scopes the result by course run. A course run's groups are its course-level
+    group (the group whose ``course`` is that run) and every group nested below it, at any depth.
+    The groups above any course are the ones not nested in any course-level group.
+
+    - ``None`` (the default) returns every reachable group.
+    - A list returns the groups above any course plus the groups of the named course runs.
+      ``[]`` returns only the groups above any course.
+    - A key that matches no course run is dropped, and a repeated key counts once, at its
+      first position. A list in which no key resolves returns only the groups above any course.
+
+    Groups come back with the groups above any course first, then the named courses in the order
+    requested, and within each of those by ``(parent_id, ordering, id)`` with root-level rows first.
+    Criteria are ordered by their group's position in ``groups``, then ``id``.
+
+    ``criteria_count`` is ``len(criteria)``. ``total_criteria_count`` is the number of criteria
+    under every reachable group before ``course_keys`` is applied, so a caller scoped down to
+    zero criteria can still tell the competency is not empty.
     """
     groups = list(
         CompetencyCriteriaGroup.objects.filter(tag=tag, archived=False)
         .select_related("course")
         .order_by(F("parent_id").asc(nulls_first=True), "ordering", "id")
     )
+    if not groups:
+        return CompetencyCriteriaTree(groups=[], criteria=[], criteria_count=0, total_criteria_count=0)
 
     # Walk down from the roots instead of relying on row order. A group's parent is fixed once
     # saved, but a QuerySet.update() can still give a group a parent with a larger id, and
     # ordering by parent_id would then visit that child before its parent.
-    children_by_parent_id: dict[int | None, list[int]] = {}
+    groups_by_parent_id: dict[int | None, list[CompetencyCriteriaGroup]] = {}
     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, []))
+        groups_by_parent_id.setdefault(group.parent_id, []).append(group)
+    # Maps each reachable group id to the course run id of its branch, or None above any course.
+    branch_course_ids: dict[int, int | None] = {}
+    pending = [(group, group.course_id) for group in groups_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, []))
-
+        group, branch_course_id = pending.pop()
+        branch_course_ids[group.id] = branch_course_id
+        for child in groups_by_parent_id.get(group.id, []):
+            pending.append((child, branch_course_id if branch_course_id is not None else child.course_id))
+    reachable_groups = [group for group in groups if group.id in branch_course_ids]
+
+    if course_keys is None:
+        scoped_groups = reachable_groups
+    else:
+        run_ids_by_key = get_course_run_ids(course_keys)
+        rank_by_run_id = {
+            run_ids_by_key[key]: rank
+            for rank, key in enumerate((key for key in dict.fromkeys(course_keys) if key in run_ids_by_key), start=1)
+        }
+        # Groups above any course rank 0, so a scoped tree never orphans a child from its parent, and
+        # a branch ranks by its course. A branch whose course was not requested is left out.
+        # sorted() is stable, so each rank keeps the query order.
+        ranked_groups = []
+        for group in reachable_groups:
+            group_course_id = branch_course_ids[group.id]
+            if group_course_id is None:
+                ranked_groups.append((0, group))
+            elif group_course_id in rank_by_run_id:
+                ranked_groups.append((rank_by_run_id[group_course_id], group))
+        scoped_groups = [group for _, group in sorted(ranked_groups, key=lambda ranked: ranked[0])]
+
+    position_by_group_id = {group.id: position for position, group in enumerate(scoped_groups)}
     criteria = list(
-        CompetencyCriterion.objects.filter(group_id__in=kept_group_ids, archived=False)
+        CompetencyCriterion.objects.filter(group_id__in=position_by_group_id.keys(), archived=False)
         .select_related("object_tag")
         .order_by("group_id", "id")
     )
+    # sort() is stable, so criteria of one group keep their id order.
+    criteria.sort(key=lambda criterion: position_by_group_id[criterion.group_id])
+    if course_keys is None:
+        total_criteria_count = len(criteria)
+    else:
+        total_criteria_count = CompetencyCriterion.objects.filter(
+            group_id__in=branch_course_ids.keys(), archived=False,
+        ).count()
 
     return CompetencyCriteriaTree(
-        groups=[group for group in groups if group.id in kept_group_ids],
+        groups=scoped_groups,
         criteria=criteria,
+        criteria_count=len(criteria),
+        total_criteria_count=total_criteria_count,
     )
 
 

src/openedx_learning/applets/cbe/rest_api/v1/serializers.py

@@ -5,7 +5,7 @@
 
 from django.utils.translation import gettext_lazy as _
 from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import UsageKey
+from opaque_keys.edx.keys import CourseKey, UsageKey
 from rest_framework import serializers
 
 from ...models import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, LogicOperator
@@ -173,6 +173,46 @@ def validate(self, attrs: dict) -> dict:
         return attrs
 
 
+MAX_COURSE_KEYS = 100
+
+
+class CompetencyCriteriaQueryParamsSerializer(serializers.Serializer):
+    """Query params for the criteria-tree read endpoint."""
+
+    course_keys = serializers.CharField(required=False, allow_blank=True)
+
+    def validate_course_keys(self, value: str) -> list[CourseKey]:
+        """
+        Parse a comma-separated course_keys value into a deduplicated list of CourseKey.
+
+        Stray separators (trailing, doubled, or all-commas) are ignored, not rejected. Raises
+        a validation error if more than MAX_COURSE_KEYS entries remain after splitting, checked
+        before dedup so repeating a key can't dodge the cap, or if any entry isn't a valid key.
+        """
+        entries = [entry for entry in (entry.strip() for entry in value.split(",")) if entry]
+        if len(entries) > MAX_COURSE_KEYS:
+            raise serializers.ValidationError(
+                _("No more than %(max)s course_keys may be requested at once.") % {"max": MAX_COURSE_KEYS}
+            )
+        parsed = []
+        for entry in entries:
+            try:
+                parsed.append(CourseKey.from_string(entry))
+            except InvalidKeyError as exc:
+                raise serializers.ValidationError(
+                    _("'%(entry)s' is not a valid course key.") % {"entry": entry}
+                ) from exc
+        return list(dict.fromkeys(parsed))
+
+    def create(self, validated_data):
+        """Not supported: this serializer only validates query parameters."""
+        raise NotImplementedError
+
+    def update(self, instance, validated_data):
+        """Not supported: this serializer only validates query parameters."""
+        raise NotImplementedError
+
+
 class CompetencyCriterionReadSerializer(serializers.ModelSerializer):
     """
     Read-only representation of a CompetencyCriterion, for the criteria-tree read endpoint.

src/openedx_learning/applets/cbe/rest_api/v1/views.py

@@ -37,6 +37,7 @@
 from .serializers import (
     CompetencyCriteriaGroupSerializer,
     CompetencyCriteriaGroupUpdateSerializer,
+    CompetencyCriteriaQueryParamsSerializer,
     CompetencyCriterionBulkUpdateSerializer,
     CompetencyCriterionReadSerializer,
     CompetencyCriterionSerializer,
@@ -185,6 +186,17 @@ class CompetencyCriteriaTreeView(generics.GenericAPIView):
     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.
+
+    The optional ``course_keys`` query parameter (comma-separated course keys) scopes the tree by
+    course run. A course run's groups are its course-level group and every group nested below it.
+    Omitting the parameter returns every reachable group. Supplying it returns the groups above
+    any course plus the groups of the named runs, ordered with the groups above any course first
+    and then by the order requested; keys that match no run are ignored, and an empty value
+    returns only the groups above any course. The parameter may be supplied once.
+
+    ``criteria_count`` is the number of criteria returned. ``total_criteria_count`` is the number
+    under every reachable group regardless of ``course_keys``, so a client scoped down to zero
+    criteria can still tell the competency is not empty.
     """
 
     authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
@@ -197,10 +209,17 @@ def get(self, request, 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)
+        if len(request.query_params.getlist("course_keys")) > 1:
+            raise DRFValidationError({"course_keys": _("Supply course_keys once, as a comma-separated list.")})
+        query_params = CompetencyCriteriaQueryParamsSerializer(data=request.query_params.dict())
+        query_params.is_valid(raise_exception=True)
+        course_keys = query_params.validated_data.get("course_keys")
+        tree = get_competency_criteria_tree(tag, course_keys)
         return Response({
             "groups": CompetencyCriteriaGroupSerializer(tree.groups, many=True).data,
             "criteria": CompetencyCriterionReadSerializer(tree.criteria, many=True).data,
+            "criteria_count": tree.criteria_count,
+            "total_criteria_count": tree.total_criteria_count,
         })
 
 

tests/openedx_catalog/test_api.py

@@ -178,6 +178,50 @@ def test_get_course_run(python100_summer26: CourseRun) -> None:
     assert run == python100_summer26
 
 
+# get_course_run_ids
+
+
+def test_get_course_run_ids_empty_input_returns_empty_dict_with_no_queries(django_assert_num_queries) -> None:
+    """An empty course_keys input returns {} and costs zero queries: nothing to look up."""
+    with django_assert_num_queries(0):
+        assert api.get_course_run_ids([]) == {}
+
+
+def test_get_course_run_ids_unknown_key_is_absent_from_the_result() -> None:
+    """A course key with no matching CourseRun is simply absent from the result, not an error or a None value."""
+    unknown_key = CourseKey.from_string("course-v1:NoOrg+NoCourse+NoRun")
+
+    result = api.get_course_run_ids([unknown_key])
+
+    assert result == {}
+
+
+def test_get_course_run_ids_duplicate_key_yields_one_entry(python100_summer26: CourseRun) -> None:
+    """The same course key passed twice in the input produces exactly one entry in the result dict."""
+    assert python100_summer26.course_key is not None
+    key = python100_summer26.course_key
+
+    result = api.get_course_run_ids([key, key])
+
+    assert result == {key: python100_summer26.id}
+
+
+def test_get_course_run_ids_maps_found_keys_and_omits_the_unknown_one(
+    python100_summer26: CourseRun, python100_winter26: CourseRun,
+) -> None:
+    """With two keys that resolve and one that does not, the result maps exactly the two found keys to their ids."""
+    assert python100_summer26.course_key is not None
+    assert python100_winter26.course_key is not None
+    unknown_key = CourseKey.from_string("course-v1:NoOrg+NoCourse+NoRun")
+
+    result = api.get_course_run_ids([python100_summer26.course_key, unknown_key, python100_winter26.course_key])
+
+    assert result == {
+        python100_summer26.course_key: python100_summer26.id,
+        python100_winter26.course_key: python100_winter26.id,
+    }
+
+
 # sync_course_run_details