From 889d8bc6a0f366c180a09e1b757a5520216fa470 Mon Sep 17 00:00:00 2001 From: alezconsultant Date: Tue, 13 Oct 2026 19:39:44 +0400 Subject: [PATCH 1/2] feat: roll up learner competency statuses above a graded criterion Add roll_up_competency_statuses, which recomputes every learner status above the criteria attached to the given object ids: each criteria group up to the root, the competency at the root, and every parent competency up to the top of the taxonomy. It runs in autocommit, takes no lock, and every write only raises a stored status, so concurrent, repeated and out-of-order runs for the same learner converge on the right values. Related to #643 Co-Authored-By: Claude Sonnet 5.5 --- src/openedx_learning/applets/cbe/api.py | 2 + src/openedx_learning/applets/cbe/rollup.py | 254 ++++++++++ src/openedx_learning/applets/cbe/statuses.py | 30 ++ .../applets/cbe/test_rollup_api.py | 445 ++++++++++++++++++ .../applets/cbe/test_rollup_rules.py | 71 +++ .../applets/cbe/test_statuses.py | 60 +++ .../applets/cbe/test_tag_rollup.py | 313 ++++++++++++ tests/openedx_tagging/test_api.py | 2 +- 8 files changed, 1176 insertions(+), 1 deletion(-) create mode 100644 src/openedx_learning/applets/cbe/rollup.py create mode 100644 src/openedx_learning/applets/cbe/statuses.py create mode 100644 tests/openedx_learning/applets/cbe/test_rollup_api.py create mode 100644 tests/openedx_learning/applets/cbe/test_rollup_rules.py create mode 100644 tests/openedx_learning/applets/cbe/test_statuses.py create mode 100644 tests/openedx_learning/applets/cbe/test_tag_rollup.py diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index 4aed9cba8..7b9feef2b 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -9,11 +9,13 @@ from openedx_tagging.models import Taxonomy from .models import CompetencyRuleProfile, CompetencyTaxonomy +from .rollup import roll_up_competency_statuses __all__ = [ "create_competency_taxonomy", "get_competency_rule_profiles", "is_competency_taxonomy", + "roll_up_competency_statuses", "select_competency_taxonomies", ] diff --git a/src/openedx_learning/applets/cbe/rollup.py b/src/openedx_learning/applets/cbe/rollup.py new file mode 100644 index 000000000..5ece0558b --- /dev/null +++ b/src/openedx_learning/applets/cbe/rollup.py @@ -0,0 +1,254 @@ +""" +Roll up a learner's competency statuses from changed criteria to the top of the competency hierarchy. +""" +from __future__ import annotations + +import logging +from collections.abc import Sequence +from datetime import datetime, timezone +from functools import reduce +from operator import or_ + +from django.db.models import Q + +from openedx_tagging.models import Tag +from openedx_tagging.models.base import TAXONOMY_MAX_DEPTH + +from .models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + LogicOperator, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriterionStatus, + StudentCompetencyStatus, +) +from .statuses import raise_status + +log = logging.getLogger(__name__) + +__all__ = ["combine_children", "combine_parent_competency_status", "roll_up_competency_statuses"] + + +def combine_children( + logic_operator: str | None, child_statuses: Sequence[MasteryStatus | None], +) -> MasteryStatus | None: + """ + Return a criteria group's status from its children's, or None when no child has a status. + + ``None`` entries are children with no row. A null ``logic_operator`` is treated as OR. + """ + attempted = [status for status in child_statuses if status is not None] + if not attempted: + return None + all_attempted = len(attempted) == len(child_statuses) + if logic_operator == LogicOperator.AND: + if all_attempted and all(status == MasteryStatus.DEMONSTRATED for status in attempted): + return MasteryStatus.DEMONSTRATED + if MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED in attempted: + return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + return MasteryStatus.PARTIALLY_ATTEMPTED + if MasteryStatus.DEMONSTRATED in attempted: + return MasteryStatus.DEMONSTRATED + if all_attempted and all(status == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED for status in attempted): + return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + return MasteryStatus.PARTIALLY_ATTEMPTED + + +def combine_parent_competency_status( + own: MasteryStatus | None, from_sub_competencies: MasteryStatus | None, +) -> MasteryStatus | None: + """ + Return a parent competency's status: the higher of its own criteria result and its sub-competencies'. + + Either source alone demonstrates the parent, the same either-suffices rule as alternative routes in a group. + Both inputs are only DEMONSTRATED, PARTIALLY_ATTEMPTED or None. + """ + return max((status for status in (own, from_sub_competencies) if status is not None), default=None) + + +def _child_groups(group_ids: set[int]) -> list[tuple[int, int]]: + """Return (child group id, parent group id) for every child group of ``group_ids``.""" + return list(CompetencyCriteriaGroup.objects.filter(parent_id__in=group_ids).values_list("pk", "parent_id")) + + +def _child_criteria(group_ids: set[int]) -> list[tuple[int, int]]: + """Return (criterion id, group id) for every criterion directly under ``group_ids``.""" + return list(CompetencyCriterion.objects.filter(group_id__in=group_ids).values_list("pk", "group_id")) + + +def _to_status(value: int | None) -> MasteryStatus | None: + return None if value is None else MasteryStatus(value) + + +def _children_statuses(user_id: int, group_ids: set[int]) -> dict[int, list[MasteryStatus | None]]: + """Return, per group, one status (or None for no row) for each of its child groups and criteria.""" + child_groups = _child_groups(group_ids) + child_criteria = _child_criteria(group_ids) + group_status = dict( + StudentCompetencyCriteriaGroupStatus.objects.filter( + user_id=user_id, group_id__in=[child_id for child_id, _ in child_groups], + ).values_list("group_id", "status_id") + ) + criterion_status = dict( + StudentCompetencyCriterionStatus.objects.filter( + user_id=user_id, criterion_id__in=[child_id for child_id, _ in child_criteria], + ).values_list("criterion_id", "status_id") + ) + statuses: dict[int, list[MasteryStatus | None]] = {group_id: [] for group_id in group_ids} + for child_id, parent_id in child_groups: + statuses[parent_id].append(_to_status(group_status.get(child_id))) + for child_id, parent_id in child_criteria: + statuses[parent_id].append(_to_status(criterion_status.get(child_id))) + return statuses + + +def _competency_level(status: MasteryStatus) -> MasteryStatus: + """Map a group status to a competency status, which never holds ATTEMPTED_NOT_DEMONSTRATED.""" + if status == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED: + return MasteryStatus.PARTIALLY_ATTEMPTED + return status + + +def _roll_up_groups(user_id: int, group_ids: set[int], now: datetime) -> set[int]: + """ + Recompute each group in ``group_ids`` and then its parent, one level at a time, up to the roots. + + Returns the ids of the competency tags whose root group was reached. + """ + competency_tag_ids: set[int] = set() + frontier = set(group_ids) + while frontier: + statuses = _children_statuses(user_id, frontier) + next_frontier: set[int] = set() + for group in CompetencyCriteriaGroup.objects.filter(pk__in=frontier): + status = combine_children(group.logic_operator, statuses[group.pk]) + if status is not None: + raise_status( + StudentCompetencyCriteriaGroupStatus.objects, + lookup={"user_id": user_id, "group_id": group.pk}, + status=status, + now=now, + ) + if group.parent_id is not None: + next_frontier.add(group.parent_id) + continue + competency_tag_ids.add(group.tag_id) + if status is not None: + raise_status( + StudentCompetencyStatus.objects, + lookup={"user_id": user_id, "tag_id": group.tag_id}, + status=_competency_level(status), + now=now, + ) + frontier = next_frontier + return competency_tag_ids + + +def _own_criteria_statuses(user_id: int, tag_ids: set[int]) -> dict[int, MasteryStatus]: + """Return each tag's own criteria result: its best root-group status, for tags that have an attempted root.""" + best: dict[int, int] = {} + rows = StudentCompetencyCriteriaGroupStatus.objects.filter( + user_id=user_id, group__tag_id__in=tag_ids, group__parent__isnull=True, + ).values_list("group__tag_id", "status_id") + for tag_id, status_id in rows: + best[tag_id] = max(best.get(tag_id, status_id), status_id) + return {tag_id: _competency_level(MasteryStatus(status_id)) for tag_id, status_id in best.items()} + + +def _sub_competency_status(child_ids: list[int], competency_status: dict[int, int]) -> MasteryStatus | None: + """Return DEMONSTRATED if every sub-competency is, PARTIALLY_ATTEMPTED if any has a status, else None.""" + statuses = [competency_status[child_id] for child_id in child_ids if child_id in competency_status] + if not statuses: + return None + if len(statuses) == len(child_ids) and all(status == MasteryStatus.DEMONSTRATED for status in statuses): + return MasteryStatus.DEMONSTRATED + return MasteryStatus.PARTIALLY_ATTEMPTED + + +def _log_unearnable_sub_competencies(parents: list[Tag], children_by_parent: dict[int, list[tuple[int, str]]]) -> None: + """Log each direct sub-competency with no root criteria group anywhere in its subtree.""" + candidates = [ + (parent, child_id, lineage) + for parent in parents + for child_id, lineage in children_by_parent.get(parent.pk, []) + ] + if not candidates: + return + root_lineages: dict[int | None, list[str]] = {} + rows = CompetencyCriteriaGroup.objects.filter(parent__isnull=True).filter( + reduce(or_, ( + Q(tag__taxonomy_id=parent.taxonomy_id, tag__lineage__startswith=lineage) + for parent, _, lineage in candidates + )), + ).values_list("tag__taxonomy_id", "tag__lineage") + for taxonomy_id, lineage in rows: + root_lineages.setdefault(taxonomy_id, []).append(lineage.lower()) + # A lineage ends with a tab, so a prefix match cannot confuse "Math" with "Mathematics". + for parent, child_id, lineage in candidates: + prefix = lineage.lower() + if not any(root_lineage.startswith(prefix) for root_lineage in root_lineages.get(parent.taxonomy_id, [])): + log.warning( + "Competency %s: sub-competency %s has no criteria beneath it and can never be demonstrated", + parent.pk, + child_id, + ) + + +def _roll_up_parent_tags(user_id: int, tag_ids: set[int], now: datetime) -> None: + """Recompute the parent of each tag in ``tag_ids``, then its parent, one level at a time, up to the top.""" + frontier = set(tag_ids) + # The depth cap bounds the walk to a few iterations, and a tag's parents stay in its own taxonomy. + for _ in range(TAXONOMY_MAX_DEPTH): + parent_ids = set( + Tag.objects.filter(pk__in=frontier, parent_id__isnull=False).values_list("parent_id", flat=True) + ) + if not parent_ids: + return + parents = list(Tag.objects.filter(pk__in=parent_ids)) + children_by_parent: dict[int, list[tuple[int, str]]] = {} + for child_id, parent_id, lineage in Tag.objects.filter(parent_id__in=parent_ids).values_list( + "pk", "parent_id", "lineage", + ): + children_by_parent.setdefault(parent_id, []).append((child_id, lineage)) + competency_status = dict( + StudentCompetencyStatus.objects.filter(user_id=user_id, tag__parent_id__in=parent_ids).values_list( + "tag_id", "status_id", + ) + ) + own = _own_criteria_statuses(user_id, parent_ids) + _log_unearnable_sub_competencies(parents, children_by_parent) + for parent in parents: + child_ids = [child_id for child_id, _ in children_by_parent.get(parent.pk, [])] + status = combine_parent_competency_status( + own.get(parent.pk), _sub_competency_status(child_ids, competency_status), + ) + if status is not None: + raise_status( + StudentCompetencyStatus.objects, + lookup={"user_id": user_id, "tag_id": parent.pk}, + status=status, + now=now, + ) + frontier = parent_ids + + +def roll_up_competency_statuses(*, user_id: int, object_ids: Sequence[str]) -> None: + """ + Recompute the learner's statuses above the criteria attached to ``object_ids``, up to the top of the framework. + + The caller must not be inside a transaction, because each level has to commit before the next is read. + Repeated calls with the same arguments are safe, since every write only ever raises a stored status. + """ + if not object_ids: + return + group_ids = set( + CompetencyCriterion.objects.filter( + object_tag__object_id__in=list(set(object_ids)), + object_tag__tag__taxonomy__competencytaxonomy__isnull=False, + ).values_list("group_id", flat=True) + ) + if not group_ids: + return + now = datetime.now(tz=timezone.utc) + _roll_up_parent_tags(user_id, _roll_up_groups(user_id, group_ids, now), now) diff --git a/src/openedx_learning/applets/cbe/statuses.py b/src/openedx_learning/applets/cbe/statuses.py new file mode 100644 index 000000000..3f61d1b25 --- /dev/null +++ b/src/openedx_learning/applets/cbe/statuses.py @@ -0,0 +1,30 @@ +""" +The monotone write the rollup uses for the group and competency status tables. +""" +from __future__ import annotations + +from datetime import datetime +from typing import TypeVar + +from django.db import models + +from .models import MasteryStatus + +__all__ = ["raise_status"] + +_ModelT = TypeVar("_ModelT", bound=models.Model) + + +def raise_status( + manager: models.Manager[_ModelT], *, lookup: dict[str, int], status: MasteryStatus, now: datetime, +) -> int: + """ + Create or raise the learner status row identified by ``lookup``; never lowers a stored status. + + Returns 1 if a row was created or raised, 0 if the stored status was already at least ``status``. + """ + row, created = manager.get_or_create(**lookup, defaults={"status_id": status, "created": now, "modified": now}) + if created: + return 1 + # A single conditional UPDATE, never read-then-write: a stale read can only compute a value that is too low. + return manager.filter(pk=row.pk, status_id__lt=status).update(status_id=status, modified=now) diff --git a/tests/openedx_learning/applets/cbe/test_rollup_api.py b/tests/openedx_learning/applets/cbe/test_rollup_api.py new file mode 100644 index 000000000..089f88822 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rollup_api.py @@ -0,0 +1,445 @@ +""" +Tests for `roll_up_competency_statuses` below the competency level: criterion statuses up through the +criteria groups to the competency a root group belongs to. +""" +from datetime import datetime, timezone +from types import SimpleNamespace +from unittest import mock + +import pytest +from django.db import DatabaseError, connection +from django.test.utils import CaptureQueriesContext +from freezegun import freeze_time + +from openedx_learning.api import roll_up_competency_statuses +from openedx_learning.applets.cbe import rollup +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + LogicOperator, + MasteryStatus, + RuleType, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriterionStatus, + StudentCompetencyStatus, +) +from openedx_tagging.models import ObjectTag, Tag, Taxonomy + +pytestmark = pytest.mark.django_db + +DEMONSTRATED = MasteryStatus.DEMONSTRATED +PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED +FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + +NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) +ASSIGNMENT_1 = "block-v1:Org1+Python100+Fall2026+problem+a1" +ASSIGNMENT_2 = "block-v1:Org1+Python100+Fall2026+problem+a2" + + +def _make_group(tag: Tag, *, parent=None, logic_operator=None) -> CompetencyCriteriaGroup: + """Create a criteria group; a null operator is how real root and course-level groups are stored.""" + return CompetencyCriteriaGroup.objects.create(tag=tag, parent=parent, logic_operator=logic_operator) + + +def _make_criterion( + group: CompetencyCriteriaGroup, object_id: str, profile: CompetencyRuleProfile, +) -> CompetencyCriterion: + """Create a criterion on ``group`` for ``object_id``, tagging the object with the group's competency.""" + object_tag = ObjectTag.objects.create(object_id=object_id, taxonomy=group.tag.taxonomy, tag=group.tag) + return CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=profile) + + +def _record(user, criterion: CompetencyCriterion, status: MasteryStatus) -> None: + """Store a leaf status as the grade transaction would have.""" + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=status, created=NOW, modified=NOW, + ) + + +def _group_status(user, group: CompetencyCriteriaGroup) -> int | None: + row = StudentCompetencyCriteriaGroupStatus.objects.filter(user=user, group=group).first() + return None if row is None else row.status_id + + +def _competency_status(user, tag: Tag) -> int | None: + row = StudentCompetencyStatus.objects.filter(user=user, tag=tag).first() + return None if row is None else row.status_id + + +@pytest.fixture(name="and_tree") +def _and_tree(tag: Tag, default_rule_profile: CompetencyRuleProfile) -> SimpleNamespace: + """A competency whose root has one leaf group requiring two assignments together.""" + root = _make_group(tag) + leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + return SimpleNamespace( + root=root, + leaf=leaf, + first=_make_criterion(leaf, ASSIGNMENT_1, default_rule_profile), + second=_make_criterion(leaf, ASSIGNMENT_2, default_rule_profile), + ) + + +@pytest.fixture(name="or_tree") +def _or_tree(tag: Tag, default_rule_profile: CompetencyRuleProfile) -> SimpleNamespace: + """A competency with two alternative routes, each a leaf group holding one assignment.""" + root = _make_group(tag, logic_operator=LogicOperator.OR) + route_1 = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + route_2 = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + return SimpleNamespace( + root=root, + route_1=route_1, + route_2=route_2, + first=_make_criterion(route_1, ASSIGNMENT_1, default_rule_profile), + second=_make_criterion(route_2, ASSIGNMENT_2, default_rule_profile), + ) + + +def _roll_up(user, *object_ids: str) -> None: + roll_up_competency_statuses(user_id=user.id, object_ids=list(object_ids)) + + +def test_completing_the_last_requirement_demonstrates_the_competency(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _group_status(user, and_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_completing_one_of_several_requirements_shows_progress(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.leaf) == PARTIAL + assert _competency_status(user, tag) == PARTIAL + + +def test_failing_one_of_two_attempted_requirements_shows_the_failure_on_the_group(user, tag, and_tree) -> None: + """Holds when both grades are recorded before the first rollup; see the next test for the other order.""" + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, FAILED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) == FAILED + assert _competency_status(user, tag) == PARTIAL + + +def test_a_group_stays_partially_attempted_once_stored_even_when_the_last_requirement_fails( + user, tag, and_tree, +) -> None: + """Raise-only writes discard the failure computed on the second rollup, because it ranks below what is stored.""" + _record(user, and_tree.first, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1) + _record(user, and_tree.second, FAILED) + + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) == PARTIAL + assert _competency_status(user, tag) == PARTIAL + + +def test_either_alternative_route_demonstrates_the_competency(user, tag, or_tree) -> None: + _record(user, or_tree.first, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, or_tree.route_1) == DEMONSTRATED + assert _group_status(user, or_tree.route_2) is None + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_attempting_some_routes_without_succeeding_shows_progress(user, tag, or_tree) -> None: + _record(user, or_tree.first, FAILED) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, or_tree.root) == PARTIAL + assert _competency_status(user, tag) == PARTIAL + + +def test_failing_every_route_shows_the_failure_on_the_root_but_not_on_the_competency(user, tag, or_tree) -> None: + """Holds when both grades are recorded before the first rollup, as in the AND-group test above.""" + _record(user, or_tree.first, FAILED) + _record(user, or_tree.second, FAILED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, or_tree.root) == FAILED + assert _competency_status(user, tag) == PARTIAL + + +def test_failing_every_route_one_rollup_at_a_time_leaves_the_root_partially_attempted(user, tag, or_tree) -> None: + """Pins the raise-only behavior for the other grading order: the stored PARTIAL is never replaced by FAILED.""" + _record(user, or_tree.first, FAILED) + _roll_up(user, ASSIGNMENT_1) + _record(user, or_tree.second, FAILED) + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, or_tree.route_1) == FAILED + assert _group_status(user, or_tree.route_2) == FAILED + assert _group_status(user, or_tree.root) == PARTIAL + assert _competency_status(user, tag) == PARTIAL + + +def test_a_group_with_nothing_attempted_has_no_status(user, tag, and_tree) -> None: + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) is None + assert _group_status(user, and_tree.root) is None + assert _competency_status(user, tag) is None + + +def test_a_root_group_authored_as_and_still_evaluates(user, tag, default_rule_profile) -> None: + root = _make_group(tag, logic_operator=LogicOperator.AND) + first = _make_criterion(root, ASSIGNMENT_1, default_rule_profile) + second = _make_criterion(root, ASSIGNMENT_2, default_rule_profile) + _record(user, first, DEMONSTRATED) + _record(user, second, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_nested_groups_roll_up_level_by_level(user, tag, default_rule_profile) -> None: + root = _make_group(tag) + course_level = _make_group(tag, parent=root) + leaf = _make_group(tag, parent=course_level, logic_operator=LogicOperator.AND) + first = _make_criterion(leaf, ASSIGNMENT_1, default_rule_profile) + second = _make_criterion(leaf, ASSIGNMENT_2, default_rule_profile) + _record(user, first, DEMONSTRATED) + _record(user, second, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert [_group_status(user, group) for group in (leaf, course_level, root)] == [DEMONSTRATED] * 3 + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_two_grades_recorded_one_after_the_other_both_count(user, tag, and_tree) -> None: + """Each writer sees only its own assignment finished; the later rollup reads both and gets it right.""" + _record(user, and_tree.first, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1) + assert _competency_status(user, tag) == PARTIAL + + _record(user, and_tree.second, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_two_grades_recorded_in_the_opposite_order_both_count(user, tag, and_tree) -> None: + """The result does not depend on which assignment is graded first.""" + _record(user, and_tree.second, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_2) + assert _group_status(user, and_tree.leaf) == PARTIAL + assert _competency_status(user, tag) == PARTIAL + + _record(user, and_tree.first, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _group_status(user, and_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_a_stale_recomputation_never_sets_a_learner_back(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + StudentCompetencyCriterionStatus.objects.filter(criterion=and_tree.second).delete() + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +_StatusRows = list[tuple[int, int, datetime]] + + +def _status_tables() -> tuple[_StatusRows, _StatusRows]: + """Snapshot both rollup tables so a test can prove a rollup wrote nothing.""" + return ( + list(StudentCompetencyCriteriaGroupStatus.objects.order_by("pk").values_list("pk", "status_id", "modified")), + list(StudentCompetencyStatus.objects.order_by("pk").values_list("pk", "status_id", "modified")), + ) + + +def test_repeating_a_recomputation_changes_nothing(user, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + before = _status_tables() + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _status_tables() == before + + +def test_a_recomputation_that_fails_partway_reaches_the_same_result_on_retry(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, DEMONSTRATED) + real_raise_status = rollup.raise_status + calls = [] + + def fail_on_second_write(*args, **kwargs): + calls.append(1) + if len(calls) == 2: + raise DatabaseError("simulated failure between two levels") + return real_raise_status(*args, **kwargs) + + with mock.patch.object(rollup, "raise_status", side_effect=fail_on_second_write): + with pytest.raises(DatabaseError): + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + assert _competency_status(user, tag) is None + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_a_level_whose_value_does_not_change_keeps_its_last_changed_time(user, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + with freeze_time("2026-02-01"): + _roll_up(user, ASSIGNMENT_1) + before = _status_tables() + + with freeze_time("2026-03-01"): + _roll_up(user, ASSIGNMENT_1) + + assert _status_tables() == before + assert StudentCompetencyCriteriaGroupStatus.objects.get(group=and_tree.leaf).modified == datetime( + 2026, 2, 1, tzinfo=timezone.utc, + ) + + +@pytest.mark.parametrize("passing_root_first", [True, False]) +def test_a_competency_with_two_root_groups_ends_at_the_higher_result( + user, tag, default_rule_profile, passing_root_first, +) -> None: + """The database does not forbid a second root; raise-only writes make the higher result win in any order.""" + roots = [_make_group(tag), _make_group(tag)] + passing_root, failing_root = roots if passing_root_first else reversed(roots) + _record(user, _make_criterion(failing_root, ASSIGNMENT_1, default_rule_profile), FAILED) + _record(user, _make_criterion(passing_root, ASSIGNMENT_2, default_rule_profile), DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_the_walk_continues_upward_past_a_group_whose_stored_status_is_already_higher(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, DEMONSTRATED) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=and_tree.leaf, status_id=DEMONSTRATED, created=NOW, modified=NOW, + ) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_one_object_backing_criteria_in_two_groups_computes_each_group_once(user, tag, default_rule_profile) -> None: + root = _make_group(tag) + group_1 = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + group_2 = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + object_tag = ObjectTag.objects.create(object_id=ASSIGNMENT_1, taxonomy=tag.taxonomy, tag=tag) + criteria = [ + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + for group in (group_1, group_2) + ] + for criterion in criteria: + _record(user, criterion, DEMONSTRATED) + + with mock.patch.object(rollup, "combine_children", wraps=rollup.combine_children) as combine: + _roll_up(user, ASSIGNMENT_1) + + assert combine.call_count == 3 # group_1, group_2, then the shared root once + assert _competency_status(user, tag) == DEMONSTRATED + + +@pytest.mark.parametrize("object_ids", [[], ["block-v1:Org1+Nothing+Fall2026+problem+x"], [ASSIGNMENT_1, ASSIGNMENT_1]]) +@pytest.mark.usefixtures("tag") +def test_object_ids_with_no_criteria_do_nothing(user, object_ids) -> None: + roll_up_competency_statuses(user_id=user.id, object_ids=object_ids) + + assert not StudentCompetencyCriteriaGroupStatus.objects.exists() + assert not StudentCompetencyStatus.objects.exists() + + +def test_an_empty_object_list_runs_no_queries(user, django_assert_num_queries) -> None: + with django_assert_num_queries(0): + roll_up_competency_statuses(user_id=user.id, object_ids=[]) + + +def test_a_criterion_tagged_outside_a_competency_taxonomy_is_not_rolled_up(user, tag, default_rule_profile) -> None: + plain_taxonomy = Taxonomy.objects.create(name="Plain") + plain_tag = Tag.objects.create(taxonomy=plain_taxonomy, value="Plain tag") + root = _make_group(tag) + object_tag = ObjectTag.objects.create(object_id=ASSIGNMENT_1, taxonomy=plain_taxonomy, tag=plain_tag) + criterion = CompetencyCriterion.objects.create(group=root, object_tag=object_tag, rule_profile=default_rule_profile) + _record(user, criterion, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert not StudentCompetencyCriteriaGroupStatus.objects.exists() + + +def test_a_storage_error_propagates_so_the_caller_can_retry(user, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + + with mock.patch.object(rollup, "raise_status", side_effect=DatabaseError("down")): + with pytest.raises(DatabaseError): + _roll_up(user, ASSIGNMENT_1) + + +_SEEDED_STATUS_LABELS = { + MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED: "AttemptedNotDemonstrated", + MasteryStatus.PARTIALLY_ATTEMPTED: "PartiallyAttempted", + MasteryStatus.DEMONSTRATED: "Demonstrated", +} + + +@pytest.mark.django_db(transaction=True) +def test_every_write_runs_outside_any_enclosing_transaction(user, competency_taxonomy) -> None: + """Each level must commit before the next is read, so the walk may not wrap itself in a transaction.""" + for status, label in _SEEDED_STATUS_LABELS.items(): + CompetencyMasteryStatus.objects.get_or_create(id=status.value, defaults={"status": label}) + parent = Tag.objects.create(taxonomy=competency_taxonomy, value="Parent") + child = Tag.objects.create(taxonomy=competency_taxonomy, value="Child", parent=parent) + root = CompetencyCriteriaGroup.objects.create(tag=child) + object_tag = ObjectTag.objects.create(object_id=ASSIGNMENT_1, taxonomy=competency_taxonomy, tag=child) + criterion = CompetencyCriterion.objects.create( + group=root, + object_tag=object_tag, + rule_type_override=RuleType.GRADE, + rule_payload_override={"op": "gte", "value": 0.8, "scale": "percent"}, + ) + _record(user, criterion, DEMONSTRATED) + real_raise_status = rollup.raise_status + atomic_at_write = [] + + def observe(*args, **kwargs): + atomic_at_write.append(connection.in_atomic_block) + return real_raise_status(*args, **kwargs) + + with mock.patch.object(rollup, "raise_status", side_effect=observe), CaptureQueriesContext(connection) as queries: + _roll_up(user, ASSIGNMENT_1) + + # Three writes: the root group, the child competency, and the parent competency in the second walk. + assert len(atomic_at_write) == 3 + assert not any(atomic_at_write) + # An enclosing transaction would turn get_or_create's own atomic() into a savepoint. + assert not any("SAVEPOINT" in query["sql"] for query in queries) diff --git a/tests/openedx_learning/applets/cbe/test_rollup_rules.py b/tests/openedx_learning/applets/cbe/test_rollup_rules.py new file mode 100644 index 000000000..108aecf92 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rollup_rules.py @@ -0,0 +1,71 @@ +""" +Tests for the pure combination rules used by the rollup. These need no database. +""" +from itertools import permutations + +import pytest + +from openedx_learning.applets.cbe.rollup import combine_children, combine_parent_competency_status +from openedx_learning.models import LogicOperator, MasteryStatus + +DEMONSTRATED = MasteryStatus.DEMONSTRATED +PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED +FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + +AND = LogicOperator.AND +OR = LogicOperator.OR + + +@pytest.mark.parametrize( + ("operator", "children", "expected"), + [ + (AND, [DEMONSTRATED, DEMONSTRATED], DEMONSTRATED), + (AND, [DEMONSTRATED, None], PARTIAL), + (AND, [None, None], None), + (AND, [DEMONSTRATED, FAILED], FAILED), + (AND, [None, FAILED], FAILED), + (AND, [PARTIAL, DEMONSTRATED], PARTIAL), + (AND, [FAILED, FAILED], FAILED), + (OR, [DEMONSTRATED, None], DEMONSTRATED), + (OR, [FAILED, DEMONSTRATED], DEMONSTRATED), + (OR, [FAILED, FAILED], FAILED), + (OR, [FAILED, None], PARTIAL), + (OR, [PARTIAL, FAILED], PARTIAL), + (OR, [None, None], None), + (None, [FAILED, DEMONSTRATED], DEMONSTRATED), + (None, [FAILED], FAILED), + (AND, [], None), + (OR, [], None), + ], +) +def test_combine_children(operator, children, expected) -> None: + """Each row is one acceptance-criteria shape; a null operator behaves as OR.""" + assert combine_children(operator, children) == expected + + +@pytest.mark.parametrize("operator", [AND, OR, None]) +@pytest.mark.parametrize( + "children", [[DEMONSTRATED, FAILED, None], [None, FAILED, PARTIAL], [DEMONSTRATED, DEMONSTRATED, PARTIAL]], +) +def test_combine_children_ignores_child_order(operator, children) -> None: + """The result depends only on the set of child values, never on the order they are read in.""" + results = {combine_children(operator, list(order)) for order in permutations(children)} + assert len(results) == 1 + + +@pytest.mark.parametrize( + ("own", "from_sub_competencies", "expected"), + [ + (DEMONSTRATED, None, DEMONSTRATED), + (DEMONSTRATED, PARTIAL, DEMONSTRATED), + (None, DEMONSTRATED, DEMONSTRATED), + (PARTIAL, DEMONSTRATED, DEMONSTRATED), + (PARTIAL, None, PARTIAL), + (None, PARTIAL, PARTIAL), + (PARTIAL, PARTIAL, PARTIAL), + (None, None, None), + ], +) +def test_combine_parent_competency_status(own, from_sub_competencies, expected) -> None: + """A parent is demonstrated when either source is, and partially attempted on any progress.""" + assert combine_parent_competency_status(own, from_sub_competencies) == expected diff --git a/tests/openedx_learning/applets/cbe/test_statuses.py b/tests/openedx_learning/applets/cbe/test_statuses.py new file mode 100644 index 000000000..5c7112144 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_statuses.py @@ -0,0 +1,60 @@ +""" +Tests for `raise_status`, the monotone write shared by the learner status tables. +""" +from datetime import datetime, timezone + +import pytest + +from openedx_learning.applets.cbe.statuses import raise_status +from openedx_learning.models import MasteryStatus, StudentCompetencyStatus + +pytestmark = pytest.mark.django_db + +FIRST = datetime(2026, 1, 1, tzinfo=timezone.utc) +SECOND = datetime(2026, 2, 1, tzinfo=timezone.utc) + + +def _write(user, tag, status: MasteryStatus, now: datetime) -> int: + return raise_status( + StudentCompetencyStatus.objects, lookup={"user_id": user.id, "tag_id": tag.id}, status=status, now=now, + ) + + +def test_creates_a_missing_row(user, tag) -> None: + """A first write creates the row with created equal to modified.""" + assert _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, FIRST) == 1 + + row = StudentCompetencyStatus.objects.get(user=user, tag=tag) + assert row.status_id == MasteryStatus.PARTIALLY_ATTEMPTED + assert (row.created, row.modified) == (FIRST, FIRST) + + +def test_raises_a_lower_stored_status(user, tag) -> None: + """A higher status replaces a lower one and moves modified but not created.""" + _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, FIRST) + + assert _write(user, tag, MasteryStatus.DEMONSTRATED, SECOND) == 1 + + row = StudentCompetencyStatus.objects.get(user=user, tag=tag) + assert row.status_id == MasteryStatus.DEMONSTRATED + assert (row.created, row.modified) == (FIRST, SECOND) + + +def test_never_lowers_a_stored_status(user, tag) -> None: + """A lower status is discarded and the row is left exactly as it was.""" + _write(user, tag, MasteryStatus.DEMONSTRATED, FIRST) + + assert _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, SECOND) == 0 + + row = StudentCompetencyStatus.objects.get(user=user, tag=tag) + assert row.status_id == MasteryStatus.DEMONSTRATED + assert row.modified == FIRST + + +def test_equal_status_changes_nothing(user, tag) -> None: + """Writing the stored value again is a no-op, so modified keeps its old value.""" + _write(user, tag, MasteryStatus.DEMONSTRATED, FIRST) + + assert _write(user, tag, MasteryStatus.DEMONSTRATED, SECOND) == 0 + + assert StudentCompetencyStatus.objects.get(user=user, tag=tag).modified == FIRST diff --git a/tests/openedx_learning/applets/cbe/test_tag_rollup.py b/tests/openedx_learning/applets/cbe/test_tag_rollup.py new file mode 100644 index 000000000..046d011c0 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_tag_rollup.py @@ -0,0 +1,313 @@ +""" +Tests for `roll_up_competency_statuses` above the competency level: a graded competency's parents. +""" +from datetime import datetime, timezone +from types import SimpleNamespace +from unittest import mock + +import pytest + +from openedx_learning.api import roll_up_competency_statuses +from openedx_learning.applets.cbe import rollup +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriterionStatus, + StudentCompetencyStatus, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + +DEMONSTRATED = MasteryStatus.DEMONSTRATED +PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED +FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + +NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) + + +def _make_tag(taxonomy: CompetencyTaxonomy, value: str, parent: Tag | None = None) -> Tag: + return Tag.objects.create(taxonomy=taxonomy, value=value, parent=parent) + + +def _object_id(name: str) -> str: + """Return the block id of the problem called ``name``.""" + return f"block-v1:Org1+Python100+Fall2026+problem+{name}" + + +def _earnable(tag: Tag, object_id: str, profile: CompetencyRuleProfile) -> SimpleNamespace: + """Give ``tag`` a root group with one criterion on ``object_id``.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag) + object_tag = ObjectTag.objects.create(object_id=object_id, taxonomy=tag.taxonomy, tag=tag) + criterion = CompetencyCriterion.objects.create(group=root, object_tag=object_tag, rule_profile=profile) + return SimpleNamespace(root=root, criterion=criterion, object_id=object_id) + + +def _record(user, earnable: SimpleNamespace, status: MasteryStatus) -> None: + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=earnable.criterion, status_id=status, created=NOW, modified=NOW, + ) + + +def _set_group_status(user, group: CompetencyCriteriaGroup, status: MasteryStatus) -> None: + """Store a group status as an earlier rollup would have.""" + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=status, created=NOW, modified=NOW, + ) + + +def _competency_status(user, tag: Tag) -> int | None: + row = StudentCompetencyStatus.objects.filter(user=user, tag=tag).first() + return None if row is None else row.status_id + + +def _roll_up(user, *object_ids: str) -> None: + roll_up_competency_statuses(user_id=user.id, object_ids=list(object_ids)) + + +def _warned_about(warning: mock.MagicMock) -> set[tuple[int, int]]: + """Return the (competency id, sub-competency id) pairs a mocked ``log.warning`` was called with.""" + return {(call.args[1], call.args[2]) for call in warning.call_args_list} + + +@pytest.fixture(name="family") +def _family(competency_taxonomy: CompetencyTaxonomy) -> SimpleNamespace: + """A grandparent, a parent, and three sibling sub-competencies under the parent.""" + grandparent = _make_tag(competency_taxonomy, "Grandparent") + parent = _make_tag(competency_taxonomy, "Parent", grandparent) + children = [_make_tag(competency_taxonomy, f"Child {n}", parent) for n in (1, 2, 3)] + return SimpleNamespace(grandparent=grandparent, parent=parent, children=children) + + +def test_demonstrating_the_only_sub_competency_raises_the_parent_and_grandparent( + user, competency_taxonomy, default_rule_profile, +) -> None: + grandparent = _make_tag(competency_taxonomy, "Grandparent") + parent = _make_tag(competency_taxonomy, "Parent", grandparent) + child = _make_tag(competency_taxonomy, "Child", parent) + earnable = _earnable(child, _object_id("c"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert [_competency_status(user, tag) for tag in (child, parent, grandparent)] == [DEMONSTRATED] * 3 + + +def test_a_parent_whose_own_requirements_are_met_is_demonstrated_with_a_sub_competency_outstanding( + user, family, default_rule_profile, +) -> None: + earnable = _earnable(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, earnable, FAILED) + own = _earnable(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, own.root, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert _competency_status(user, family.parent) == DEMONSTRATED + + +def test_a_parent_is_demonstrated_when_every_sub_competency_is(user, family, default_rule_profile) -> None: + """The parent has its own requirements, which this learner has not attempted.""" + _earnable(family.parent, _object_id("p"), default_rule_profile) + earnables = [ + _earnable(child, _object_id(f"c{index}"), default_rule_profile) + for index, child in enumerate(family.children) + ] + for earnable in earnables: + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, *(earnable.object_id for earnable in earnables)) + + assert _competency_status(user, family.parent) == DEMONSTRATED + + +def test_a_parent_with_progress_from_either_source_is_partially_attempted(user, family, default_rule_profile) -> None: + """The parent has its own requirements, which this learner has not attempted.""" + _earnable(family.parent, _object_id("p"), default_rule_profile) + earnable = _earnable(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + _earnable(family.children[1], _object_id("c2"), default_rule_profile) + + _roll_up(user, earnable.object_id) + + assert _competency_status(user, family.parent) == PARTIAL + + +def test_a_failed_own_root_group_shows_progress_on_the_parent(user, family, default_rule_profile) -> None: + earnable = _earnable(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, earnable, FAILED) + own = _earnable(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, own.root, FAILED) + + _roll_up(user, earnable.object_id) + + assert _competency_status(user, family.parent) == PARTIAL + + +def test_an_unearnable_sub_competency_is_logged_and_does_not_block_a_parent_with_its_own_requirements( + user, family, default_rule_profile, +) -> None: + earnable = _earnable(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + own = _earnable(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, own.root, DEMONSTRATED) + + with mock.patch.object(rollup.log, "warning") as warning: + _roll_up(user, earnable.object_id) + + assert _competency_status(user, family.parent) == DEMONSTRATED + assert _warned_about(warning) == { + (family.parent.pk, family.children[1].pk), + (family.parent.pk, family.children[2].pk), + } + + +def test_an_unearnable_sub_competency_caps_a_parent_that_has_no_requirements_of_its_own( + user, competency_taxonomy, default_rule_profile, +) -> None: + parent = _make_tag(competency_taxonomy, "Parent") + earnable_child = _make_tag(competency_taxonomy, "Earnable", parent) + unearnable_child = _make_tag(competency_taxonomy, "Unearnable", parent) + earnable = _earnable(earnable_child, _object_id("c"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + with mock.patch.object(rollup.log, "warning") as warning: + _roll_up(user, earnable.object_id) + + assert _competency_status(user, parent) == PARTIAL + assert _warned_about(warning) == {(parent.pk, unearnable_child.pk)} + + +def test_a_sub_competency_with_criteria_only_further_down_still_participates( + user, competency_taxonomy, default_rule_profile, +) -> None: + parent = _make_tag(competency_taxonomy, "Parent") + middle = _make_tag(competency_taxonomy, "Middle", parent) + leaf = _make_tag(competency_taxonomy, "Leaf", middle) + earnable = _earnable(leaf, _object_id("c"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert [_competency_status(user, tag) for tag in (leaf, middle, parent)] == [DEMONSTRATED] * 3 + + +def test_a_stored_demonstrated_parent_is_not_lowered_when_only_some_sub_competencies_are_attempted( + user, family, default_rule_profile, +) -> None: + StudentCompetencyStatus.objects.create( + user=user, tag=family.parent, status_id=DEMONSTRATED, created=NOW, modified=NOW, + ) + earnable = _earnable(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert _competency_status(user, family.parent) == DEMONSTRATED + + +def test_a_tag_at_the_taxonomy_root_ends_the_walk(user, competency_taxonomy, default_rule_profile) -> None: + root_tag = _make_tag(competency_taxonomy, "Root") + earnable = _earnable(root_tag, _object_id("r"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert list(StudentCompetencyStatus.objects.values_list("tag_id", flat=True)) == [root_tag.pk] + + +def test_the_deepest_allowed_taxonomy_is_walked_to_the_top(user, competency_taxonomy, default_rule_profile) -> None: + tags = [_make_tag(competency_taxonomy, "Level 0")] + for depth in range(1, 6): + tags.append(_make_tag(competency_taxonomy, f"Level {depth}", tags[-1])) + earnable = _earnable(tags[-1], _object_id("deep"), default_rule_profile) + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, earnable.object_id) + + assert [_competency_status(user, tag) for tag in tags] == [DEMONSTRATED] * 6 + + +def test_two_graded_competencies_sharing_a_parent_recompute_that_parent_once( + user, family, default_rule_profile, +) -> None: + earnables = [ + _earnable(child, _object_id(f"c{index}"), default_rule_profile) + for index, child in enumerate(family.children[:2]) + ] + for earnable in earnables: + _record(user, earnable, DEMONSTRATED) + + with mock.patch.object( + rollup, "combine_parent_competency_status", wraps=rollup.combine_parent_competency_status, + ) as combine: + _roll_up(user, *(earnable.object_id for earnable in earnables)) + + assert combine.call_count == 2 # the parent once, then the grandparent once + + +def test_parents_at_the_same_level_are_rolled_up_independently( + user, competency_taxonomy, default_rule_profile, +) -> None: + parent_one = _make_tag(competency_taxonomy, "Parent 1") + only_child = _make_tag(competency_taxonomy, "Only child", parent_one) + parent_two = _make_tag(competency_taxonomy, "Parent 2") + first = _make_tag(competency_taxonomy, "First", parent_two) + second = _make_tag(competency_taxonomy, "Second", parent_two) + graded = [ + _earnable(only_child, _object_id("only"), default_rule_profile), + _earnable(first, _object_id("first"), default_rule_profile), + ] + _earnable(second, _object_id("second"), default_rule_profile) + for earnable in graded: + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, *(earnable.object_id for earnable in graded)) + + assert _competency_status(user, parent_one) == DEMONSTRATED + assert _competency_status(user, parent_two) == PARTIAL + + +def test_unearnable_detection_does_not_mix_taxonomies_that_share_tag_names( + user, competency_taxonomy, default_rule_profile, +) -> None: + other_taxonomy = CompetencyTaxonomy.objects.create(name="Other", export_id="other-v1") + parent_a = _make_tag(competency_taxonomy, "Parent") + shared_a = _make_tag(competency_taxonomy, "Shared", parent_a) + orphan_a = _make_tag(competency_taxonomy, "Orphan", parent_a) + parent_b = _make_tag(other_taxonomy, "Parent") + orphan_b = _make_tag(other_taxonomy, "Orphan", parent_b) + graded = [ + _earnable(shared_a, _object_id("shared"), default_rule_profile), + _earnable(orphan_b, _object_id("orphan-b"), default_rule_profile), + ] + for earnable in graded: + _record(user, earnable, DEMONSTRATED) + + with mock.patch.object(rollup.log, "warning") as warning: + _roll_up(user, *(earnable.object_id for earnable in graded)) + + assert _warned_about(warning) == {(parent_a.pk, orphan_a.pk)} + + +def test_competencies_graded_at_different_depths_converge_on_the_shared_ancestor( + user, competency_taxonomy, default_rule_profile, +) -> None: + top = _make_tag(competency_taxonomy, "Top") + middle = _make_tag(competency_taxonomy, "Middle", top) + deep = _make_tag(competency_taxonomy, "Deep", middle) + shallow = _make_tag(competency_taxonomy, "Shallow", top) + graded = [ + _earnable(deep, _object_id("deep"), default_rule_profile), + _earnable(shallow, _object_id("shallow"), default_rule_profile), + ] + for earnable in graded: + _record(user, earnable, DEMONSTRATED) + + _roll_up(user, *(earnable.object_id for earnable in graded)) + + assert [_competency_status(user, tag) for tag in (deep, middle, shallow, top)] == [DEMONSTRATED] * 4 diff --git a/tests/openedx_tagging/test_api.py b/tests/openedx_tagging/test_api.py index a2a8a8e2a..8b17e62bc 100644 --- a/tests/openedx_tagging/test_api.py +++ b/tests/openedx_tagging/test_api.py @@ -67,7 +67,7 @@ def test_create_taxonomy_without_export_id(self) -> None: def test_get_taxonomy(self) -> None: tax1 = tagging_api.get_taxonomy(1) assert tax1 == self.taxonomy - no_tax = tagging_api.get_taxonomy(200) + no_tax = tagging_api.get_taxonomy(2**31 - 1) assert no_tax is None def test_get_taxonomy_by_export_id(self) -> None: From 6752464498a44cc0ebbae596503e9eea6d66bae2 Mon Sep 17 00:00:00 2001 From: alezconsultant Date: Fri, 16 Oct 2026 05:24:34 +0400 Subject: [PATCH 2/2] refactor: clarify rollup group logic and address review feedback --- src/openedx_learning/applets/cbe/rollup.py | 44 ++++++++++--------- .../applets/cbe/test_tag_rollup.py | 16 +++++-- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/openedx_learning/applets/cbe/rollup.py b/src/openedx_learning/applets/cbe/rollup.py index 5ece0558b..73048756b 100644 --- a/src/openedx_learning/applets/cbe/rollup.py +++ b/src/openedx_learning/applets/cbe/rollup.py @@ -1,5 +1,5 @@ """ -Roll up a learner's competency statuses from changed criteria to the top of the competency hierarchy. +Roll up a learner's competency statuses from the criteria of newly graded objects to the top of the hierarchy. """ from __future__ import annotations @@ -9,7 +9,7 @@ from functools import reduce from operator import or_ -from django.db.models import Q +from django.db.models import Max, Q from openedx_tagging.models import Tag from openedx_tagging.models.base import TAXONOMY_MAX_DEPTH @@ -38,20 +38,21 @@ def combine_children( ``None`` entries are children with no row. A null ``logic_operator`` is treated as OR. """ - attempted = [status for status in child_statuses if status is not None] + attempted = {status for status in child_statuses if status is not None} if not attempted: return None - all_attempted = len(attempted) == len(child_statuses) + every_child_attempted = None not in child_statuses if logic_operator == LogicOperator.AND: - if all_attempted and all(status == MasteryStatus.DEMONSTRATED for status in attempted): + if every_child_attempted and attempted == {MasteryStatus.DEMONSTRATED}: return MasteryStatus.DEMONSTRATED if MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED in attempted: return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED - return MasteryStatus.PARTIALLY_ATTEMPTED - if MasteryStatus.DEMONSTRATED in attempted: - return MasteryStatus.DEMONSTRATED - if all_attempted and all(status == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED for status in attempted): - return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + else: # OR, or a null operator, which behaves as OR. + if MasteryStatus.DEMONSTRATED in attempted: + return MasteryStatus.DEMONSTRATED + if every_child_attempted and attempted == {MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED}: + return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + # Some child has a status, but neither rule above is settled: the learner is in progress. return MasteryStatus.PARTIALLY_ATTEMPTED @@ -59,10 +60,10 @@ def combine_parent_competency_status( own: MasteryStatus | None, from_sub_competencies: MasteryStatus | None, ) -> MasteryStatus | None: """ - Return a parent competency's status: the higher of its own criteria result and its sub-competencies'. + Return a parent competency's status: the better of its own criteria result and its sub-competencies' result. - Either source alone demonstrates the parent, the same either-suffices rule as alternative routes in a group. - Both inputs are only DEMONSTRATED, PARTIALLY_ATTEMPTED or None. + A parent is demonstrated when either its own criteria are demonstrated or every direct sub-competency is. + Both inputs are only DEMONSTRATED, PARTIALLY_ATTEMPTED or None, so the better one is the higher value. """ return max((status for status in (own, from_sub_competencies) if status is not None), default=None) @@ -147,13 +148,15 @@ def _roll_up_groups(user_id: int, group_ids: set[int], now: datetime) -> set[int def _own_criteria_statuses(user_id: int, tag_ids: set[int]) -> dict[int, MasteryStatus]: """Return each tag's own criteria result: its best root-group status, for tags that have an attempted root.""" - best: dict[int, int] = {} - rows = StudentCompetencyCriteriaGroupStatus.objects.filter( - user_id=user_id, group__tag_id__in=tag_ids, group__parent__isnull=True, - ).values_list("group__tag_id", "status_id") - for tag_id, status_id in rows: - best[tag_id] = max(best.get(tag_id, status_id), status_id) - return {tag_id: _competency_level(MasteryStatus(status_id)) for tag_id, status_id in best.items()} + rows = ( + StudentCompetencyCriteriaGroupStatus.objects.filter( + user_id=user_id, group__tag_id__in=tag_ids, group__parent__isnull=True, + ) + .values("group__tag_id") + .annotate(best=Max("status_id")) + .values_list("group__tag_id", "best") + ) + return {tag_id: _competency_level(MasteryStatus(best)) for tag_id, best in rows} def _sub_competency_status(child_ids: list[int], competency_status: dict[int, int]) -> MasteryStatus | None: @@ -184,7 +187,6 @@ def _log_unearnable_sub_competencies(parents: list[Tag], children_by_parent: dic ).values_list("tag__taxonomy_id", "tag__lineage") for taxonomy_id, lineage in rows: root_lineages.setdefault(taxonomy_id, []).append(lineage.lower()) - # A lineage ends with a tab, so a prefix match cannot confuse "Math" with "Mathematics". for parent, child_id, lineage in candidates: prefix = lineage.lower() if not any(root_lineage.startswith(prefix) for root_lineage in root_lineages.get(parent.taxonomy_id, [])): diff --git a/tests/openedx_learning/applets/cbe/test_tag_rollup.py b/tests/openedx_learning/applets/cbe/test_tag_rollup.py index 046d011c0..b9e92da79 100644 --- a/tests/openedx_learning/applets/cbe/test_tag_rollup.py +++ b/tests/openedx_learning/applets/cbe/test_tag_rollup.py @@ -193,7 +193,9 @@ def test_a_sub_competency_with_criteria_only_further_down_still_participates( _roll_up(user, earnable.object_id) - assert [_competency_status(user, tag) for tag in (leaf, middle, parent)] == [DEMONSTRATED] * 3 + assert _competency_status(user, leaf) == DEMONSTRATED + assert _competency_status(user, middle) == DEMONSTRATED + assert _competency_status(user, parent) == DEMONSTRATED def test_a_stored_demonstrated_parent_is_not_lowered_when_only_some_sub_competencies_are_attempted( @@ -229,7 +231,12 @@ def test_the_deepest_allowed_taxonomy_is_walked_to_the_top(user, competency_taxo _roll_up(user, earnable.object_id) - assert [_competency_status(user, tag) for tag in tags] == [DEMONSTRATED] * 6 + assert _competency_status(user, tags[0]) == DEMONSTRATED + assert _competency_status(user, tags[1]) == DEMONSTRATED + assert _competency_status(user, tags[2]) == DEMONSTRATED + assert _competency_status(user, tags[3]) == DEMONSTRATED + assert _competency_status(user, tags[4]) == DEMONSTRATED + assert _competency_status(user, tags[5]) == DEMONSTRATED def test_two_graded_competencies_sharing_a_parent_recompute_that_parent_once( @@ -310,4 +317,7 @@ def test_competencies_graded_at_different_depths_converge_on_the_shared_ancestor _roll_up(user, *(earnable.object_id for earnable in graded)) - assert [_competency_status(user, tag) for tag in (deep, middle, shallow, top)] == [DEMONSTRATED] * 4 + assert _competency_status(user, deep) == DEMONSTRATED + assert _competency_status(user, middle) == DEMONSTRATED + assert _competency_status(user, shallow) == DEMONSTRATED + assert _competency_status(user, top) == DEMONSTRATED