feat: add PATCH endpoint for a competency criteria group's logic_operator (#760)

src/openedx_learning/applets/cbe/api.py

@@ -37,6 +37,7 @@
 from .rule_payloads import evaluate_rule
 
 __all__ = [
+    "CompetencyCriteriaGroupArchivedError",
     "CompetencyCriteriaTree",
     "CompetencyCriterionArchivedError",
     "GradedObjectScore",
@@ -52,6 +53,7 @@
     "resolve_competency_tag",
     "resolve_supplied_leaf_group",
     "select_competency_taxonomies",
+    "update_competency_criteria_group",
 ]
 
 log = logging.getLogger(__name__)
@@ -67,6 +69,15 @@ class CompetencyCriterionArchivedError(Exception):
     """
 
 
+class CompetencyCriteriaGroupArchivedError(Exception):
+    """
+    Raised when a request would edit a CompetencyCriteriaGroup that has been archived.
+
+    UNSTABLE: the criteria-authoring family is incomplete, so its archive behavior may change this
+    exception, or when it is raised, without a deprecation cycle.
+    """
+
+
 def create_competency_taxonomy(  # pylint: disable=too-many-positional-arguments
     name: str,
     description: str | None = None,
@@ -781,3 +792,75 @@ def record_graded_object_statuses(*, user_id: int, scores: Sequence[GradedObject
         changed += _raise_leaf_status(user_id, criterion.pk, status, now)
 
     return changed
+
+
+def update_competency_criteria_group(
+    group_id: int,
+    *,
+    logic_operator: str,
+    user: UserType,
+) -> CompetencyCriteriaGroup:
+    """
+    Set how the CompetencyCriteriaGroup ``group_id`` names combines its children, and return it.
+
+    UNSTABLE: the criteria-authoring family is incomplete, so its validation and archive behavior
+    may change this function's shape without a deprecation cycle.
+
+    ``logic_operator`` is the only thing this changes. The group's name, ordering, competency,
+    course, and parent stay as they were created, and so does everything beneath it. A request
+    that leaves ``logic_operator`` as stored saves nothing, so the group's history gains a row
+    only for a real change. Learner status is never read or written.
+
+    Raises CompetencyCriteriaGroup.DoesNotExist if no group has ``group_id``, or Tag.DoesNotExist
+    if its tag isn't on a CompetencyTaxonomy.
+    Raises ValidationError, keyed by the REST field names, for a root group, a group with no course
+    anywhere above it to check permission against, or a ``logic_operator`` other than AND or OR, and
+    PermissionDenied if ``user`` fails ``oel_tagging.can_tag_object`` for the group's taxonomy and
+    course. Raises CompetencyCriteriaGroupArchivedError if the group is archived, even when nothing
+    would change.
+    """
+    with transaction.atomic():
+        group = CompetencyCriteriaGroup.objects.select_related("course", "parent__course").get(pk=group_id)
+        # Nothing in the model stops a group's tag being on another kind of taxonomy, so it's checked
+        # the way the create and tree endpoints check theirs.
+        tag = resolve_competency_tag(group.tag_id)
+        # tag.taxonomy is already confirmed set; re-asserted since that doesn't cross function boundaries for mypy.
+        assert tag.taxonomy is not None
+        # A root has neither a parent nor a course, and its subtree can span several courses, so
+        # there's no one course to check permission against.
+        if group.parent_id is None and group.course_id is None:
+            raise ValidationError({"group_id": _("A root CompetencyCriteriaGroup cannot be targeted directly.")})
+        # A course-level group carries its own course, and a group beneath one takes the course of
+        # its nearest ancestor that has one, however deeply it's nested.
+        ancestor = group
+        while ancestor.course is None and ancestor.parent is not None:
+            ancestor = ancestor.parent
+        course_run = ancestor.course
+        if course_run is None:
+            raise ValidationError(
+                {"group_id": _("This CompetencyCriteriaGroup has no course above it to check permission against.")}
+            )
+        perm_obj = ObjectTagPermissionItem(taxonomy=tag.taxonomy, object_id=str(course_run.course_key))
+        # has_perm()'s stub expects a Model, but a rules predicate takes any object.
+        if not user.has_perm("oel_tagging.can_tag_object", perm_obj):  # type: ignore[arg-type]
+            raise PermissionDenied
+
+        # Null is a stored state (a single-child group), but never one a caller may ask for.
+        if logic_operator not in LogicOperator.values:
+            raise ValidationError(
+                {"logic_operator": _("logic_operator must be one of: {choices}.").format(
+                    choices=", ".join(LogicOperator.values),
+                )}
+            )
+        # ADR-0002 Decision 2 hides archived groups from authoring; nothing reopens one here.
+        if group.archived:
+            raise CompetencyCriteriaGroupArchivedError(_("An archived CompetencyCriteriaGroup cannot be edited."))
+        if group.logic_operator == logic_operator:
+            return group
+
+        group.logic_operator = logic_operator
+        # Set on the instance so attribution doesn't depend on simple_history's request middleware.
+        group._history_user = user  # type: ignore[attr-defined]  # pylint: disable=protected-access
+        # Never QuerySet.update(): it skips post_save, so simple_history would record nothing.
+        group.save(update_fields=["logic_operator"])
+        return group

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

@@ -141,6 +141,38 @@ class Meta:
         fields = ["id", "parent_id", "tag_id", "course_key", "name", "ordering", "logic_operator"]
 
 
+class CompetencyCriteriaGroupUpdateSerializer(serializers.Serializer):  # pylint: disable=abstract-method
+    """
+    Parse the group-update request body against the stored group, which is this serializer's instance.
+
+    ``logic_operator`` is required and is the only field that can change. Its type is checked here,
+    and its value is checked by :func:`update_competency_criteria_group`. Every other field that
+    CompetencyCriteriaGroupSerializer emits is accepted too, so a client can send back the group it
+    read. Each one must match what is stored, and the error names any that don't. A mismatch is a
+    rejected edit, not something to ignore silently.
+    """
+
+    logic_operator = serializers.CharField()
+    id = serializers.IntegerField(required=False)
+    parent_id = serializers.IntegerField(required=False, allow_null=True)
+    tag_id = serializers.IntegerField(required=False)
+    course_key = serializers.CharField(required=False, allow_null=True)
+    # Compared as sent: trimming would turn an exact echo of a padded name into an apparent rename.
+    name = serializers.CharField(required=False, allow_blank=True, trim_whitespace=False)
+    ordering = serializers.IntegerField(required=False)
+
+    def validate(self, attrs: dict) -> dict:
+        """Reject any key this serializer does not declare, and any fixed field that differs from the stored one."""
+        unrecognized = sorted(set(self.initial_data) - set(self.fields))
+        if unrecognized:
+            raise serializers.ValidationError({key: "This field is not recognized." for key in unrecognized})
+        stored = CompetencyCriteriaGroupSerializer(self.instance).data
+        changed = sorted(key for key, value in attrs.items() if key != "logic_operator" and value != stored[key])
+        if changed:
+            raise serializers.ValidationError({key: "This field cannot be changed." for key in changed})
+        return attrs
+
+
 class CompetencyCriterionReadSerializer(serializers.ModelSerializer):
     """
     Read-only representation of a CompetencyCriterion, for the criteria-tree read endpoint.

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

