diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index e2268c297..aa52d33f7 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -3,9 +3,15 @@ """ from __future__ import annotations +import logging +from datetime import datetime, timezone +from decimal import Decimal +from typing import Any, Sequence + from django.core.exceptions import ValidationError -from django.db import transaction +from django.db import connection, transaction from django.db.models import QuerySet +from django.db.transaction import TransactionManagementError from django.utils.translation import gettext_lazy as _ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey @@ -15,25 +21,33 @@ from openedx_tagging.api import create_taxonomy, get_object_tags, tag_object from openedx_tagging.models import ObjectTag, Tag, Taxonomy +from .data import GradedObjectScore from .models import ( CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, CompetencyTaxonomy, LogicOperator, + MasteryStatus, + StudentCompetencyCriterionStatus, ) +from .rule_payloads import evaluate_rule __all__ = [ + "GradedObjectScore", "associate_competency_criterion", "create_competency_criterion", "create_competency_taxonomy", "get_competency_rule_profiles", "is_competency_taxonomy", + "record_graded_object_statuses", "resolve_competency_tag", "resolve_supplied_leaf_group", "select_competency_taxonomies", ] +log = logging.getLogger(__name__) + def create_competency_taxonomy( # pylint: disable=too-many-positional-arguments name: str, @@ -345,3 +359,95 @@ def associate_competency_criterion( rule_type_override=rule_type_override, rule_payload_override=rule_payload_override, ) + + +def _effective_rule(criterion: CompetencyCriterion) -> tuple[str, Any]: + """Return (rule_type, payload) for whichever of criterion's profile or override is set.""" + if criterion.rule_profile_id is not None: + # The oel_cbe_criterion_profile_xor_override_check constraint guarantees rule_profile is + # set whenever rule_profile_id is; this assertion only narrows that fact for mypy. + assert criterion.rule_profile is not None + return criterion.rule_profile.rule_type, criterion.rule_profile.rule_payload + # Same constraint, the other way around: rule_type_override is set whenever rule_profile_id + # is not. + assert criterion.rule_type_override is not None + return criterion.rule_type_override, criterion.rule_payload_override + + +def _raise_leaf_status(user_id: int, criterion_id: int, status: MasteryStatus, now: datetime) -> int: + """ + Create or raise a learner's leaf status row for one criterion; never lowers a stored status. + + Returns the count of rows created or raised (0 or 1), matching the semantics of + ``QuerySet.update()``'s own return value. + """ + row, created = StudentCompetencyCriterionStatus.objects.get_or_create( + user_id=user_id, criterion_id=criterion_id, + defaults={"status_id": status, "created": now, "modified": now}, + ) + if created: + return 1 + return StudentCompetencyCriterionStatus.objects.filter( + pk=row.pk, status_id__lt=status, + ).update(status_id=status, modified=now) + + +def record_graded_object_statuses(*, user_id: int, scores: Sequence[GradedObjectScore]) -> int: + """ + Record a learner's leaf-level competency status for each graded object in ``scores``. + + UNSTABLE: the rollup and learner-status read entry points still to come may change what this + function returns without a deprecation cycle. + + Call this only from inside the same transaction that also writes the grade: this function opens + no transaction of its own and lets any storage error propagate rather than swallowing it, so the + grade and its leaf statuses always commit or roll back together. Pass each ``object_id`` at most + once, using the exact string already stored in ``ObjectTag.object_id``; the match performed + against that value is case-sensitive. Leave out of ``scores`` any object worth zero possible + points and any object the learner has not attempted. Only criteria whose tag belongs to a + competency taxonomy are evaluated, and archived criteria are skipped. An evaluated criterion's + status is only ever raised, never lowered. The return value is the count of leaf rows created or + raised, where ``0`` means nothing changed. This function performs no authorization check of its + own; as with ``openedx_tagging.api``, the caller is responsible for enforcing permissions. + + :raises TransactionManagementError: if called outside an atomic block. + """ + # In autocommit mode get_or_create() would commit a leaf row on its own, apart from the grade. + if not connection.in_atomic_block: + raise TransactionManagementError("record_graded_object_statuses() must run inside the grade's transaction.") + if not scores: + return 0 + + fraction_by_object_id: dict[str, Decimal] = {} + for score in scores: + if score.object_id in fraction_by_object_id: + raise ValueError(f"duplicate object_id in scores: {score.object_id!r}") + fraction_by_object_id[score.object_id] = score.fraction + + criteria = ( + CompetencyCriterion.objects.filter( + object_tag__object_id__in=list(fraction_by_object_id), + object_tag__taxonomy__competencytaxonomy__isnull=False, + archived=False, + ) + .select_related("object_tag", "rule_profile") + .order_by("pk") + ) + + now = datetime.now(tz=timezone.utc) + changed = 0 + for criterion in criteria: + fraction = fraction_by_object_id[criterion.object_tag.object_id] + rule_type, payload = _effective_rule(criterion) + try: + demonstrated = evaluate_rule(rule_type, payload, fraction) + except ValidationError as exc: + log.warning( + "Skipping CompetencyCriterion %s for user %s on object %s: its rule cannot be applied: %s", + criterion.pk, user_id, criterion.object_tag.object_id, exc.messages, + ) + continue + status = MasteryStatus.DEMONSTRATED if demonstrated else MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + changed += _raise_leaf_status(user_id, criterion.pk, status, now) + + return changed diff --git a/src/openedx_learning/applets/cbe/data.py b/src/openedx_learning/applets/cbe/data.py new file mode 100644 index 000000000..a985c6eba --- /dev/null +++ b/src/openedx_learning/applets/cbe/data.py @@ -0,0 +1,38 @@ +""" +Value objects passed across the CBE applet's public API boundary. +""" +from __future__ import annotations + +from dataclasses import dataclass +from decimal import Decimal + + +@dataclass(frozen=True, kw_only=True, slots=True) +class GradedObjectScore: + """ + One learner's final, override-adjusted fraction earned on one graded object. + + ``fraction`` is the caller's final answer for how much of ``object_id`` this learner earned; + this library cannot detect whether an override was already folded into it. Leave an object + worth zero possible points out of the list entirely rather than passing it with a + ``Decimal("0")`` fraction, since it has no meaningful fraction to evaluate. ``Decimal("0")`` + itself is a valid, meaningful input: it means the learner attempted the object and earned + nothing on it. + + ``fraction`` must be built from a string or from ``Decimal`` arithmetic, never from a + ``float``: ``Decimal(0.7)`` is a ``Decimal`` instance, so it passes this class's own type + check, but its value is ``0.69999999999999995559...``, not ``0.7``. + """ + + object_id: str + fraction: Decimal + + def __post_init__(self) -> None: + """Enforce this class's type and range contract once, at construction time.""" + if not isinstance(self.fraction, Decimal): + raise TypeError(f"fraction must be a Decimal, not {type(self.fraction)!r}") + # is_finite() must run first: 0 <= Decimal("NaN") raises decimal.InvalidOperation, not + # ValueError, so checking finiteness first is what makes every invalid input, including + # 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}") diff --git a/src/openedx_learning/applets/cbe/rule_payloads.py b/src/openedx_learning/applets/cbe/rule_payloads.py index 931eba76e..1740152d2 100644 --- a/src/openedx_learning/applets/cbe/rule_payloads.py +++ b/src/openedx_learning/applets/cbe/rule_payloads.py @@ -3,11 +3,14 @@ the shape its rule_type defines. See :ref:`openedx-learning-adr-0002` Decision 3 for the payload contract. ``RuleType`` declares exactly the rule types with a shape defined here, so a rule type can never be offered as a choice without also being saveable. These messages reach an API caller -or admin form, so they must not leak internal class or function names. +or admin form, so they must not leak internal class or function names. ``evaluate_rule()`` applies a +validated payload to a learner's score fraction and returns whether the rule is met. """ from __future__ import annotations -from typing import Literal, TypedDict, get_args +import operator +from decimal import Decimal +from typing import Callable, Literal, TypedDict from django.core.exceptions import ValidationError from django.db import models @@ -16,6 +19,7 @@ __all__ = [ "GradeRulePayload", "RuleType", + "evaluate_rule", "validate_rule_payload", ] @@ -35,7 +39,15 @@ class RuleType(models.TextChoices): GradeOperator = Literal["gte", "lte", "eq"] -_GRADE_OPERATORS: frozenset[str] = frozenset(get_args(GradeOperator)) +# The single source of truth for which operators exist: _GRADE_OPERATORS is derived from this +# map's keys, rather than independently from get_args(GradeOperator), so validate_rule_payload +# and evaluate_rule can never drift apart on which operators are supported. +_GRADE_OPERATOR_FUNCS: dict[str, Callable[[Decimal, Decimal], bool]] = { + "gte": operator.ge, + "lte": operator.le, + "eq": operator.eq, +} +_GRADE_OPERATORS: frozenset[str] = frozenset(_GRADE_OPERATOR_FUNCS) class GradeRulePayload(TypedDict): @@ -107,3 +119,23 @@ def validate_rule_payload(rule_type: str, payload: object) -> None: _("Rule type '%(rule_type)s' is not supported yet; only 'Grade' has a defined rule_payload shape.") % {"rule_type": rule_type} ) + + +def evaluate_rule(rule_type: str, payload: object, fraction: Decimal) -> bool: + """ + Return whether ``fraction`` satisfies ``rule_type``'s ``payload``. + + Validates the payload's shape first (see :func:`validate_rule_payload`); raises + ``ValidationError`` and never returns a value when the shape is invalid, or when + ``rule_type`` has no defined evaluation yet (today, everything except ``RuleType.GRADE``). + """ + validate_rule_payload(rule_type, payload) + if rule_type != RuleType.GRADE: + raise ValidationError( + _("no evaluator is defined yet for rule type '%(rule_type)s'") % {"rule_type": rule_type} + ) + # validate_rule_payload already confirmed payload is a dict with a Grade payload's exact + # keys; this assertion only narrows that fact for mypy, which sees payload as `object`. + assert isinstance(payload, dict) + threshold = Decimal(str(payload["value"])) + return _GRADE_OPERATOR_FUNCS[payload["op"]](fraction, threshold) diff --git a/src/openedx_learning/models_api.py b/src/openedx_learning/models_api.py index 4f02b4f98..998dd99ca 100644 --- a/src/openedx_learning/models_api.py +++ b/src/openedx_learning/models_api.py @@ -1,9 +1,18 @@ """ -Models that we want callers to extend or make foreign keys to. +Models that we want callers to extend, make foreign keys to, or use in type annotations. -This is also the stable import point for the model class itself, for callers that -need to create competency taxonomies directly. +Other apps should create and change these rows, including test data, only through +`openedx_learning.api`, which keeps the invariants that direct model writes would skip. """ # pylint: disable=unused-import -from .models import CompetencyTaxonomy +from .models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + RuleType, + StudentCompetencyCriterionStatus, +) diff --git a/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py b/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py new file mode 100644 index 000000000..c19f5f569 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_record_graded_object_statuses.py @@ -0,0 +1,520 @@ +""" +Tests for `record_graded_object_statuses`, exercising each acceptance-criteria scenario for +leaf-level competency status recording. +""" +import logging +from datetime import datetime, timezone +from decimal import Decimal + +import pytest +from django.db import DatabaseError, transaction +from django.db.transaction import TransactionManagementError +from freezegun import freeze_time + +from openedx_learning.api import GradedObjectScore, record_graded_object_statuses +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + RuleType, + StudentCompetencyCriterionStatus, +) +from openedx_tagging.models import ObjectTag, Tag, Taxonomy + +pytestmark = pytest.mark.django_db + + +def _make_criterion( + *, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + rule_profile: CompetencyRuleProfile | None = None, + override: tuple[str, dict] | None = None, +) -> CompetencyCriterion: + """Build a leaf CompetencyCriterion using whichever of rule_profile or override the caller passes.""" + rule_type_override, rule_payload_override = override if override is not None else (None, None) + return CompetencyCriterion.objects.create( + group=group, + object_tag=object_tag, + rule_profile=rule_profile, + rule_type_override=rule_type_override, + rule_payload_override=rule_payload_override, + ) + + +@pytest.fixture(name="criterion") +def _criterion( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> CompetencyCriterion: + """A leaf CompetencyCriterion directly under the root `group`, using the seeded default profile.""" + return _make_criterion(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + + +@pytest.fixture(name="rule_profile_75") +def _rule_profile_75(competency_taxonomy: CompetencyTaxonomy) -> CompetencyRuleProfile: + """A CompetencyRuleProfile scoped to `competency_taxonomy`, requiring at least 0.75.""" + return CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, + rule_type=RuleType.GRADE, + rule_payload={"op": "gte", "value": 0.75, "scale": "percent"}, + ) + + +def test_grade_meeting_threshold_demonstrates( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile +) -> None: + """A grade of 80% against a 75% threshold demonstrates the criterion.""" + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.80"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_grade_below_threshold_is_attempted_not_demonstrated( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile +) -> None: + """A grade of 60% against a 75% threshold is attempted but not demonstrated.""" + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.60"))], + ) + + assert changed == 1 + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + + +@pytest.mark.parametrize( + "fraction, expected_status", + [ + pytest.param(Decimal("0.74"), MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, id="just_below"), + pytest.param(Decimal("0.76"), MasteryStatus.DEMONSTRATED, id="just_above"), + pytest.param(Decimal("0.749999999999"), MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, id="fractionally_below"), + pytest.param(Decimal("0.7500000000"), MasteryStatus.DEMONSTRATED, id="exact_with_trailing_zeros"), + pytest.param(Decimal("0.7500000001"), MasteryStatus.DEMONSTRATED, id="fractionally_above"), + ], +) +def test_threshold_boundary_precision( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, rule_profile_75: CompetencyRuleProfile, + *, fraction: Decimal, expected_status: MasteryStatus, +) -> None: + """ + Decimal arithmetic is exact, so a fraction on either side of a 0.75 threshold -- including at + extreme precision, and including one with trailing zeros -- resolves without rounding error. + """ + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=fraction)], + ) + + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == expected_status + + +def test_score_exactly_at_default_threshold_demonstrates( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """ + A score of exactly Decimal("0.8") against the seeded default profile (at least 0.8) demonstrates + the criterion. This is the regression test for comparing a Decimal fraction against a rule_payload + value that Django's JSONField returns as a plain float. + """ + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_object_with_no_criteria_costs_one_query_and_writes_nothing(user, django_assert_num_queries) -> None: + """Content with no competency criteria is unaffected, costing exactly one query.""" + score = GradedObjectScore(object_id="block-v1:Org1+Python100+Fall2026+problem+none", fraction=Decimal("0.9")) + with django_assert_num_queries(1): + changed = record_graded_object_statuses(user_id=user.id, scores=[score]) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_object_tagged_with_unrelated_tag_writes_nothing(user) -> None: + """A subsection tagged with a tag no criterion points at is unaffected.""" + plain_taxonomy = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v1") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Unrelated") + plain_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+unrelated", taxonomy=plain_taxonomy, tag=plain_tag, + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=plain_object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_empty_scores_costs_no_queries(user, django_assert_num_queries) -> None: + """An empty scores sequence returns 0 and issues no queries at all.""" + with django_assert_num_queries(0): + changed = record_graded_object_statuses(user_id=user.id, scores=[]) + + assert changed == 0 + + +def test_one_score_evaluates_each_criterion_on_its_own_threshold( + user, competency_taxonomy: CompetencyTaxonomy, group: CompetencyCriteriaGroup, object_tag: ObjectTag, + rule_profile_75: CompetencyRuleProfile, +) -> None: + """ + One grade satisfies two criteria with different thresholds: two competency tags each carry their + own ObjectTag sharing one object_id, one criterion resolved via a profile, the other via a + per-criterion override, and a single score demonstrates one but not the other. + """ + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Other Competency") + other_object_tag = ObjectTag.objects.create( + object_id=object_tag.object_id, taxonomy=competency_taxonomy, tag=other_tag, + ) + other_group = CompetencyCriteriaGroup.objects.create(tag=other_tag) + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=rule_profile_75) + other_criterion = _make_criterion( + group=other_group, object_tag=other_object_tag, + override=(RuleType.GRADE, {"op": "gte", "value": 0.9, "scale": "percent"}), + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 2 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=other_criterion).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + + +def test_lower_grade_does_not_lower_demonstrated( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A later, lower grade does not take away an already-demonstrated status.""" + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + assert first == 1 + + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.6"))], + ) + + assert second == 0 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_higher_grade_raises_attempted_to_demonstrated( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A later, higher grade raises a stored status, and only `modified` moves.""" + first_time = datetime(2026, 1, 1, tzinfo=timezone.utc) + second_time = datetime(2026, 1, 2, tzinfo=timezone.utc) + + with freeze_time(first_time): + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.6"))], + ) + assert first == 1 + + with freeze_time(second_time): + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + assert second == 1 + + row = StudentCompetencyCriterionStatus.objects.get(criterion=criterion) + assert row.status_id == MasteryStatus.DEMONSTRATED + assert row.created == first_time + assert row.modified == second_time + + +def test_identical_repeat_changes_nothing(user, criterion: CompetencyCriterion, object_tag: ObjectTag) -> None: + """Repeating an identical evaluation changes nothing, including `modified`.""" + first = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + assert first == 1 + modified_after_first = StudentCompetencyCriterionStatus.objects.get(criterion=criterion).modified + + second = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert second == 0 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).modified == modified_after_first + + +@pytest.mark.parametrize( + "corrupt_kwargs", + [ + pytest.param({"rule_type_override": "MasteryLevel"}, id="unsupported_rule_type"), + pytest.param( + {"rule_payload_override": {"op": "invalid_op", "value": 0.8, "scale": "percent"}}, + id="invalid_operator", + ), + ], +) +def test_unappliable_rule_is_skipped_and_logged( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, + caplog: pytest.LogCaptureFixture, corrupt_kwargs: dict, +) -> None: + """ + A criterion whose rule cannot be applied is passed over: the well-formed sibling criterion still + gets its row and the function still returns 1, and a WARNING names the skipped criterion, the + learner, and the object. + + The bad criterion is first created with a valid override, then corrupted with a direct queryset + `update()`, which bypasses `CompetencyCriterion.save()`'s own validation: `save()` itself would + reject either corrupted shape. + """ + competency_taxonomy = object_tag.taxonomy + bad_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Bad Rule") + bad_group = CompetencyCriteriaGroup.objects.create(tag=bad_tag) + bad_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+bad", taxonomy=competency_taxonomy, tag=bad_tag, + ) + bad_criterion = _make_criterion( + group=bad_group, object_tag=bad_object_tag, + override=(RuleType.GRADE, {"op": "gte", "value": 0.8, "scale": "percent"}), + ) + CompetencyCriterion.objects.filter(pk=bad_criterion.pk).update(**corrupt_kwargs) + + with caplog.at_level(logging.WARNING, logger="openedx_learning.applets.cbe.api"): + changed = record_graded_object_statuses( + user_id=user.id, + scores=[ + GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9")), + GradedObjectScore(object_id=bad_object_tag.object_id, fraction=Decimal("0.9")), + ], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=bad_criterion).exists() + assert any( + record.levelname == "WARNING" + and f"CompetencyCriterion {bad_criterion.pk} for user {user.id} on object {bad_object_tag.object_id}" + in record.getMessage() + for record in caplog.records + ) + + +def test_invalid_profile_payload_is_skipped( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile, + caplog: pytest.LogCaptureFixture, +) -> None: + """ + A criterion resolved through a profile whose stored payload is invalid is skipped and logged, the + same as an invalid per-criterion override. The profile is corrupted with a direct queryset `update()`, + since `save()` itself would reject an invalid payload. + """ + CompetencyRuleProfile.objects.filter(pk=default_rule_profile.pk).update( + rule_payload={"op": "invalid_op", "value": 0.8, "scale": "percent"}, + ) + + with caplog.at_level(logging.WARNING, logger="openedx_learning.applets.cbe.api"): + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + assert any( + record.levelname == "WARNING" and str(criterion.pk) in record.getMessage() + for record in caplog.records + ) + + +def test_criterion_on_non_competency_taxonomy_tag_is_not_evaluated( + user, group: CompetencyCriteriaGroup, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + A criterion on a tag outside a competency taxonomy is not evaluated, even when its parent group + sits on a competency-taxonomy tag. + """ + plain_taxonomy = Taxonomy.objects.create(name="Plain Tags", export_id="plain-v2") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Plain") + plain_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+plain", taxonomy=plain_taxonomy, tag=plain_tag, + ) + _make_criterion(group=group, object_tag=plain_object_tag, rule_profile=default_rule_profile) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=plain_object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 0 + assert not StudentCompetencyCriterionStatus.objects.exists() + + +def test_omitted_zero_point_object_gets_no_status( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, competency_taxonomy: CompetencyTaxonomy, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """A subsection worth zero points is simply omitted from scores, and only the scored object gets a status.""" + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Unscored") + other_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+unscored", taxonomy=competency_taxonomy, tag=other_tag, + ) + other_criterion = _make_criterion( + group=CompetencyCriteriaGroup.objects.create(tag=other_tag), + object_tag=other_object_tag, + rule_profile=default_rule_profile, + ) + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=other_criterion).exists() + + +def test_rolled_back_grade_leaves_no_status( + user, criterion: CompetencyCriterion, object_tag: ObjectTag +) -> None: + """A grade that is not saved leaves no competency status behind, since this function opens no transaction.""" + + class _BoomError(Exception): + """Raised deliberately to force the enclosing transaction to roll back.""" + + with pytest.raises(_BoomError), transaction.atomic(): + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + raise _BoomError() + + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + + +@pytest.mark.django_db(transaction=True) +def test_call_outside_an_atomic_block_raises() -> None: + """Called in autocommit mode it raises, and the same call inside an atomic block goes through.""" + scores = [GradedObjectScore(object_id="block-v1:Org+Course+Run+type@problem+block@p1", fraction=Decimal("0.9"))] + + with pytest.raises(TransactionManagementError): + record_graded_object_statuses(user_id=1, scores=scores) + + with transaction.atomic(): + assert record_graded_object_statuses(user_id=1, scores=scores) == 0 + + +def test_storage_error_propagates( + user, criterion: CompetencyCriterion, object_tag: ObjectTag, monkeypatch: pytest.MonkeyPatch, +) -> None: + """A genuine storage failure propagates out uncaught, rather than being mistaken for an unappliable rule.""" + def _boom(*args, **kwargs): + raise DatabaseError("boom") + + monkeypatch.setattr(StudentCompetencyCriterionStatus.objects, "get_or_create", _boom) + + with pytest.raises(DatabaseError): + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.9"))], + ) + + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=criterion).exists() + + +def test_duplicate_object_id_raises_value_error(user, django_assert_num_queries) -> None: + """A repeated object_id in one call raises ValueError before any query runs.""" + scores = [ + GradedObjectScore(object_id="dup", fraction=Decimal("0.5")), + GradedObjectScore(object_id="dup", fraction=Decimal("0.9")), + ] + + with django_assert_num_queries(0): + with pytest.raises(ValueError): + record_graded_object_statuses(user_id=user.id, scores=scores) + + +def test_graded_object_score_validation() -> None: + """ + Decimal("0") and Decimal("1") are valid; a fraction outside 0-1, NaN, or Infinity raises ValueError; + a plain float raises TypeError. + """ + GradedObjectScore(object_id="x", fraction=Decimal("0")) + GradedObjectScore(object_id="x", fraction=Decimal("1")) + + for bad_fraction in (Decimal("-0.01"), Decimal("1.01"), Decimal("NaN"), Decimal("Infinity")): + with pytest.raises(ValueError): + GradedObjectScore(object_id="x", fraction=bad_fraction) + + with pytest.raises(TypeError): + GradedObjectScore(object_id="x", fraction=0.8) # type: ignore[arg-type] + + +def test_archived_rule_profile_still_evaluates( + user, competency_taxonomy: CompetencyTaxonomy, group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + An archived CompetencyRuleProfile remains queryable and is still evaluated normally, per ADR-0002 + Decision 3. + """ + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, + rule_payload={"op": "gte", "value": 0.75, "scale": "percent"}, + ) + criterion = _make_criterion(group=group, object_tag=object_tag, rule_profile=profile) + profile.archived = True + profile.save() + + changed = record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.8"))], + ) + + assert changed == 1 + assert StudentCompetencyCriterionStatus.objects.get(criterion=criterion).status_id == MasteryStatus.DEMONSTRATED + + +def test_archived_criterion_is_skipped( + user, group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + An archived criterion neither gets a new status row nor has an existing one raised, per ADR-0002 + Decision 4: archiving removes it from the tree. + """ + tag = object_tag.tag + other_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+other", taxonomy=object_tag.taxonomy, tag=tag, + ) + with_status = _make_criterion(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + without_status = _make_criterion(group=group, object_tag=other_object_tag, rule_profile=default_rule_profile) + record_graded_object_statuses( + user_id=user.id, scores=[GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("0.1"))], + ) + for criterion in (with_status, without_status): + criterion.archived = True + criterion.save() + + changed = record_graded_object_statuses( + user_id=user.id, + scores=[ + GradedObjectScore(object_id=object_tag.object_id, fraction=Decimal("1")), + GradedObjectScore(object_id=other_object_tag.object_id, fraction=Decimal("1")), + ], + ) + + assert changed == 0 + assert ( + StudentCompetencyCriterionStatus.objects.get(criterion=with_status).status_id + == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + ) + assert not StudentCompetencyCriterionStatus.objects.filter(criterion=without_status).exists() diff --git a/tests/openedx_learning/applets/cbe/test_rule_payloads.py b/tests/openedx_learning/applets/cbe/test_rule_payloads.py index 47c342793..555726c27 100644 --- a/tests/openedx_learning/applets/cbe/test_rule_payloads.py +++ b/tests/openedx_learning/applets/cbe/test_rule_payloads.py @@ -5,10 +5,19 @@ "Grade", whose payload is {"op": ..., "value": ..., "scale": ...} where op is one of gte, lte or eq, value is a fraction from 0.0 to 1.0 rather than a number out of 100, and scale is "percent". """ +from decimal import Decimal + import pytest from django.core.exceptions import ValidationError -from openedx_learning.applets.cbe.rule_payloads import GradeRulePayload, RuleType, validate_rule_payload +from openedx_learning.applets.cbe.rule_payloads import ( + _GRADE_OPERATOR_FUNCS, + _GRADE_OPERATORS, + GradeRulePayload, + RuleType, + evaluate_rule, + validate_rule_payload, +) _GRADE_PAYLOAD: GradeRulePayload = {"op": "gte", "value": 0.8, "scale": "percent"} @@ -128,3 +137,51 @@ def test_every_rule_type_choice_has_a_validation_branch(rule_type: RuleType) -> validate_rule_payload(rule_type, {}) assert "not supported yet" not in " ".join(exc_info.value.messages) + + +@pytest.mark.parametrize( + "op, fraction, expected", + [ + pytest.param("gte", Decimal("0.81"), True, id="gte_above"), + pytest.param("gte", Decimal("0.8"), True, id="gte_exact_threshold"), + pytest.param("gte", Decimal("0.79"), False, id="gte_below"), + pytest.param("lte", Decimal("0.8"), True, id="lte_exact_threshold"), + pytest.param("lte", Decimal("0.79"), True, id="lte_below"), + pytest.param("lte", Decimal("0.81"), False, id="lte_above"), + pytest.param("eq", Decimal("0.8"), True, id="eq_exact"), + pytest.param("eq", Decimal("0.79"), False, id="eq_not_exact"), + ], +) +def test_evaluate_rule(op: str, fraction: Decimal, expected: bool) -> None: + """ + evaluate_rule() applies each supported operator correctly, including the exact-threshold case + that regresses comparing a Decimal fraction against the float this rule_payload's "value" comes + back as from a JSONField: gte 0.8 against exactly Decimal("0.8") must return True, not False. + """ + payload = {**_GRADE_PAYLOAD, "op": op} + assert evaluate_rule(RuleType.GRADE, payload, fraction) is expected + + +@pytest.mark.parametrize( + "rule_type, payload", + [ + pytest.param("MasteryLevel", {"level": 3}, id="unsupported_rule_type"), + pytest.param(RuleType.GRADE, {"op": "startswith", "value": 0.8, "scale": "percent"}, id="unsupported_operator"), + ], +) +def test_evaluate_rule_raises_for_unappliable_rule(rule_type: str, payload: dict) -> None: + """ + ValidationError propagates out of evaluate_rule() uncaught for an unsupported rule type or + operator. validate_rule_payload() itself already has thorough, separate coverage above; this + only confirms evaluate_rule() surfaces the same exception, not that the validator is correct. + """ + with pytest.raises(ValidationError): + evaluate_rule(rule_type, payload, Decimal("0.8")) + + +def test_grade_operators_frozenset_matches_evaluator_map() -> None: + """ + _GRADE_OPERATORS is derived from _GRADE_OPERATOR_FUNCS's own keys, so the two cannot drift apart: + a future operator added to one without the other would fail this test immediately. + """ + assert _GRADE_OPERATORS == frozenset(_GRADE_OPERATOR_FUNCS)