diff --git a/projects/urls.py b/projects/urls.py index 1e1f0d7c1..b9feea3b3 100644 --- a/projects/urls.py +++ b/projects/urls.py @@ -8,5 +8,7 @@ path("admin/doc/", include("django.contrib.admindocs.urls")), path("admin/", admin.site.urls), path("tagging/rest_api/", include("openedx_tagging.urls")), + # Mirrors the prefix openedx-platform mounts CBE at; a consuming project picks its own. + path("api/", include("openedx_learning.urls")), path('__debug__/', include('debug_toolbar.urls')), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index a137901e1..16667b4bf 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -3,16 +3,37 @@ """ from __future__ import annotations +from django.core.exceptions import ValidationError +from django.db import transaction from django.db.models import QuerySet +from django.http import Http404 +from django.shortcuts import get_object_or_404 +from django.utils.translation import gettext_lazy as _ +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey -from openedx_tagging.api import create_taxonomy -from openedx_tagging.models import Taxonomy +from openedx_catalog.api import get_course_run +from openedx_catalog.models import CourseRun +from openedx_tagging.api import create_taxonomy, get_object_tags, tag_object +from openedx_tagging.models import ObjectTag, Tag, Taxonomy -from .models import CompetencyTaxonomy +from .models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + LogicOperator, +) __all__ = [ + "associate_competency_criterion", + "create_competency_criterion", "create_competency_taxonomy", + "get_competency_rule_profiles", "is_competency_taxonomy", + "resolve_competency_tag", + "create_leaf_group", + "resolve_supplied_leaf_group", "select_competency_taxonomies", ] @@ -43,6 +64,21 @@ def create_competency_taxonomy( # pylint: disable=too-many-positional-arguments return taxonomy +def get_competency_rule_profiles() -> QuerySet[CompetencyRuleProfile]: + """ + Return every live CompetencyRuleProfile, in ascending ``id`` order. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive entry + points still to come may change this function's shape without a deprecation cycle. + + Archived profiles are left out: retirement is archive-only. + + The ordering is part of the contract rather than a cosmetic detail: an unordered queryset + gives a paginating caller overlapping and skipped pages. + """ + return CompetencyRuleProfile.objects.filter(archived=False).order_by("id") + + def is_competency_taxonomy(taxonomy: Taxonomy) -> bool: """ Return True if ``taxonomy`` is competency-enabled, i.e. has a CompetencyTaxonomy row. @@ -63,3 +99,199 @@ def select_competency_taxonomies(taxonomies: QuerySet[Taxonomy]) -> QuerySet[Tax so the check costs no additional query per row. """ return taxonomies.select_related("competencytaxonomy") + + +def resolve_competency_tag(tag_id: int) -> Tag: + """ + Return the competency Tag identified by ``tag_id``. + + Raises Http404 if no such Tag exists, or if it isn't on a CompetencyTaxonomy. Shared by + the view and :func:`associate_competency_criterion` so both agree on what counts as valid. + """ + tag = get_object_or_404(Tag, pk=tag_id) + # Explicit None check narrows the type for mypy and gives a precise 404 reason. + if tag.taxonomy is None or not is_competency_taxonomy(tag.taxonomy): + raise Http404("Tag is not on a CompetencyTaxonomy.") + return tag + + +def create_leaf_group( + tag: Tag, + course_run: CourseRun, + logic_operator: str | None = None, +) -> CompetencyCriteriaGroup: + """ + Return a fresh leaf CompetencyCriteriaGroup under ``tag``'s root/course-level groups. + + Gets or creates the root and course-level groups (race-safe by locking ``tag``'s row), then + always creates a new leaf: a tag/course pair can hold more than one leaf, one per criterion. + """ + with transaction.atomic(): + # MySQL has no partial unique index to stop two concurrent requests from each creating a + # root for the same tag, so they are serialized on the tag row instead. + Tag.objects.select_for_update().get(pk=tag.pk) + root, _ = CompetencyCriteriaGroup.objects.get_or_create( + tag=tag, + parent=None, + defaults={"name": f"{tag.value} (root)"}, + ) + course_level_group, _ = CompetencyCriteriaGroup.objects.get_or_create( + tag=tag, + course=course_run, + parent=root, + defaults={"name": f"{tag.value} — {course_run.title}"}, + ) + return CompetencyCriteriaGroup.objects.create( + tag=tag, + course=None, + parent=course_level_group, + logic_operator=logic_operator or LogicOperator.OR, + ) + + +def resolve_supplied_leaf_group(group_id: int, tag: Tag, course_run: CourseRun) -> CompetencyCriteriaGroup: + """ + Return the CompetencyCriteriaGroup identified by ``group_id``, validated as a usable leaf. + + Raises Http404 if it doesn't exist, or ValidationError (keyed "group_id") if it isn't a + leaf, doesn't belong to ``tag``, or its course doesn't match ``course_run``. + """ + group = get_object_or_404(CompetencyCriteriaGroup, pk=group_id) + if group.parent_id is None or group.course_id is not None: + raise ValidationError({"group_id": _("group_id must reference a leaf CompetencyCriteriaGroup.")}) + if group.tag_id != tag.id: + raise ValidationError({"group_id": _("group_id must belong to the given competency tag.")}) + if group.parent.course_id != course_run.id: + raise ValidationError({"group_id": _("group_id must belong to the given course.")}) + return group + + +def create_competency_criterion( + group: CompetencyCriteriaGroup, + object_id: str, + rule_profile_id: int | None = None, + rule_type_override: str | None = None, + rule_payload_override: dict | None = None, +) -> CompetencyCriterion: + """ + Create and return a CompetencyCriterion under ``group``, tagging ``object_id`` along the way. + + ``tag_object()`` replaces an object's full tag list for a taxonomy rather than appending, so + this reads the object's existing tags first and unions in the new one, rather than silently + dropping a sibling competency's tag under the same taxonomy. + + When no rule fields are supplied, resolves to the seeded system-default CompetencyRuleProfile + rather than persisting null: ADR-0002 Decision 4 always resolves to a concrete profile, and + the model's own xor-override constraint forbids leaving all three fields null. + """ + tag = group.tag + # tag.taxonomy_id is nullable at the model level; already guaranteed set by the caller. + assert tag.taxonomy_id is not None + existing_values = [existing_tag.value for existing_tag in get_object_tags(object_id, taxonomy_id=tag.taxonomy_id)] + if tag.value not in existing_values: + existing_values.append(tag.value) + tag_object(object_id, tag.taxonomy, existing_values) + object_tag = ObjectTag.objects.get(object_id=object_id, taxonomy_id=tag.taxonomy_id, tag_id=tag.id) + + if rule_profile_id is None and rule_type_override is None and rule_payload_override is None: + rule_profile_id = CompetencyRuleProfile.objects.get( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + archived=False, + ).id + + # #666 inserts its parent-competency dominance/containment check here, before creation. + + return CompetencyCriterion.objects.create( + group=group, + object_tag=object_tag, + rule_profile_id=rule_profile_id, + rule_type_override=rule_type_override, + rule_payload_override=rule_payload_override, + ) + + +def _reject_duplicate_criterion(tag: Tag, object_id: str, group_id: int | None) -> None: + """ + Raise ValidationError if ``object_id`` is already associated with ``tag`` (in ``group_id``, when supplied). + + Must run inside a transaction holding the tag row lock. + """ + assert tag.taxonomy_id is not None # narrowing for mypy; callers resolve competency tags only + # Locking reads, so a duplicate committed by a concurrent request is seen under MySQL REPEATABLE READ. + existing_object_tag = ObjectTag.objects.select_for_update().filter( + object_id=object_id, taxonomy_id=tag.taxonomy_id, tag_id=tag.id, + ).first() + if existing_object_tag is None: + return + duplicate_criteria = CompetencyCriterion.objects.select_for_update().filter(object_tag=existing_object_tag) + if group_id is None: + # Can't know which group slot is intended, so any existing criterion is a duplicate. + if list(duplicate_criteria.values_list("pk", flat=True)[:1]): + raise ValidationError( + {"object_id": _("A CompetencyCriterion already associates this tag with this object_id.")} + ) + elif list(duplicate_criteria.filter(group_id=group_id).values_list("pk", flat=True)[:1]): + # A different explicit group is a deliberate second association (ADR-0002), not a duplicate. + raise ValidationError( + {"group_id": _("A CompetencyCriterion already associates this tag with this object_id in this group.")} + ) + + +def associate_competency_criterion( + tag_id: int, + object_id: str, + *, + group_id: int | None = None, + logic_operator: str | None = None, + competency_rule_profile_id: int | None = None, + rule_type_override: str | None = None, + rule_payload_override: dict | None = None, +) -> CompetencyCriterion: + """ + Associate ``object_id`` with the competency ``tag_id`` names, creating a criterion for it. + + The public entry point for #665's create-criterion endpoint; the caller is expected to have + already checked ``oel_tagging.can_tag_object``. Rejects re-targeting the same group a + (tag_id, object_id) pair already uses, and rejects a duplicate via the derive-or-create + path; a *different* explicit group is a deliberate second association per ADR-0002, not a + duplicate. Branches on ``group_id`` to resolve or derive/create a leaf, then creates the + criterion, all inside one transaction so a downstream failure rolls back any group just + created. + + Raises ValidationError (never DRF's) on rejected input, Http404 if tag_id/group_id don't + resolve. + """ + tag = resolve_competency_tag(tag_id) + # tag.taxonomy is already confirmed set; re-asserted since that doesn't cross function boundaries for mypy. + assert tag.taxonomy_id is not None + + try: + usage_key = UsageKey.from_string(object_id) + except InvalidKeyError as exc: + raise ValidationError({"object_id": _("object_id is not a valid usage key.")}) from exc + + try: + course_run = get_course_run(usage_key.course_key) + except CourseRun.DoesNotExist as exc: + raise ValidationError({"object_id": _("No course run matches object_id's course.")}) from exc + + if group_id is not None and logic_operator is not None: + raise ValidationError({"logic_operator": _("group_id and logic_operator cannot both be supplied.")}) + + with transaction.atomic(): + # Serializes concurrent callers so the duplicate check below can't be passed by two requests at once. + Tag.objects.select_for_update().get(pk=tag.pk) + _reject_duplicate_criterion(tag, object_id, group_id) + if group_id is not None: + group = resolve_supplied_leaf_group(group_id, tag, course_run) + else: + group = create_leaf_group(tag, course_run, logic_operator) + return create_competency_criterion( + group=group, + object_id=object_id, + rule_profile_id=competency_rule_profile_id, + rule_type_override=rule_type_override, + rule_payload_override=rule_payload_override, + ) diff --git a/src/openedx_learning/applets/cbe/models/criteria.py b/src/openedx_learning/applets/cbe/models/criteria.py index 084ae81f2..56290d970 100644 --- a/src/openedx_learning/applets/cbe/models/criteria.py +++ b/src/openedx_learning/applets/cbe/models/criteria.py @@ -107,6 +107,21 @@ class Meta: # indexes every ForeignKey column by default, so a second explicit one here would only # cost write throughput without adding any read benefit. ] + # No constraint tying `logic_operator` to child count, and no UniqueConstraint on (parent, + # ordering): a child group cannot be saved until its parent's primary key exists, so + # neither has a single-row state to check at save time. See ADR-0002 Decision 2. + constraints = [ + # Backstops create_leaf_group()'s get_or_create() for course-level groups. It has no + # `condition` because MySQL has no partial indexes: a unique index allows repeated + # NULLs, and `course` is NULL for roots and leaves. + models.UniqueConstraint( + fields=["tag", "course", "parent"], + name="oel_cbe_criteria_group_one_course_group_per_tag_course", + violation_error_message=_( + "A competency tag may have at most one course-level CompetencyCriteriaGroup per course." + ), + ), + ] class CompetencyRuleProfile(models.Model): diff --git a/src/openedx_learning/applets/cbe/rest_api/__init__.py b/src/openedx_learning/applets/cbe/rest_api/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_learning/applets/cbe/rest_api/paginators.py b/src/openedx_learning/applets/cbe/rest_api/paginators.py new file mode 100644 index 000000000..ea23f67c9 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/paginators.py @@ -0,0 +1,20 @@ +""" +Paginators for the CBE REST API. + +These sit outside the versioned package, as openedx_tagging's do, because a page size is +version independent. +""" +from edx_rest_framework_extensions.paginators import DefaultPagination # type: ignore[import] + + +class CompetencyRuleProfilePagination(DefaultPagination): + """ + Page size for the competency rule profile collection. + + Pinned here rather than inherited from the consuming project's DEFAULT_PAGINATION_CLASS, the + same way openedx_tagging pins TaxonomyPagination: this is a published library, so the page + size is part of its REST contract instead of varying by deployment. + """ + + page_size = 100 + max_page_size = 500 diff --git a/src/openedx_learning/applets/cbe/rest_api/urls.py b/src/openedx_learning/applets/cbe/rest_api/urls.py new file mode 100644 index 000000000..8b882d982 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/urls.py @@ -0,0 +1,14 @@ +""" +CBE API URLs. +""" + +from django.urls import include, path + +from .v1 import urls as v1_urls + +# Namespaces the whole CBE API, so routes reverse as "cbe:". Declared here rather than in +# v1/ because the namespace spans every version, and rather than on the app-root module because +# that one aggregates every applet. +app_name = "cbe" + +urlpatterns = [path("v1/", include(v1_urls))] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/__init__.py b/src/openedx_learning/applets/cbe/rest_api/v1/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py new file mode 100644 index 000000000..2c3cf186f --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/permissions.py @@ -0,0 +1,19 @@ +""" +Permissions for the CBE REST API, v1. +""" +from rest_framework.permissions import DjangoObjectPermissions + + +class CompetencyRuleProfilePermissions(DjangoObjectPermissions): + """ + Maps each REST API method to its corresponding CompetencyRuleProfile permission. + + Only the read methods are mapped, so DRF answers a write attempt with 405 rather than + checking a permission that no write endpoint would honor anyway. + """ + + perms_map = { + "GET": ["%(app_label)s.view_%(model_name)s"], + "OPTIONS": [], + "HEAD": ["%(app_label)s.view_%(model_name)s"], + } diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py new file mode 100644 index 000000000..e937e758f --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/serializers.py @@ -0,0 +1,89 @@ +""" +Serializers for the CBE REST API, v1. +""" +from __future__ import annotations + +from rest_framework import serializers + +from ...models import CompetencyCriterion, CompetencyRuleProfile, LogicOperator + + +class CompetencyRuleProfileSerializer(serializers.ModelSerializer): + """ + Read-only representation of a CompetencyRuleProfile. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive + endpoints still to come may change this shape without a deprecation cycle. + + ``rule_payload`` is emitted verbatim as stored. For ``Grade``, the only rule type + supported in this phase, that shape is ``{"op": ..., "value": ..., "scale": ...}``: + ``op`` is one of ``gte``, ``lte``, or ``eq``, and ``value`` is a fraction between 0.0 + and 1.0 inclusive. ``GradePayload`` and ``validate_rule_payload`` in the + ``rule_payloads`` module own that shape, so it is not renormalized here. ``scale`` + confirms the fraction is a percentage, ruling out any other scale. + + ``scope_code`` and the raw ``organization``, ``course``, and ``competency_taxonomy`` + columns are left out: ``scope_type`` below is what a client can act on, while the + others are internal, existing only to enforce the one-profile-per-scope constraint + (:ref:`openedx-learning-adr-0002` Decision 3). + """ + + scope_type = serializers.SerializerMethodField() + + class Meta: + model = CompetencyRuleProfile + fields = ["id", "scope_type", "rule_type", "rule_payload", "archived"] + # scope_type is absent here because DRF refuses a field that is both declared above and + # named in read_only_fields; a SerializerMethodField is read-only in any case. + read_only_fields = ["id", "rule_type", "rule_payload", "archived"] + + def get_scope_type(self, profile: CompetencyRuleProfile) -> str: + """ + Return which kind of scope ``profile`` applies to. + + All four kinds are recognized from the outset, even though only the system default can + exist today, so enabling a narrower scope needs no edit here. The scope columns are read + by their ``_id`` attributes so that no row costs a query, and the system default is + recognized by those columns being null rather than by matching the internal + ``scope_code`` string. + """ + if profile.competency_taxonomy_id is not None: + return "taxonomy" + if profile.course_id is not None: + return "course" + if profile.organization_id is not None: + return "organization" + return "system_default" + + +class CompetencyCriterionSerializer(serializers.ModelSerializer): + """ + Doubles as the request-body parser and the response representation for a criterion. + + ``object_id`` and ``logic_operator`` are not CompetencyCriterion fields at all (``object_id`` + isn't stored anywhere on this model; ``logic_operator`` belongs to CompetencyCriteriaGroup), + so they're declared as plain write_only fields the view reads out of ``validated_data``, not + model-bound fields. Every other field's JSON name matches its model attribute exactly + (``group_id``, ``rule_profile_id``, ``object_tag_id``), so none of them need a ``source=``. + """ + + object_id = serializers.CharField(write_only=True) + group_id = serializers.IntegerField(required=False, allow_null=True) + # Only ever applies on the derive-or-create path (group_id omitted): it sets the AND/OR + # operator on the brand-new leaf group this request creates. Rejected alongside an explicit + # group_id because changing an existing leaf's operator is a future group-update endpoint's + # job, not this one's -- a caller can't use this field to silently change an existing + # group's behavior. + logic_operator = serializers.ChoiceField( + choices=LogicOperator.choices, write_only=True, required=False, allow_null=True, + ) + rule_profile_id = serializers.IntegerField(required=False, allow_null=True) + object_tag_id = serializers.IntegerField(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", + ] + read_only_fields = ["id", "object_tag_id"] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/urls.py b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py new file mode 100644 index 000000000..faab07b98 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/urls.py @@ -0,0 +1,20 @@ +""" +CBE API v1 URLs. +""" + +from django.urls.conf import include, path +from rest_framework.routers import DefaultRouter + +from . import views + +router = DefaultRouter() +router.register("rule_profiles", views.CompetencyRuleProfileView, basename="rule_profile") + +urlpatterns = [ + path("", include(router.urls)), + path( + "competencies//criteria/", + views.CompetencyCriterionCreateView.as_view(), + name="criterion-create", + ), +] diff --git a/src/openedx_learning/applets/cbe/rest_api/v1/views.py b/src/openedx_learning/applets/cbe/rest_api/v1/views.py new file mode 100644 index 000000000..13a1b1f91 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rest_api/v1/views.py @@ -0,0 +1,100 @@ +""" +Views for the CBE REST API, v1. +""" +from __future__ import annotations + +from django.core.exceptions import ValidationError as DjangoValidationError +from django.db.models import QuerySet +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] + SessionAuthenticationAllowInactiveUser, +) +from rest_framework import generics, mixins, status +from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import ValidationError as DRFValidationError +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.viewsets import GenericViewSet + +from openedx_tagging.rules import ObjectTagPermissionItem + +from ...api import associate_competency_criterion, get_competency_rule_profiles, resolve_competency_tag +from ...models import CompetencyRuleProfile +from ..paginators import CompetencyRuleProfilePagination +from .permissions import CompetencyRuleProfilePermissions +from .serializers import CompetencyCriterionSerializer, CompetencyRuleProfileSerializer + + +class CompetencyRuleProfileView(mixins.ListModelMixin, GenericViewSet): + """ + Read the rule profiles this instance defines. + + ``GET api/cbe/v1/rule_profiles/`` lists them to callers who may administer competency + configuration, paginated by ``page`` and ``page_size``. + + UNSTABLE: the rule profile family is incomplete, so the create, update, and archive + endpoints still to come may change this shape without a deprecation cycle. + + This is a collection even while the seeded system default is the only profile an instance + holds, and it is paginated from the first release: wrapping a bare array in an envelope later + would change the top-level JSON type. A viewset rather than a ListAPIView, so the deferred + create and detail routes can be added as further mixins without touching the URL module. + """ + + serializer_class = CompetencyRuleProfileSerializer + permission_classes = [CompetencyRuleProfilePermissions] + pagination_class = CompetencyRuleProfilePagination + # Set here rather than through openedx_tagging's view_auth_classes decorator, which lives in + # another app's REST internals rather than in an API this library publishes. + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + + def get_queryset(self) -> QuerySet[CompetencyRuleProfile]: + """Return the live rule profiles, filtered and ordered by the applet's public API.""" + return get_competency_rule_profiles() + + +class CompetencyCriterionCreateView(generics.CreateAPIView): + """ + POST-only. Create a CompetencyCriterion associating an object_id with a competency tag. + + Overrides ``create()`` rather than relying on ``ModelSerializer.save()``, because creation + is a multi-step domain operation (tag/course resolution, permission check, duplicate check, + group derivation, criterion creation) that :func:`associate_competency_criterion` already + owns end-to-end -- the serializer's job here is validating/shaping the request body and + representing the result, not constructing the instance itself. + + ``permission_classes`` is ``IsAuthenticated`` only, not a ``DjangoObjectPermissions`` + subclass: every existing such class in this codebase only ever contributes the + authentication gate, since the ``rules`` predicate behind it returns True whenever called + with no object (as every class-level check does). Copying that shape here would mean + registering a new, unused ``openedx_learning.add_competencycriterion`` permission for no + behavioral gain, so the real, object-scoped authorization is a manual ``has_perm()`` call + below instead. + """ + + serializer_class = CompetencyCriterionSerializer + authentication_classes = (JwtAuthentication, SessionAuthenticationAllowInactiveUser) + permission_classes = [IsAuthenticated] + + def create(self, request, *args, **kwargs): + """Validate the request body, check permission, then delegate to the public API.""" + serializer = self.get_serializer(data=request.data) + serializer.is_valid(raise_exception=True) + data = serializer.validated_data + tag = resolve_competency_tag(self.kwargs["tag_id"]) + perm_obj = ObjectTagPermissionItem(taxonomy=tag.taxonomy, object_id=data["object_id"]) + if not request.user.has_perm("oel_tagging.can_tag_object", perm_obj): + raise PermissionDenied() + try: + criterion = associate_competency_criterion( + tag_id=tag.id, + object_id=data["object_id"], + group_id=data.get("group_id"), + logic_operator=data.get("logic_operator"), + competency_rule_profile_id=data.get("rule_profile_id"), + rule_type_override=data.get("rule_type_override"), + rule_payload_override=data.get("rule_payload_override"), + ) + except DjangoValidationError as exc: + raise DRFValidationError(exc.message_dict if hasattr(exc, "message_dict") else exc.messages) from exc + return Response(self.get_serializer(criterion).data, status=status.HTTP_201_CREATED) diff --git a/src/openedx_learning/applets/cbe/rules.py b/src/openedx_learning/applets/cbe/rules.py new file mode 100644 index 000000000..58a725b57 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rules.py @@ -0,0 +1,38 @@ +""" +Django rules-based permissions for Competency-Based Education (CBE). + +Registered with the ``rules`` permission registry on import. ``src/openedx_learning/rules.py`` +is what makes that import happen; see the note there. +""" +from __future__ import annotations + +import rules + +from openedx_tagging.rules import UserType, is_taxonomy_admin + +from .models import CompetencyRuleProfile + +__all__ = [ + "can_view_competency_rule_profile", +] + + +@rules.predicate +def can_view_competency_rule_profile( + user: UserType, + profile: CompetencyRuleProfile | None = None, # pylint: disable=unused-argument +) -> bool: + """ + Taxonomy admins can read any competency rule profile. + + The system default is instance-wide competency configuration, administered by the same + people who administer taxonomies, so this reuses the tagging app's notion of an + administrator rather than defining a second one inside CBE that could drift from it. + + ``profile`` is accepted but not consulted yet: the scoped profiles still to come will branch + on it and filter rows, rather than refuse a whole request in order to hide some of its rows. + """ + return is_taxonomy_admin(user) + + +rules.add_perm("openedx_learning.view_competencyruleprofile", can_view_competency_rule_profile) diff --git a/src/openedx_learning/migrations/0014_criteria_group_unique_constraints.py b/src/openedx_learning/migrations/0014_criteria_group_unique_constraints.py new file mode 100644 index 000000000..d1a9f73f2 --- /dev/null +++ b/src/openedx_learning/migrations/0014_criteria_group_unique_constraints.py @@ -0,0 +1,26 @@ +""" +Add the UniqueConstraint on CompetencyCriteriaGroup that backstops course-level group creation. +""" +# Generated by Django 5.2.16 on 2026-09-11 00:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_learning', '0013_alter_historicalcompetencycriteriagroup_history_date_and_more'), + ] + + operations = [ + migrations.AddConstraint( + model_name='competencycriteriagroup', + constraint=models.UniqueConstraint( + fields=('tag', 'course', 'parent'), + name='oel_cbe_criteria_group_one_course_group_per_tag_course', + violation_error_message=( + 'A competency tag may have at most one course-level CompetencyCriteriaGroup per course.' + ), + ), + ), + ] diff --git a/src/openedx_learning/rules.py b/src/openedx_learning/rules.py new file mode 100644 index 000000000..8364ed0c8 --- /dev/null +++ b/src/openedx_learning/rules.py @@ -0,0 +1,9 @@ +""" +This is where the openedx_learning app's rules-based permissions are registered. + +``rules.apps.AutodiscoverRulesConfig`` imports ``.rules`` and nothing deeper, so +an applet's permissions reach the registry only by being pulled in from here. +""" +# This wildcard import is okay because the applet rules module declares __all__. +# pylint: disable=wildcard-import +from .applets.cbe.rules import * diff --git a/src/openedx_learning/urls.py b/src/openedx_learning/urls.py new file mode 100644 index 000000000..75aec0c5c --- /dev/null +++ b/src/openedx_learning/urls.py @@ -0,0 +1,16 @@ +""" +Learning API URLs. + +Maps each applet onto its own URL segment. A consuming project includes this module and never an +applet package directly, so ``applets/`` stays the internal layout detail that api.py and +models.py already exist to hide. + +No ``app_name`` here on purpose: each applet declares its own, so route names stay unique per +applet rather than sharing one flat namespace. +""" + +from django.urls import include, path + +from .applets.cbe.rest_api import urls as cbe_urls + +urlpatterns = [path("cbe/", include(cbe_urls))] diff --git a/tests/openedx_learning/applets/cbe/conftest.py b/tests/openedx_learning/applets/cbe/conftest.py index 8053fda18..db10090f0 100644 --- a/tests/openedx_learning/applets/cbe/conftest.py +++ b/tests/openedx_learning/applets/cbe/conftest.py @@ -1,10 +1,11 @@ -"""Shared fixtures for the CBE criteria test modules.""" +"""Shared fixtures for the CBE test modules: schema, deletion, criteria, tree-integration, and REST tests.""" from datetime import datetime, timezone import pytest from django.contrib.auth import get_user_model from organizations.api import ensure_organization from organizations.models import Organization +from rest_framework.test import APIClient from openedx_catalog.models import CatalogCourse, CourseRun from openedx_learning.models import CompetencyCriteriaGroup, CompetencyRuleProfile, CompetencyTaxonomy @@ -72,7 +73,7 @@ def _default_rule_profile() -> CompetencyRuleProfile: @pytest.fixture(name="user") def _user(): - """Create a single learner for use in these tests.""" + """A user without staff rights, who is also the learner in status tests.""" return get_user_model().objects.create(username="learner") @@ -80,3 +81,23 @@ def _user(): def _now() -> datetime: """A single UTC timestamp shared by writes in a test.""" return datetime.now(timezone.utc) + + +@pytest.fixture(name="staff_user") +def _staff_user(): + """A user permitted to administer instance-wide competency configuration.""" + return get_user_model().objects.create(username="staff", email="staff@example.com", is_staff=True) + + +@pytest.fixture(name="api_client") +def _api_client() -> APIClient: + """A REST client for a caller the system cannot identify.""" + return APIClient() + + +@pytest.fixture(name="staff_client") +def _staff_client(staff_user) -> APIClient: + """A REST client acting as `staff_user`.""" + client = APIClient() + client.force_authenticate(user=staff_user) + return client diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py index 53ceff16a..eecb876ee 100644 --- a/tests/openedx_learning/applets/cbe/test_api.py +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -1,14 +1,39 @@ """ Tests for the CBE public API surface (openedx_learning.api). """ +from unittest import mock + import pytest +from django.core.exceptions import ValidationError +from django.db.utils import IntegrityError +from django.http import Http404 +from organizations.models import Organization -from openedx_learning.api import create_competency_taxonomy, is_competency_taxonomy, select_competency_taxonomies -from openedx_learning.models import CompetencyTaxonomy -from openedx_tagging.models import Taxonomy +from openedx_catalog.models import CatalogCourse, CourseRun +from openedx_learning.api import ( + associate_competency_criterion, + create_competency_taxonomy, + create_leaf_group, + get_competency_rule_profiles, + is_competency_taxonomy, + resolve_supplied_leaf_group, + select_competency_taxonomies, +) +from openedx_learning.applets.cbe import api as cbe_api +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + LogicOperator, + RuleType, +) +from openedx_tagging.models import ObjectTag, Tag, Taxonomy pytestmark = pytest.mark.django_db +GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + def test_create_competency_taxonomy_saves_both_rows() -> None: """ @@ -55,6 +80,19 @@ def test_create_competency_taxonomy_defaults() -> None: assert result.export_id +def make_course_run(organization: Organization, course_code: str, run_code: str) -> CourseRun: + """Create a CourseRun distinct from the `course_run` fixture, for the same-tag-different-course tests.""" + catalog_course = CatalogCourse.objects.create(org=organization, course_code=course_code) + return CourseRun.objects.create(catalog_course=catalog_course, run_code=run_code) + + +def usage_key(course_run: CourseRun, block_id: str) -> str: + """Build a gradeable-subsection-shaped usage key string under `course_run`.""" + key = course_run.course_key + assert key is not None + return f"block-v1:{key.org}+{key.course}+{key.run}+type@sequential+block@{block_id}" + + def test_is_competency_taxonomy() -> None: """ is_competency_taxonomy() is True for a competency taxonomy, False for a plain one. @@ -104,3 +142,397 @@ def test_select_competency_taxonomies_avoids_n_plus_1(django_assert_num_queries) assert results.count(True) == 2 assert results.count(False) == 1 + + +def test_get_competency_rule_profiles_returns_the_seeded_default( + default_rule_profile: CompetencyRuleProfile, +) -> None: + """get_competency_rule_profiles() returns the system default an instance starts with.""" + assert list(get_competency_rule_profiles()) == [default_rule_profile] + + +def test_get_competency_rule_profiles_excludes_archived( + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """get_competency_rule_profiles() leaves retired profiles out.""" + archived = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, + rule_payload=GRADE_PAYLOAD, + competency_taxonomy=competency_taxonomy, + archived=True, + ) + + profiles = list(get_competency_rule_profiles()) + + assert archived not in profiles + assert profiles == [default_rule_profile] + + +def test_get_competency_rule_profiles_is_ordered_by_id( + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + organization: Organization, +) -> None: + """ + get_competency_rule_profiles() returns profiles in ascending id order. + + Without a deterministic order, paginating the collection would repeat and skip rows. + """ + taxonomy_scoped = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=GRADE_PAYLOAD, competency_taxonomy=competency_taxonomy + ) + organization_scoped = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=GRADE_PAYLOAD, organization=organization + ) + + expected = [default_rule_profile, taxonomy_scoped, organization_scoped] + assert list(get_competency_rule_profiles()) == expected + assert list(get_competency_rule_profiles()) == expected + + +# ============================================================================================== +# create_leaf_group +# ============================================================================================== + + +def test_create_leaf_group_builds_the_full_hierarchy_on_first_use( + tag: Tag, course_run: CourseRun +) -> None: + """A first call with no existing groups creates a root, a course-level group, and a leaf.""" + 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 + assert root.parent is None + assert root.tag_id == tag.id + assert root.course_id is None + assert root.name == f"{tag.value} (root)" + assert course_level.tag_id == tag.id + assert course_level.course_id == course_run.id + assert course_level.name == f"{tag.value} — {course_run.title}" + assert leaf.tag_id == tag.id + assert leaf.course_id is None + assert leaf.logic_operator == LogicOperator.OR + + +def test_create_leaf_group_passes_through_an_explicit_logic_operator( + tag: Tag, course_run: CourseRun +) -> None: + """A supplied logic_operator is stored on the leaf as-is, not defaulted to OR.""" + leaf = create_leaf_group(tag, course_run, logic_operator=LogicOperator.AND) + assert leaf.logic_operator == LogicOperator.AND + + +def test_create_leaf_group_reuses_the_root_and_course_level_group_on_a_second_call( + tag: Tag, course_run: CourseRun +) -> None: + """A second call for the same tag and course reuses the root and course-level group.""" + first_leaf = create_leaf_group(tag, course_run) + second_leaf = create_leaf_group(tag, course_run) + + assert first_leaf.id != second_leaf.id + assert first_leaf.parent_id == second_leaf.parent_id + assert first_leaf.parent is not None + assert second_leaf.parent is not None + assert first_leaf.parent.parent_id == second_leaf.parent.parent_id + assert CompetencyCriteriaGroup.objects.filter(tag=tag, parent__isnull=True).count() == 1 + assert CompetencyCriteriaGroup.objects.filter(tag=tag, course=course_run).count() == 1 + + +def test_create_leaf_group_creates_a_new_course_level_group_for_a_different_course( + tag: Tag, course_run: CourseRun, organization: Organization +) -> None: + """A second course under the same tag gets its own course-level group, but shares the root.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + + first_leaf = create_leaf_group(tag, course_run) + second_leaf = create_leaf_group(tag, other_course_run) + + assert first_leaf.parent is not None + assert second_leaf.parent is not None + assert first_leaf.parent.parent_id == second_leaf.parent.parent_id + assert first_leaf.parent_id != second_leaf.parent_id + assert CompetencyCriteriaGroup.objects.filter(tag=tag, parent__isnull=True).count() == 1 + assert CompetencyCriteriaGroup.objects.filter(tag=tag, course__isnull=False).count() == 2 + + +def test_create_leaf_group_reuses_a_root_a_concurrent_request_already_committed( + tag: Tag, course_run: CourseRun +) -> None: + """ + A root created out-of-band (standing in for a concurrent request's winning commit) is reused. + + Callers are serialized on the tag row, so a request that waited on the lock finds the + winner's committed root and ``get_or_create()`` fetches it instead of creating a second one. + """ + already_committed_root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None, name="pre-existing root") + + leaf = create_leaf_group(tag, course_run) + + assert leaf.parent is not None + assert leaf.parent.parent_id == already_committed_root.id + assert CompetencyCriteriaGroup.objects.filter(tag=tag, parent__isnull=True).count() == 1 + + +def test_create_leaf_group_reuses_a_course_level_group_a_concurrent_request_already_committed( + tag: Tag, course_run: CourseRun +) -> None: + """The course-level group half of the same guarantee, isolated from the root.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None) + already_committed_course_level = CompetencyCriteriaGroup.objects.create( + tag=tag, course=course_run, parent=root, name="pre-existing course-level group", + ) + + leaf = create_leaf_group(tag, course_run) + + assert leaf.parent_id == already_committed_course_level.id + assert CompetencyCriteriaGroup.objects.filter(tag=tag, course=course_run).count() == 1 + + +def test_create_leaf_group_locks_the_tag_row_before_creating_groups(tag: Tag, course_run: CourseRun) -> None: + """ + create_leaf_group() takes a row lock on the tag, which is what serializes concurrent callers. + + MySQL has no partial unique index to reject a second root, so this lock is the only thing + preventing two roots for one tag. + """ + with mock.patch.object(Tag.objects, "select_for_update", wraps=Tag.objects.select_for_update) as lock: + create_leaf_group(tag, course_run) + + lock.assert_called_once_with() + + +def test_course_level_group_unique_constraint_rejects_a_second_group_for_the_same_tag_and_course( + tag: Tag, course_run: CourseRun +) -> None: + """ + The DB constraint backstopping create_leaf_group()'s course-level get_or_create() exists. + + Proven directly (bypassing get_or_create) so a silently-dropped migration can't go unnoticed. + """ + root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None) + CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run, parent=root) + with pytest.raises(IntegrityError): + CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run, parent=root) + + +# ============================================================================================== +# resolve_supplied_leaf_group +# ============================================================================================== + + +def test_resolve_supplied_leaf_group_returns_the_leaf_when_it_is_valid(tag: Tag, course_run: CourseRun) -> None: + """A group_id that names a genuine, matching leaf is returned unchanged.""" + leaf = create_leaf_group(tag, course_run) + assert resolve_supplied_leaf_group(leaf.id, tag, course_run) == leaf + + +def test_resolve_supplied_leaf_group_404s_when_the_group_does_not_exist(tag: Tag, course_run: CourseRun) -> None: + """An unknown group_id 404s rather than raising an unhandled 500.""" + with pytest.raises(Http404): + resolve_supplied_leaf_group(999999, tag, course_run) + + +def test_resolve_supplied_leaf_group_rejects_a_root_group_as_not_a_leaf(tag: Tag, course_run: CourseRun) -> None: + """A root group (no parent) is not a usable leaf.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None) + with pytest.raises(ValidationError, match="group_id"): + resolve_supplied_leaf_group(root.id, tag, course_run) + + +def test_resolve_supplied_leaf_group_rejects_a_course_level_group_as_not_a_leaf( + tag: Tag, course_run: CourseRun +) -> None: + """A course-level group (has its own course) is not a usable leaf either.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None) + course_level = CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run, parent=root) + with pytest.raises(ValidationError, match="group_id"): + resolve_supplied_leaf_group(course_level.id, tag, course_run) + + +def test_resolve_supplied_leaf_group_rejects_a_group_for_a_different_tag( + tag: Tag, course_run: CourseRun, competency_taxonomy: CompetencyTaxonomy +) -> None: + """A leaf that belongs to a different competency tag is rejected.""" + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Other Competency") + leaf = create_leaf_group(other_tag, course_run) + with pytest.raises(ValidationError, match="group_id"): + resolve_supplied_leaf_group(leaf.id, tag, course_run) + + +def test_resolve_supplied_leaf_group_rejects_a_group_for_a_different_course( + tag: Tag, course_run: CourseRun, organization: Organization +) -> None: + """A leaf whose course-level parent belongs to a different course is rejected.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + leaf = create_leaf_group(tag, other_course_run) + with pytest.raises(ValidationError, match="group_id"): + resolve_supplied_leaf_group(leaf.id, tag, course_run) + + +# ============================================================================================== +# associate_competency_criterion +# ============================================================================================== + + +def test_associate_competency_criterion_creates_the_hierarchy_and_criterion_when_nothing_exists( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile +) -> None: + """The happy path: no group_id, no existing groups, no rule fields supplied.""" + object_id = usage_key(course_run, "p1") + + criterion = associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + assert criterion.group.tag_id == tag.id + assert criterion.group.parent is not None + assert criterion.group.parent.course_id == course_run.id + root = criterion.group.parent.parent + assert root is not None + assert root.parent is None + assert root.tag_id == tag.id + assert criterion.object_tag.object_id == object_id + assert criterion.object_tag.tag_id == tag.id + assert criterion.rule_profile_id == default_rule_profile.id + assert criterion.rule_type_override is None + assert criterion.rule_payload_override is None + + +def test_associate_competency_criterion_uses_a_supplied_group_id(tag: Tag, course_run: CourseRun) -> None: + """A caller-supplied group_id is used as-is, rather than deriving or creating a new leaf.""" + leaf = create_leaf_group(tag, course_run) + object_id = usage_key(course_run, "p1") + + criterion = associate_competency_criterion(tag_id=tag.id, object_id=object_id, group_id=leaf.id) + + assert criterion.group_id == leaf.id + + +def test_associate_competency_criterion_rejects_group_id_and_logic_operator_together( + tag: Tag, course_run: CourseRun +) -> None: + """Supplying both group_id and logic_operator is rejected before anything is created.""" + leaf = create_leaf_group(tag, course_run) + object_id = usage_key(course_run, "p1") + + with pytest.raises(ValidationError, match="logic_operator"): + associate_competency_criterion( + tag_id=tag.id, object_id=object_id, group_id=leaf.id, logic_operator=LogicOperator.AND, + ) + + +def test_associate_competency_criterion_rejects_a_duplicate_tag_object_association( + tag: Tag, course_run: CourseRun +) -> None: + """Via the derive-or-create path (no group_id), a second criterion for the same pair is rejected.""" + object_id = usage_key(course_run, "p1") + associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + with pytest.raises(ValidationError, match="object_id"): + associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + +@pytest.mark.parametrize("supply_group_id", [True, False]) +def test_associate_competency_criterion_locks_the_tag_row_before_the_duplicate_check( + tag: Tag, course_run: CourseRun, supply_group_id: bool +) -> None: + """ + The tag row lock is taken before the duplicate check, so two concurrent requests can't both pass it. + + The order is asserted because a real concurrency test isn't practical. + """ + group_id = create_leaf_group(tag, course_run).id if supply_group_id else None + events: list[str] = [] + real_tag_lock = Tag.objects.select_for_update + real_object_tag_lock = ObjectTag.objects.select_for_update + + def record_tag_lock(*args, **kwargs): + events.append("tag lock") + return real_tag_lock(*args, **kwargs) + + def record_duplicate_check(*args, **kwargs): + events.append("duplicate check") + return real_object_tag_lock(*args, **kwargs) + + with ( + mock.patch.object(Tag.objects, "select_for_update", side_effect=record_tag_lock), + mock.patch.object(ObjectTag.objects, "select_for_update", side_effect=record_duplicate_check), + ): + associate_competency_criterion(tag_id=tag.id, object_id=usage_key(course_run, "p1"), group_id=group_id) + + assert events[:2] == ["tag lock", "duplicate check"] + + +def test_associate_competency_criterion_rejects_re_targeting_the_same_group( + tag: Tag, course_run: CourseRun +) -> None: + """Re-supplying the exact group a (tag_id, object_id) pair is already associated with is rejected.""" + object_id = usage_key(course_run, "p1") + first = associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + with pytest.raises(ValidationError, match="group_id"): + associate_competency_criterion(tag_id=tag.id, object_id=object_id, group_id=first.group_id) + + +def test_associate_competency_criterion_allows_a_different_explicit_group_for_the_same_pair( + tag: Tag, course_run: CourseRun +) -> None: + """ + A different, explicitly supplied existing group creates a second, deliberate association -- + not a duplicate. ADR-0002's own worked example requires the same tag/object association to + be able to participate in more than one CompetencyCriteriaGroup. + """ + object_id = usage_key(course_run, "p1") + first = associate_competency_criterion(tag_id=tag.id, object_id=object_id) + other_leaf = create_leaf_group(tag, course_run) + + second = associate_competency_criterion(tag_id=tag.id, object_id=object_id, group_id=other_leaf.id) + + assert second.object_tag_id == first.object_tag_id + assert second.group_id == other_leaf.id + assert CompetencyCriterion.objects.filter(object_tag_id=first.object_tag_id).count() == 2 + + +def test_associate_competency_criterion_404s_for_an_unknown_tag_id(course_run: CourseRun) -> None: + """An unresolvable tag_id 404s before anything else is validated.""" + object_id = usage_key(course_run, "p1") + with pytest.raises(Http404): + associate_competency_criterion(tag_id=999999, object_id=object_id) + + +def test_associate_competency_criterion_rejects_a_malformed_object_id(tag: Tag) -> None: + """An object_id that isn't a parseable usage key is a 400, keyed by object_id.""" + with pytest.raises(ValidationError, match="object_id"): + associate_competency_criterion(tag_id=tag.id, object_id="not-a-usage-key") + + +def test_associate_competency_criterion_rejects_an_unresolvable_course(tag: Tag) -> None: + """A well-formed usage key whose course has no matching CourseRun is a 400, keyed by object_id.""" + object_id = "block-v1:NoOrg+NoCourse+NoRun+type@sequential+block@p1" + with pytest.raises(ValidationError, match="object_id"): + associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + +def test_associate_competency_criterion_rolls_back_newly_created_groups_when_criterion_creation_fails( + monkeypatch: pytest.MonkeyPatch, tag: Tag, course_run: CourseRun +) -> None: + """ + A downstream failure during criterion creation rolls back any group this call created. + + ADR-0002 forbids persisting empty groups, so a failed attempt via the derive-or-create path + must not leave a root, course-level, or leaf group behind. Simulated here by making the final + CompetencyCriterion.objects.create() call itself fail -- standing in for any failure at that + point, including the containment check #666 will add right before it. + """ + def _reject(**_kwargs) -> None: + raise ValidationError({"object_id": "rejected for this test"}) + + monkeypatch.setattr(cbe_api.CompetencyCriterion.objects, "create", _reject) + object_id = usage_key(course_run, "p1") + + with pytest.raises(ValidationError): + associate_competency_criterion(tag_id=tag.id, object_id=object_id) + + assert not CompetencyCriteriaGroup.objects.filter(tag=tag).exists() diff --git a/tests/openedx_learning/applets/cbe/test_criteria_group.py b/tests/openedx_learning/applets/cbe/test_criteria_group.py index 39bb755f0..aeab04c37 100644 --- a/tests/openedx_learning/applets/cbe/test_criteria_group.py +++ b/tests/openedx_learning/applets/cbe/test_criteria_group.py @@ -49,9 +49,12 @@ def test_group_logic_operator_accepts_and_or_and_null_regardless_of_child_count( childless = CompetencyCriteriaGroup.objects.create(tag=tag, logic_operator=logic_operator) assert childless.pk is not None - parent = CompetencyCriteriaGroup.objects.create(tag=tag, logic_operator=logic_operator) - CompetencyCriteriaGroup.objects.create(tag=tag, parent=parent) - CompetencyCriteriaGroup.objects.create(tag=tag, parent=parent) + # A different tag from `childless`'s: the one-root-per-tag constraint allows only one root group + # per tag, and this scenario is about child count, not about tag reuse. + other_tag = Tag.objects.create(taxonomy=tag.taxonomy, value=f"{tag.value} (other)") + parent = CompetencyCriteriaGroup.objects.create(tag=other_tag, logic_operator=logic_operator) + CompetencyCriteriaGroup.objects.create(tag=other_tag, parent=parent) + CompetencyCriteriaGroup.objects.create(tag=other_tag, parent=parent) parent.refresh_from_db() assert parent.logic_operator == logic_operator assert CompetencyCriteriaGroup.objects.filter(parent=parent).count() == 2 diff --git a/tests/openedx_learning/applets/cbe/test_rules.py b/tests/openedx_learning/applets/cbe/test_rules.py new file mode 100644 index 000000000..be977cef2 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rules.py @@ -0,0 +1,56 @@ +""" +Tests for the CBE permission predicates and their registration. + +Fixtures live in this directory's conftest.py. +""" +import pytest +from django.contrib.auth.models import AnonymousUser +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user + +from openedx_learning.applets.cbe.rules import can_view_competency_rule_profile +from openedx_learning.models import CompetencyRuleProfile + +pytestmark = pytest.mark.django_db + +VIEW_RULE_PROFILE = "openedx_learning.view_competencyruleprofile" + + +def test_staff_may_view_rule_profiles(staff_user: UserType) -> None: + """A user who may administer taxonomies may read rule profiles.""" + assert can_view_competency_rule_profile(staff_user) is True + + +def test_non_staff_may_not_view_rule_profiles(user: UserType) -> None: + """A user who may not administer taxonomies may not read rule profiles.""" + assert can_view_competency_rule_profile(user) is False + + +def test_anonymous_may_not_view_rule_profiles() -> None: + """A caller the system cannot identify may not read rule profiles.""" + assert can_view_competency_rule_profile(AnonymousUser()) is False + + +def test_staff_may_view_a_specific_profile_and_non_staff_may_not( + staff_user: UserType, + user: UserType, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + A staff user may view a specific rule profile, and a non-staff user may not, matching each + one's permission on the collection. The profile argument is accepted for the scoped profiles + still to come; nothing depends on it yet. + """ + assert can_view_competency_rule_profile(staff_user, default_rule_profile) is True + assert can_view_competency_rule_profile(user, default_rule_profile) is False + + +def test_permission_is_registered_for_the_django_permission_check(staff_user: UserType, user: UserType) -> None: + """ + The predicate answers has_perm() under the name DRF's perms_map builds. + + This is what proves src/openedx_learning/rules.py reached the rules registry: without it + the applet's module is never imported and every caller is refused. + """ + assert staff_user.has_perm(VIEW_RULE_PROFILE) is True + assert user.has_perm(VIEW_RULE_PROFILE) is False + assert AnonymousUser().has_perm(VIEW_RULE_PROFILE) is False diff --git a/tests/openedx_learning/applets/cbe/test_views.py b/tests/openedx_learning/applets/cbe/test_views.py new file mode 100644 index 000000000..e97387146 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_views.py @@ -0,0 +1,632 @@ +""" +Tests for the CBE REST API views. + +Fixtures live in this directory's conftest.py. Several scenarios here create rule profile rows +directly rather than through a live call, because no create or archive endpoint exists yet. +""" +import pytest +import rules +from django.contrib.auth.models import User as UserType # pylint: disable=imported-auth-user +from django.urls import reverse +from organizations.models import Organization +from rest_framework import status +from rest_framework.test import APIClient + +from openedx_catalog.models import CatalogCourse, CourseRun +from openedx_learning.api import create_leaf_group +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + LogicOperator, + RuleType, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + +# A marker embedded in an object_id to signal "this caller has no object-level tagging +# permission", for the one test that needs a permission failure. oel_tagging.rules hardcodes +# change_objecttag_objectid to False ("should be defined in other apps for proper permission +# checking"): real Studio-role logic lives only in openedx-platform, never in this repo. Following +# tests/openedx_tagging/test_views.py's TestObjectTagViewSet.setUp() precedent, this override +# simulates a permitted caller for every object_id except ones carrying this marker. +UNAUTHORIZED_MARKER = "unauthorized" + + +@pytest.fixture(autouse=True) +def _allow_object_level_tagging() -> None: + """Let an ordinary authenticated user tag any object_id except one carrying UNAUTHORIZED_MARKER.""" + def _predicate(_user: UserType, object_id: str) -> bool: + return UNAUTHORIZED_MARKER not in object_id + + rules.set_perm("oel_tagging.change_objecttag_objectid", _predicate) + + +@pytest.fixture(name="user_client") +def _user_client(api_client: APIClient, user: UserType) -> APIClient: + """A REST client acting as `user`, an ordinary, non-staff, authenticated caller.""" + api_client.force_authenticate(user=user) + return api_client + + +def criterion_create_url(tag_id: int) -> str: + """Return the create-criterion endpoint's path for `tag_id`, resolved through the URL name.""" + return reverse("cbe:criterion-create", kwargs={"tag_id": tag_id}) + + +def usage_key(course_run: CourseRun, block_id: str) -> str: + """Build a gradeable-subsection-shaped usage key string under `course_run`.""" + key = course_run.course_key + assert key is not None + return f"block-v1:{key.org}+{key.course}+{key.run}+type@sequential+block@{block_id}" + + +def make_course_run(organization: Organization, course_code: str, run_code: str) -> CourseRun: + """Create a CourseRun distinct from the `course_run` fixture, for the different-course tests.""" + catalog_course = CatalogCourse.objects.create(org=organization, course_code=course_code) + return CourseRun.objects.create(catalog_course=catalog_course, run_code=run_code) + + +# What migration 0005 seeds the system default with. Asserted verbatim rather than imported, so +# that a change to the seed surfaces here as a failing contract instead of passing silently. +SEEDED_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + +# A different valid payload for rows these tests create, so no assertion about the seeded row +# can pass by accident against a row a test made itself. +FIXTURE_GRADE_PAYLOAD = {"op": "gte", "value": 0.6, "scale": "percent"} + +RESPONSE_FIELDS = {"id", "scope_type", "rule_type", "rule_payload", "archived"} + + +def rule_profiles_url() -> str: + """Return the collection's path, resolved through the router rather than hardcoded.""" + return reverse("cbe:rule_profile-list") + + +def make_profile(**scope) -> CompetencyRuleProfile: + """Create a live rule profile at `scope`, which is at most one of the three scope columns.""" + return CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=dict(FIXTURE_GRADE_PAYLOAD), **scope + ) + + +def test_collection_resolves_to_the_documented_path() -> None: + """ + The router and the mounts compose into the path the CBE tickets name. + + Pinned in one place so the rest of these tests can use reverse() instead. + """ + assert rule_profiles_url() == "/api/cbe/v1/rule_profiles/" + + +def test_read_the_rule_the_instance_requires_for_mastery( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + A permitted caller reads the instance-wide default, complete enough to act on. + + The threshold arrives with the scale it is expressed on, so a caller cannot read the + fraction as a percentage or the reverse. + """ + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + assert len(response.data["results"]) == 1 + profile = response.data["results"][0] + assert profile["id"] == default_rule_profile.id + assert profile["scope_type"] == "system_default" + assert profile["rule_type"] == "Grade" + assert profile["rule_payload"] == SEEDED_GRADE_PAYLOAD + assert profile["archived"] is False + + +def test_response_omits_internal_scope_bookkeeping( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """The response describes scope in terms a caller outside this system can act on.""" + response = staff_client.get(rule_profiles_url()) + + profile = response.data["results"][0] + assert profile["id"] == default_rule_profile.id + assert set(profile.keys()) == RESPONSE_FIELDS + assert "scope_code" not in profile + assert "organization" not in profile + assert "course" not in profile + assert "competency_taxonomy" not in profile + + +def test_every_profile_reports_the_scope_it_applies_to( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """ + A caller tells the instance-wide default apart from a narrower profile by scope alone. + + Each row here holds a different scope_code, which is what shows the derivation reads the + scope columns rather than matching that string. + """ + taxonomy_scoped = make_profile(competency_taxonomy=competency_taxonomy) + course_scoped = make_profile(course=course_run) + organization_scoped = make_profile(organization=organization) + + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + scope_types = {row["id"]: row["scope_type"] for row in response.data["results"]} + assert scope_types == { + default_rule_profile.id: "system_default", + taxonomy_scoped.id: "taxonomy", + course_scoped.id: "course", + organization_scoped.id: "organization", + } + + +def test_retired_profiles_are_left_out( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """A retired profile is not returned, while the instance-wide default still is.""" + archived = make_profile(competency_taxonomy=competency_taxonomy, archived=True) + + response = staff_client.get(rule_profiles_url()) + + returned_ids = [row["id"] for row in response.data["results"]] + assert archived.id not in returned_ids + assert returned_ids == [default_rule_profile.id] + + +def test_instance_with_no_rule_profiles_reports_an_empty_collection(staff_client: APIClient) -> None: + """ + An instance holding no profiles is an empty collection, not a missing resource. + + Migration 0005 seeds the system default into the test database, so the rows are cleared + explicitly here rather than assuming an empty table. + """ + CompetencyRuleProfile.objects.all().delete() + + response = staff_client.get(rule_profiles_url()) + + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == 0 + assert response.data["results"] == [] + + +def test_response_is_the_paginated_envelope( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """The collection arrives in an envelope, never as a bare array.""" + response = staff_client.get(rule_profiles_url()) + + assert isinstance(response.data, dict) + assert {"count", "next", "previous", "results"} <= set(response.data.keys()) + assert isinstance(response.data["results"], list) + assert [row["id"] for row in response.data["results"]] == [default_rule_profile.id] + + +def test_a_collection_larger_than_one_response_arrives_whole( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """ + Following the link to the remainder yields every profile once, with none skipped. + + The page size divides the four profiles unevenly on purpose, so a partial last page is + exercised rather than a run of exactly full ones. + """ + expected_ids = [ + default_rule_profile.id, + make_profile(competency_taxonomy=competency_taxonomy).id, + make_profile(course=course_run).id, + make_profile(organization=organization).id, + ] + + collected_ids: list[int] = [] + pages = 0 + next_url = f"{rule_profiles_url()}?page_size=3" + while next_url: + response = staff_client.get(next_url) + assert response.status_code == status.HTTP_200_OK + assert response.data["count"] == len(expected_ids) + collected_ids.extend(row["id"] for row in response.data["results"]) + next_url = response.data["next"] + pages += 1 + + assert pages == 2, "page_size=3 should have split four profiles across two responses" + assert collected_ids == expected_ids + + +def test_profiles_arrive_in_the_same_order_every_time( + staff_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, +) -> None: + """Two requests with nothing changing in between return the profiles in the same order.""" + make_profile(competency_taxonomy=competency_taxonomy) + make_profile(course=course_run) + make_profile(organization=organization) + + first = [row["id"] for row in staff_client.get(rule_profiles_url()).data["results"]] + second = [row["id"] for row in staff_client.get(rule_profiles_url()).data["results"]] + + assert first == second + assert first == sorted(first) + assert default_rule_profile.id in first + + +@pytest.mark.parametrize( + "user_fixture, expected_status", + [ + (None, status.HTTP_401_UNAUTHORIZED), + ("user", status.HTTP_403_FORBIDDEN), + ("staff_user", status.HTTP_200_OK), + ], +) +def test_only_a_competency_administrator_may_read_the_collection( + request: pytest.FixtureRequest, + api_client: APIClient, + default_rule_profile: CompetencyRuleProfile, + user_fixture: str | None, + expected_status: int, +) -> None: + """An unidentified caller and an unpermitted one are both refused, and get no profile.""" + if user_fixture is not None: + api_client.force_authenticate(user=request.getfixturevalue(user_fixture)) + + response = api_client.get(rule_profiles_url()) + + assert response.status_code == expected_status + if expected_status == status.HTTP_200_OK: + assert [row["id"] for row in response.data["results"]] == [default_rule_profile.id] + else: + assert "results" not in response.data + + +# ============================================================================================== +# CompetencyCriterionCreateView (#665) +# ============================================================================================== + + +def test_no_group_no_existing_groups_creates_the_full_hierarchy_and_201s( + user_client: APIClient, tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """The happy path: no group_id, no groups yet, no rule fields supplied.""" + object_id = usage_key(course_run, "p1") + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_201_CREATED + criterion = CompetencyCriterion.objects.get(pk=response.data["id"]) + assert criterion.group.tag_id == tag.id + course_level = criterion.group.parent + assert course_level is not None + assert course_level.course_id == course_run.id + root = course_level.parent + assert root is not None + assert root.parent is None + assert response.data["object_tag_id"] == criterion.object_tag_id + assert response.data["group_id"] == criterion.group_id + assert response.data["rule_profile_id"] == default_rule_profile.id + + +def test_logic_operator_provided_is_stored_on_the_new_leaf( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """A supplied logic_operator is stored on the newly created leaf, not defaulted.""" + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "logic_operator": "AND"}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + criterion = CompetencyCriterion.objects.get(pk=response.data["id"]) + assert criterion.group.logic_operator == LogicOperator.AND + + +def test_logic_operator_omitted_defaults_to_or(user_client: APIClient, tag: Tag, course_run: CourseRun) -> None: + """Omitting logic_operator on the derive-or-create path stores "OR" literally.""" + object_id = usage_key(course_run, "p1") + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_201_CREATED + criterion = CompetencyCriterion.objects.get(pk=response.data["id"]) + assert criterion.group.logic_operator == LogicOperator.OR + + +def test_group_id_and_logic_operator_together_is_rejected( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """Supplying both group_id and logic_operator is a 400, before anything is created.""" + leaf = create_leaf_group(tag, course_run) + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), + {"object_id": object_id, "group_id": leaf.id, "logic_operator": "AND"}, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "logic_operator" in response.data + + +def test_course_level_group_is_reused_within_the_same_course( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """Two criteria created for the same tag and course share one course-level group.""" + first = user_client.post( + criterion_create_url(tag.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + second = user_client.post( + criterion_create_url(tag.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert first.status_code == status.HTTP_201_CREATED + assert second.status_code == status.HTTP_201_CREATED + first_group = CompetencyCriterion.objects.get(pk=first.data["id"]).group + second_group = CompetencyCriterion.objects.get(pk=second.data["id"]).group + assert first_group.id != second_group.id + assert first_group.parent_id == second_group.parent_id + + +def test_new_course_level_group_for_a_different_course( + user_client: APIClient, tag: Tag, course_run: CourseRun, organization: Organization, +) -> None: + """A second course under the same tag gets its own course-level group, but shares the root.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + + first = user_client.post( + criterion_create_url(tag.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + second = user_client.post( + criterion_create_url(tag.id), {"object_id": usage_key(other_course_run, "p1")}, format="json", + ) + + assert first.status_code == status.HTTP_201_CREATED + assert second.status_code == status.HTTP_201_CREATED + first_group = CompetencyCriterion.objects.get(pk=first.data["id"]).group + second_group = CompetencyCriterion.objects.get(pk=second.data["id"]).group + assert first_group.parent_id != second_group.parent_id + assert first_group.parent is not None + assert second_group.parent is not None + assert first_group.parent.parent_id == second_group.parent.parent_id + + +def test_supplied_leaf_group_happy_path(user_client: APIClient, tag: Tag, course_run: CourseRun) -> None: + """Supplying an existing, valid leaf group_id uses it directly.""" + leaf = create_leaf_group(tag, course_run) + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": leaf.id}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["group_id"] == leaf.id + + +def test_supplied_group_non_leaf_is_rejected(user_client: APIClient, tag: Tag, course_run: CourseRun) -> None: + """A supplied group_id that names a root (non-leaf) group is a 400.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag, parent=None) + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": root.id}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "group_id" in response.data + + +def test_supplied_group_for_a_different_competency_is_rejected( + user_client: APIClient, tag: Tag, course_run: CourseRun, competency_taxonomy: CompetencyTaxonomy, +) -> None: + """A supplied group_id that belongs to a different competency tag is a 400.""" + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Other Competency") + other_leaf = create_leaf_group(other_tag, course_run) + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": other_leaf.id}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "group_id" in response.data + + +def test_supplied_group_for_a_different_course_is_rejected( + user_client: APIClient, tag: Tag, course_run: CourseRun, organization: Organization, +) -> None: + """A supplied group_id whose course-level parent belongs to a different course is a 400.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + other_leaf = create_leaf_group(tag, other_course_run) + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": other_leaf.id}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "group_id" in response.data + + +def test_duplicate_association_supplying_the_same_group_is_rejected( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """A second criterion for the same (tag, object_id), re-supplying the group it just created, is a 400.""" + object_id = usage_key(course_run, "p1") + first = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + first_group_id = first.data["group_id"] + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": first_group_id}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "group_id" in response.data + + +def test_duplicate_association_supplying_a_different_existing_leaf_creates_a_second_criterion( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """ + A second criterion for the same (tag, object_id), explicitly aimed at a *different* existing + leaf, is a deliberate second association, not a duplicate -- per ADR-0002's worked example, + the same tag/object association may participate in more than one CompetencyCriteriaGroup. + Only re-targeting the exact same group already used is rejected (see the sibling test). + """ + object_id = usage_key(course_run, "p1") + first = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + first_group_id = first.data["group_id"] + other_leaf = create_leaf_group(tag, course_run) + groups_before = CompetencyCriteriaGroup.objects.count() + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": other_leaf.id}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["group_id"] == other_leaf.id + # No new groups: other_leaf already existed and was supplied explicitly. + assert CompetencyCriteriaGroup.objects.count() == groups_before + criterion_group_ids = set( + CompetencyCriterion.objects.filter(object_tag_id=response.data["object_tag_id"]) + .values_list("group_id", flat=True) + ) + assert criterion_group_ids == {first_group_id, other_leaf.id} + + +def test_duplicate_association_via_the_derive_path_is_rejected_before_creating_a_group( + user_client: APIClient, tag: Tag, course_run: CourseRun, +) -> None: + """ + A second criterion for the same (tag, object_id), with no group_id, is rejected before any new group. + + Rejected regardless of which group the existing criterion is actually in: with no group_id + supplied, there's no way to know which group slot this request intends, so any existing + criterion for the pair counts as a duplicate. + """ + object_id = usage_key(course_run, "p1") + user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + groups_before = CompetencyCriteriaGroup.objects.count() + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "object_id" in response.data + assert CompetencyCriteriaGroup.objects.count() == groups_before + + +def test_all_null_rule_fields_resolve_to_the_system_default_profile_id( + user_client: APIClient, tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + Omitting rule_profile_id, rule_type_override, and rule_payload_override together resolves + rule_profile_id to the seeded system-default profile's id, not null. + + CompetencyCriterion's own oel_cbe_criterion_profile_xor_override_check constraint never + allows all three fields null. + """ + object_id = usage_key(course_run, "p1") + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_201_CREATED + assert response.data["rule_profile_id"] == default_rule_profile.id + assert response.data["rule_type_override"] is None + assert response.data["rule_payload_override"] is None + + +def test_reusing_a_subsection_already_tagged_with_a_different_competency_preserves_both_tags( + user_client: APIClient, tag: Tag, course_run: CourseRun, competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + Creating a criterion for a second competency on an already-tagged object merges, not overwrites. + + Both tags here come from the SAME CompetencyTaxonomy, per the plan's explicit note: only the + same-taxonomy case exercises tag_object()'s replace-the-whole-list behavior, since two + different taxonomies' ObjectTag rows would never collide even against a naive + overwrite-instead-of-merge implementation. + """ + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Other Competency") + object_id = usage_key(course_run, "p1") + + first = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + second = user_client.post(criterion_create_url(other_tag.id), {"object_id": object_id}, format="json") + + assert first.status_code == status.HTTP_201_CREATED + assert second.status_code == status.HTTP_201_CREATED + applied_tag_ids = set( + ObjectTag.objects.filter(object_id=object_id, taxonomy=competency_taxonomy).values_list("tag_id", flat=True) + ) + assert applied_tag_ids == {tag.id, other_tag.id} + + +def test_competency_not_found_404s(user_client: APIClient, course_run: CourseRun) -> None: + """An unresolvable tag_id (URL path parameter) 404s.""" + object_id = usage_key(course_run, "p1") + + response = user_client.post(criterion_create_url(999999), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_group_not_found_404s(user_client: APIClient, tag: Tag, course_run: CourseRun) -> None: + """An unresolvable group_id 404s.""" + object_id = usage_key(course_run, "p1") + + response = user_client.post( + criterion_create_url(tag.id), {"object_id": object_id, "group_id": 999999}, format="json", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + + +def test_missing_object_id_is_400(user_client: APIClient, tag: Tag) -> None: + """A request with no object_id at all is a 400.""" + response = user_client.post(criterion_create_url(tag.id), {}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "object_id" in response.data + + +def test_malformed_object_id_is_400(user_client: APIClient, tag: Tag) -> None: + """An object_id that isn't a parseable usage key is a 400, keyed by object_id.""" + response = user_client.post(criterion_create_url(tag.id), {"object_id": "not-a-usage-key"}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "object_id" in response.data + + +def test_unresolvable_course_is_400(user_client: APIClient, tag: Tag) -> None: + """A well-formed usage key whose course has no matching CourseRun is a 400.""" + object_id = "block-v1:NoOrg+NoCourse+NoRun+type@sequential+block@p1" + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "object_id" in response.data + + +def test_no_permission_is_403(user_client: APIClient, tag: Tag, course_run: CourseRun) -> None: + """A caller without object-level tagging permission on this object_id is refused with a 403.""" + object_id = usage_key(course_run, UNAUTHORIZED_MARKER) + + response = user_client.post(criterion_create_url(tag.id), {"object_id": object_id}, format="json") + + assert response.status_code == status.HTTP_403_FORBIDDEN