feat: add endpoint to delete or archive a CompetencyCriterion (#674)

docs/openedx_learning/decisions/0002-competency-criteria-model.rst

@@ -319,7 +319,7 @@ Decision
 
    - If no related row exists in ``StudentCompetencyCriteriaStatus`` (the leaf-level learner status table, Decision 6) for a competency definition, delete behaves as it does today: a hard delete that cascades through competency metadata tables.
    - Once a related row exists in ``StudentCompetencyCriteriaStatus``, deletion of the associated competency definition row still succeeds, but as an archive (soft delete) instead of a hard delete: the row is hidden from authoring and new associations but remains queryable, so existing learner status rows stay resolvable. This archive-vs-hard-delete rule applies to ``oel_tagging_tag``, ``oel_tagging_taxonomy``, ``CompetencyTaxonomy``, ``oel_tagging_objecttag``, ``CompetencyCriteriaGroup``, and ``CompetencyCriteria``; see :ref:`openedx-learning-adr-0003` Decision 3 for ``oel_tagging_objecttag``'s own archive rule and traceability exception.
-   - ``StudentCompetencyCriteriaStatus`` is what determines whether a record is protected. ``StudentCompetencyCriteriaGroupStatus`` and ``StudentCompetencyStatus`` are roll-up tables derived from it (Decision 6) and are not independently checked for this purpose: :ref:`openedx-learning-adr-0004` writes the leaf table synchronously with the grade but rolls the two roll-up tables up later via an asynchronous task, which can lag behind the leaf or, per that ADR's Decision 5, need manual recovery. Checking only the roll-up tables could therefore miss real learner progress that has not rolled up yet.
+   - ``StudentCompetencyCriteriaStatus`` is what determines whether a record is protected. ``StudentCompetencyCriteriaGroupStatus`` and ``StudentCompetencyStatus`` are roll-up tables derived from it (Decision 6) and are never relied on alone for this purpose: :ref:`openedx-learning-adr-0004` writes the leaf table synchronously with the grade but rolls the two roll-up tables up later via an asynchronous task, which can lag behind the leaf or, per that ADR's Decision 5, need manual recovery. Checking only the roll-up tables could therefore miss real learner progress that has not rolled up yet.
    - Direct deletion of a ``CompetencyRuleProfile`` is never a hard delete; retirement is always archive-only, via a normal update to its ``archived`` column (Decision 3). However, if a taxonomy or course that is associated with a taxonomy- or course-scoped profile is deleted, then this profile will be deleted along with it.
    - For MVP, archiving a ``CompetencyCriteriaGroup`` or a ``CompetencyCriteria`` row is permanent, with one exception: an archived root ``CompetencyCriteriaGroup`` is reactivated when a new criterion is added for its competency (Decision 2). ``oel_tagging_objecttag`` follows :ref:`openedx-learning-adr-0003` Decision 3 instead.
 
@@ -499,6 +499,10 @@ Changelog
   competency's own status row and up through parent competencies, and states what a
   competency with no criteria tree of its own receives.
 
+2026-11-04:
+
+* Clarified Decision 7: the roll-up tables are never relied on alone to protect a record.
+
 2026-11-06:
 
 * Decision 4 states that no ancestor or descendant of a competency tag may hold live criteria

docs/openedx_learning/decisions/0004-competency-mastery-concurrency.rst

@@ -68,7 +68,7 @@ Decision
    rows above it are recalculated and overwritten rather than merged. A later grade change can raise
    what an edit lowered, but never lower what an edit raised.    The staff correction takes a row lock
    on the ``StudentCompetencyCriteriaGroupStatus`` row for that learner and the competency's root
-   criteria group, the group with no parent; no other path takes a lock.
+   criteria group, the group with no parent; no other learner-status writer takes a lock.
 
 
 
@@ -162,3 +162,10 @@ Rejected Alternatives
           before the write completes.
         - It also only checks for a race at the moment each parent is read. If the worker crashes mid-cascade before reaching the next read,
           nothing notices the rollup was left unfinished. Decision 5's manual recovery mechanism exists to catch that case.
+
+Changelog
+---------
+
+2026-11-04:
+
+* Clarified Decision 6: the rule that no other path takes a lock covers learner-status writers only.

src/openedx_learning/applets/cbe/data.py

@@ -36,3 +36,16 @@ def __post_init__(self) -> None:
         # NaN and Infinity, actually raise the documented ValueError.
         if not self.fraction.is_finite() or not 0 <= self.fraction <= 1:
             raise ValueError(f"fraction must be finite and between 0 and 1 inclusive, not {self.fraction!r}")
+
+
+@dataclass(frozen=True, kw_only=True, slots=True)
+class CriterionDeletionResult:
+    """
+    The outcome of removing a CompetencyCriterion: whether its row was hard-deleted or archived.
+
+    UNSTABLE: the criteria-authoring family is incomplete, so this shape may change without a
+    deprecation cycle.
+    """
+
+    id: int
+    archived: bool

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

@@ -83,14 +83,15 @@ class CompetencyCriterionSerializer(serializers.ModelSerializer):
     )
     rule_profile_id = serializers.IntegerField(required=False, allow_null=True)
     object_tag_id = serializers.IntegerField(read_only=True)