@@ -27,4 +27,9 @@
         views.CompetencyCriteriaTreeView.as_view(),
         name="criteria-tree",
     ),
+    path(
+        "competencies/<int:tag_id>/criteria-groups/<int:group_id>/",
+        views.CompetencyCriteriaGroupDetailView.as_view(),
+        name="criteria-group-detail",
+    ),
 ]

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

@@ -5,6 +5,7 @@
 
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.db.models import QuerySet
+from django.shortcuts import get_object_or_404
 from django.utils.translation import gettext_lazy as _
 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication  # type: ignore[import]
 from edx_rest_framework_extensions.auth.session.authentication import (  # type: ignore[import]
@@ -21,18 +22,21 @@
 from openedx_tagging.rules import ObjectTagPermissionItem
 
 from ...api import (
+    CompetencyCriteriaGroupArchivedError,
     CompetencyCriterionArchivedError,
     associate_competency_criterion,
     bulk_update_competency_criteria,
     get_competency_criteria_tree,
     get_competency_rule_profiles,
     resolve_competency_tag,
+    update_competency_criteria_group,
 )
 from ...models import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile
 from ..paginators import CompetencyRuleProfilePagination
 from .permissions import CompetencyReadPermission, CompetencyRuleProfilePermissions
 from .serializers import (
     CompetencyCriteriaGroupSerializer,
+    CompetencyCriteriaGroupUpdateSerializer,
     CompetencyCriterionBulkUpdateSerializer,
     CompetencyCriterionReadSerializer,
     CompetencyCriterionSerializer,
@@ -198,3 +202,55 @@ def get(self, request, tag_id):
             "groups": CompetencyCriteriaGroupSerializer(tree.groups, many=True).data,
             "criteria": CompetencyCriterionReadSerializer(tree.criteria, many=True).data,
         })