+    archived = serializers.BooleanField(read_only=True)
 
     class Meta:
         model = CompetencyCriterion
         fields = [
             "id", "object_id", "group_id", "logic_operator",
-            "rule_profile_id", "rule_type_override", "rule_payload_override", "object_tag_id",
+            "rule_profile_id", "rule_type_override", "rule_payload_override", "object_tag_id", "archived",
         ]
-        read_only_fields = ["id", "object_tag_id"]
+        read_only_fields = ["id", "object_tag_id", "archived"]
 
     def validate_object_id(self, value: str) -> str:
         """Reject an object_id that isn't a parseable usage key."""

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

@@ -32,6 +32,11 @@
         views.CompetencyCriteriaGroupDetailView.as_view(),
         name="criteria-group-detail",
     ),
+    path(
+        "competencies/<int:tag_id>/criteria/<int:criterion_id>/",
+        views.CompetencyCriterionDeleteView.as_view(),
+        name="criterion-delete",
+    ),
     path(
         "competencies/<int:tag_id>/blocked-courses/",
         views.CompetencyBlockedCoursesView.as_view(),

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

@@ -4,6 +4,7 @@
 from __future__ import annotations
 
 from django.core.exceptions import ValidationError as DjangoValidationError
+from django.db import IntegrityError
 from django.db.models import QuerySet
 from django.shortcuts import get_object_or_404
 from django.utils.translation import gettext_lazy as _
@@ -26,6 +27,7 @@
     CompetencyCriterionArchivedError,
     associate_competency_criterion,
     bulk_update_competency_criteria,
+    delete_or_archive_competency_criterion,
     get_blocked_courses,
     get_competency_criteria_tree,
     get_competency_rule_profiles,
@@ -167,7 +169,7 @@ def patch(self, request, group_id: int):
                 rule_payload_override=data.get("rule_payload_override"),
                 user=request.user,
             )
-        except (CompetencyCriteriaGroup.DoesNotExist, CompetencyCriterion.DoesNotExist) as exc:
+        except (CompetencyCriteriaGroup.DoesNotExist, CompetencyCriterion.DoesNotExist, Tag.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
@@ -269,7 +271,7 @@ def patch(self, request, tag_id: int, group_id: int):
             group = update_competency_criteria_group(
                 group.id, logic_operator=serializer.validated_data["logic_operator"], user=request.user,
             )
-        except CompetencyCriteriaGroup.DoesNotExist as exc:
+        except (CompetencyCriteriaGroup.DoesNotExist, Tag.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
@@ -278,6 +280,41 @@ def patch(self, request, tag_id: int, group_id: int):
         return Response(CompetencyCriteriaGroupSerializer(group).data, status=status.HTTP_200_OK)
 
 
+class CompetencyCriterionDeleteView(generics.GenericAPIView):
+    """
+    DELETE-only. Remove a CompetencyCriterion: hard-delete, or archive if learner status exists.
+
+    Does not call self.check_object_permissions(): the public API function checks
+    oel_tagging.can_tag_object itself and raises Django's PermissionDenied, which becomes a 403.
+    """
+
+    authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser)
+    permission_classes = [IsAuthenticated]
+
+    def delete(self, request, *args, **kwargs):
+        """
+        Resolve the criterion by the `criterion_id` and `tag_id` in the URL, then delegate to the public API.
+
+        Archived rows still resolve, so a repeat DELETE on one returns 200.
+        """
+        criterion = get_object_or_404(
+            CompetencyCriterion, pk=self.kwargs["criterion_id"], group__tag_id=self.kwargs["tag_id"],
+        )
+        try:
+            result = delete_or_archive_competency_criterion(criterion.id, request.user)
+        except (CompetencyCriterion.DoesNotExist, Tag.DoesNotExist) as exc:
+            # The criterion, or its competency tag, was removed between the lookup above and the API call.
+            raise NotFound() from exc
+        except DjangoValidationError as exc:
+            raise DRFValidationError(exc.message_dict if hasattr(exc, "message_dict") else exc.messages) from exc
+        except IntegrityError as exc:
+            # ProtectedError is a subclass. A learner's grade write won the race, and a retry archives the criterion.
+            raise CompetencyCriteriaConflict(
+                _("A learner's progress was recorded against this criterion during the delete. Retry the DELETE.")
+            ) from exc
+        return Response({"id": result.id, "archived": result.archived}, status=status.HTTP_200_OK)
+
+
 class CompetencyBlockedCoursesView(generics.GenericAPIView):
     """
     GET-only. List the courses where a competency can't get a new criterion, paginated.

tests/openedx_learning/applets/cbe/conftest.py

@@ -1,4 +1,5 @@
 """Shared fixtures for the CBE test modules: schema, deletion, tree-integration, status, and REST tests."""
+from collections.abc import Callable
 from datetime import datetime, timezone
 
 import pytest
@@ -126,3 +127,35 @@ def _staff_client(staff_user) -> APIClient:
     client = APIClient()
     client.force_authenticate(user=staff_user)
     return client
+
+
+@pytest.fixture(name="run_when_tag_lock_is_taken")
+def _run_when_tag_lock_is_taken(monkeypatch: pytest.MonkeyPatch) -> Callable[[Callable[[], None]], None]:
+    """
+    Return a function that makes the API run a callable as it takes the Tag row lock.
+
+    The callable runs once, before the API reads the Tag. It stands in for a transaction that commits
+    while the API waits for the lock, so a read the API makes after taking the lock sees the change and
+    a read it made earlier doesn't.
+    """
+
+    def _install(side_effect: Callable[[], None]) -> None:
+        original_select_for_update = Tag.objects.select_for_update
+        fired: list[bool] = []
+
+        def _select_for_update(*args, **kwargs):
+            queryset = original_select_for_update(*args, **kwargs)
+            original_get = queryset.get
+
+            def _get(*get_args, **get_kwargs):
+                if not fired:
+                    fired.append(True)
+                    side_effect()
+                return original_get(*get_args, **get_kwargs)
+
+            queryset.get = _get
+            return queryset
+
+        monkeypatch.setattr(Tag.objects, "select_for_update", _select_for_update)
+
+    return _install

tests/openedx_learning/applets/cbe/test_api.py

@@ -1023,6 +1023,64 @@ def _fail_on_second_save(self, *args, **kwargs) -> None:
     assert snapshot(batch) == before
 
 
+def test_bulk_update_saves_in_primary_key_order_and_returns_request_order(
+    monkeypatch: pytest.MonkeyPatch,
+    batch: tuple[CompetencyCriterion, CompetencyCriterion],
+    leaf: CompetencyCriteriaGroup,
+    default_rule_profile: CompetencyRuleProfile,
+    user,
+) -> None:
+    """Each save takes a row lock, so the saves run in ascending pk order however the ids were requested."""
+    c1, c2 = batch
+    c3 = make_criterion(leaf, "p3", rule_profile=default_rule_profile)
+    original_save = CompetencyCriterion.save
+    saves: list[int] = []
+
+    def _record_save(self, *args, **kwargs) -> None:
+        saves.append(self.pk)
+        original_save(self, *args, **kwargs)
+
+    monkeypatch.setattr(CompetencyCriterion, "save", _record_save)
+
+    result = bulk_update_competency_criteria([c3.id, c1.id, c2.id], leaf.id, user=user, **NEW_RULE)
+
+    assert saves == [c1.id, c2.id, c3.id]
+    assert [criterion.id for criterion in result] == [c3.id, c1.id, c2.id]
+
+
+def test_bulk_update_checks_archived_state_under_the_tag_lock(
+    run_when_tag_lock_is_taken,
+    batch: tuple[CompetencyCriterion, CompetencyCriterion],
+    leaf: CompetencyCriteriaGroup,
+    user,
+) -> None:
+    """A group archived while the update waits for the Tag lock refuses the batch, so the check reads after the lock."""
+    run_when_tag_lock_is_taken(lambda: CompetencyCriteriaGroup.objects.filter(pk=leaf.pk).update(archived=True))
+    before = snapshot(batch)
+
+    with pytest.raises(CompetencyCriterionArchivedError, match=f"group {leaf.id}"):
+        bulk_update_competency_criteria([c.id for c in batch], leaf.id, user=user, **NEW_RULE)
+
+    assert snapshot(batch) == before
+
+
+def test_bulk_update_reads_the_criteria_under_the_tag_lock(
+    run_when_tag_lock_is_taken,
+    batch: tuple[CompetencyCriterion, CompetencyCriterion],
+    leaf: CompetencyCriteriaGroup,
+    user,
+) -> None:
+    """A criterion archived while the update waits for the Tag lock refuses the batch, so the criteria read after it."""
+    archived = batch[1]
+    run_when_tag_lock_is_taken(lambda: CompetencyCriterion.objects.filter(pk=archived.pk).update(archived=True))
+    before = snapshot(batch)
+
+    with pytest.raises(CompetencyCriterionArchivedError, match=rf"criteria cannot be edited: {archived.id}\."):
+        bulk_update_competency_criteria([c.id for c in batch], leaf.id, user=user, **NEW_RULE)
+
+    assert snapshot(batch) == before
+
+
 # ==============================================================================================
 # get_competency_criteria_tree
 # ==============================================================================================
@@ -1957,6 +2015,20 @@ def _fail(*args, **kwargs):
     assert criterion.group_id == group.id
 
 
+def test_create_competency_criterion_takes_the_tag_lock_itself(
+    run_when_tag_lock_is_taken, tag: Tag, course_run: CourseRun,
+) -> None:
+    """
+    A caller outside associate_competency_criterion() is still serialized with a delete of the competency's
+    criteria: the lock is taken before anything else, so a tag deleted while the call waits for it is seen.
+    """
+    group = _create_leaf_group(tag, course_run)
+    run_when_tag_lock_is_taken(lambda: Tag.objects.filter(pk=tag.pk).delete())
+
+    with pytest.raises(Tag.DoesNotExist):
+        cbe_api.create_competency_criterion(group, usage_key(course_run, "p1"), course_run=course_run)
+
+
 def test_associate_competency_criterion_creates_the_course_runs_cbe_row(
     tag: Tag, course_run: CourseRun,
 ) -> None:
@@ -2242,6 +2314,19 @@ def test_update_raises_does_not_exist_for_an_unknown_group(user) -> None:
         update_competency_criteria_group(999999, logic_operator=LogicOperator.AND, user=user)
 
 
+def test_group_update_re_reads_the_group_under_the_tag_lock(
+    run_when_tag_lock_is_taken, leaf: CompetencyCriteriaGroup, user,
+) -> None:
+    """A group archived while the update waits for the Tag lock is refused, not written onto."""
+    run_when_tag_lock_is_taken(lambda: CompetencyCriteriaGroup.objects.filter(pk=leaf.pk).update(archived=True))
+
+    with pytest.raises(CompetencyCriteriaGroupArchivedError):
+        update_competency_criteria_group(leaf.id, logic_operator=LogicOperator.AND, user=user)
+
+    leaf.refresh_from_db()
+    assert leaf.logic_operator == LogicOperator.OR
+
+
 @pytest.mark.parametrize("level", ["course-level", "leaf"])
 def test_update_checks_permission_against_the_groups_course(  # pylint: disable=too-many-positional-arguments
     level: str,