+
+
+class CompetencyCriteriaGroupDetailView(generics.GenericAPIView):
+    """
+    PATCH-only. Change how one CompetencyCriteriaGroup combines the criteria and groups beneath it.
+
+    **Example Request**
+        PATCH api/cbe/v1/competencies/<tag_id>/criteria-groups/<group_id>/
+        ``{"logic_operator": "AND"}``
+
+    The response is the group as stored, in the criteria-tree endpoint's shape. ``logic_operator``
+    is the only editable field. The body may also repeat any other field the group's representation
+    carries, so a client can send back what it read, as long as each one matches what is stored.
+
+    A thin adapter: :func:`update_competency_criteria_group` makes every other refusal, including
+    the edit permission check, so an in-process caller is refused in the same cases. The body's
+    fixed fields are compared here, before that function runs, because only a REST body carries
+    them. Comparing them reveals what's stored, so a caller who can't read the competency's tree is
+    refused with 403 before the group is looked up.
+    """
+
+    serializer_class = CompetencyCriteriaGroupUpdateSerializer
+    http_method_names = ["patch"]
+    authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
+    permission_classes = [IsAuthenticated, CompetencyReadPermission]
+
+    def patch(self, request, tag_id: int, group_id: int):
+        """Check the body against the stored group, then delegate to the public API."""
+        # Same 404 the create and tree endpoints give a tag that isn't a competency, ahead of the body checks.
+        try:
+            tag = resolve_competency_tag(tag_id)
+        except Tag.DoesNotExist as exc:
+            raise NotFound(str(exc)) from exc
+        # The group lookup and body check below reveal what's stored, so only a tree reader may reach them.
+        self.check_object_permissions(request, tag)
+        # Scoped by tag_id, so addressing a group under a competency it doesn't belong to is a 404.
+        group = get_object_or_404(
+            CompetencyCriteriaGroup.objects.select_related("course"), pk=group_id, tag_id=tag_id,
+        )
+        serializer = self.get_serializer(group, data=request.data)
+        serializer.is_valid(raise_exception=True)
+        try:
+            group = update_competency_criteria_group(
+                group.id, logic_operator=serializer.validated_data["logic_operator"], user=request.user,
+            )
+        except CompetencyCriteriaGroup.DoesNotExist as exc:
+            raise NotFound(str(exc)) from exc
+        except DjangoValidationError as exc:
+            raise DRFValidationError(exc.message_dict if hasattr(exc, "message_dict") else exc.messages) from exc
+        except CompetencyCriteriaGroupArchivedError as exc:
+            raise CompetencyCriteriaConflict(str(exc)) from exc
+        return Response(CompetencyCriteriaGroupSerializer(group).data, status=status.HTTP_200_OK)