diff --git a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst index 5869b2885..a3acf3dd1 100644 --- a/docs/openedx_learning/decisions/0002-competency-criteria-model.rst +++ b/docs/openedx_learning/decisions/0002-competency-criteria-model.rst @@ -87,7 +87,7 @@ Decision 5. ``name``: string 6. ``ordering``: A sibling sort key. The read API returns a group's children ordered by ``(ordering, id)`` so a tree comes back in a stable order. Evaluation ignores it, because the result of ``AND`` or ``OR`` over a group's children cannot depend on the order they are read in. Whether authors will ever need to control that order is an open question; if the authoring UI turns out not to need it, the field should be removed in a follow-up. 7. ``logic_operator``: Either “AND” or “OR” or null. This determines how children are combined at a group node ("AND" or "OR"). - 8. ``archived``: Boolean, defaults to false. Set instead of deleting a group once a student status exists for it. Archived groups are hidden from authoring and new associations but remain queryable, so existing ``CompetencyCriteria`` rows and learner status rows stay resolvable. For MVP, archiving a group or a criterion is permanent, with the root as the one reversible exception: a new criterion for an archived root's competency reactivates that root instead of creating a second one, because archiving it for good would remove the ability to ever associate criteria with that competency again. Archiving a course-level group blocks that course for that competency; archiving a leaf closes only that leaf to new criteria. More unarchival capabilities may be added after MVP. + 8. ``archived``: Boolean, defaults to false. Set instead of deleting a group once a student status exists for it. Archived groups are hidden from authoring and new associations but remain queryable, so existing ``CompetencyCriteria`` rows and learner status rows stay resolvable. For MVP, archiving a group or a criterion is permanent, with the root as the one reversible exception: a new criterion for an archived root's competency reactivates that root instead of creating a second one, because archiving it for good would remove the ability to ever associate criteria with that competency again. Archiving a course-level group blocks that course for that competency; archiving a leaf closes only that leaf to new criteria. More unarchival capabilities may be added after MVP. Archived groups do not count when a group's status is combined from its children, and an archived root's stored status still raises its competency's row and counts toward its parent competency (Decision 6). 9. ``scope_code``: Derived from ``oel_tagging_tag_id``, set on the root group and null on every other group, whether or not the root is archived. It exists so the database can hold at most one root per competency, the same way Decision 3's derived ``scope_code`` holds at most one live profile per scope. Example: A root group uses "OR" with two child groups. @@ -166,7 +166,7 @@ Decision 1. ``Grade``: ``{"op": "gte", "value": 0.75, "scale": "percent"}``. Allowed ``op`` values: ``gte``, ``lte``, ``eq``. ``value`` must be a fraction between 0.0 and 1.0 inclusive, matching the platform's existing fractional grade representation, not a 0-100 scale. - 7. ``archived``: Boolean, defaults to false. Set instead of deleting a criterion once a student status exists for it. Archived criteria are hidden from authoring and new associations but remain queryable, so existing learner status rows stay resolvable. + 7. ``archived``: Boolean, defaults to false. Set instead of deleting a criterion once a student status exists for it. Archived criteria are hidden from authoring and new associations but remain queryable, so existing learner status rows stay resolvable. Archived criteria do not count toward learner statuses (Decision 6). Exactly one of the following holds for a given criterion, never both, never neither: @@ -270,6 +270,7 @@ Decision - When a root group is recomputed, write the competency's ``StudentCompetencyStatus`` from it, mapping ``AttemptedNotDemonstrated`` to ``PartiallyAttempted`` because that table holds only ``Demonstrated`` and ``PartiallyAttempted``. - Then recompute each parent competency up the taxonomy, one level at a time. A parent's status is the better of two inputs: its own criteria result, which is its root group's status mapped the same way, and its sub-competency result, which is ``Demonstrated`` when every direct sub-competency is ``Demonstrated`` and ``PartiallyAttempted`` when at least one has a status. A competency with no criteria tree of its own gets a status row only through its sub-competencies; a competency with neither a root group nor a sub-competency with a status gets no row. - Every write only ever raises a stored status, and each level commits before the next is read. :ref:`openedx-learning-adr-0004` records why. + - An archived criterion is not evaluated when a grade is recorded, and a group is recomputed from its non-archived children only. Neither archiving nor the rollup lowers a stored status, including a status that a non-archived group or a competency earned through a child that is archived. Rows of archived criteria and groups are left unchanged. A competency's row is raised from the status stored for its root group, whether or not the root is archived, and that stored status counts toward its parent competency. Archiving recomputes nothing itself: a learner's statuses reflect it the next time a grade raises a non-archived criterion's status in that group's subtree, or when an operator recomputes them. 1. Add new database table for ``CompetencyMasteryStatuses`` with these columns: @@ -490,3 +491,10 @@ Changelog supersedes the ordered, short-circuited evaluation the earlier text described. Decision 6's update flow now continues past the root group to the competency's own status row and up through parent competencies, and states what a competency with no criteria tree of its own receives. + +2026-11-06: + +* Amended Decision 6 for issue #815: a group is recomputed from its non-archived children only, + so an archived requirement or branch does not have to be met. A status recorded against an + archived criterion or group is kept. A root's stored status, archived or not, raises its + competency's row and counts toward its parent competency. Archiving by itself recomputes nothing. diff --git a/src/openedx_learning/applets/cbe/rollup.py b/src/openedx_learning/applets/cbe/rollup.py index 0ef6c2678..ab299dfde 100644 --- a/src/openedx_learning/applets/cbe/rollup.py +++ b/src/openedx_learning/applets/cbe/rollup.py @@ -30,6 +30,24 @@ __all__ = ["combine_children", "combine_parent_competency_status", "roll_up_competency_statuses"] +def _combine_and(child_statuses: Sequence[MasteryStatus | None]) -> MasteryStatus: + """Return an AND group's status: demonstrated when every child is, not demonstrated when any child is.""" + if all(status == MasteryStatus.DEMONSTRATED for status in child_statuses): + return MasteryStatus.DEMONSTRATED + if MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED in child_statuses: + return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + return MasteryStatus.PARTIALLY_ATTEMPTED + + +def _combine_or(child_statuses: Sequence[MasteryStatus | None]) -> MasteryStatus: + """Return an OR group's status: demonstrated when any child is, not demonstrated when every child is.""" + if MasteryStatus.DEMONSTRATED in child_statuses: + return MasteryStatus.DEMONSTRATED + if all(status == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED for status in child_statuses): + return MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + return MasteryStatus.PARTIALLY_ATTEMPTED + + def combine_children( logic_operator: str | None, child_statuses: Sequence[MasteryStatus | None], ) -> MasteryStatus | None: @@ -38,22 +56,11 @@ 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} - if not attempted: + if all(status is None for status in child_statuses): return None - every_child_attempted = None not in child_statuses if logic_operator == LogicOperator.AND: - if every_child_attempted and attempted == {MasteryStatus.DEMONSTRATED}: - return MasteryStatus.DEMONSTRATED - if MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED 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 + return _combine_and(child_statuses) + return _combine_or(child_statuses) def combine_parent_competency_status( @@ -69,21 +76,34 @@ def combine_parent_competency_status( 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")) + """Return (child group id, parent group id) for every non-archived child group of ``group_ids``.""" + return list( + CompetencyCriteriaGroup.objects.filter(parent_id__in=group_ids, archived=False).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")) + """Return (criterion id, group id) for every non-archived criterion directly under ``group_ids``.""" + return list( + CompetencyCriterion.objects.filter(group_id__in=group_ids, archived=False).values_list("pk", "group_id") + ) def _to_status(value: int | None) -> MasteryStatus | None: return None if value is None else MasteryStatus(value) +def _stored_group_status(user_id: int, group_id: int) -> MasteryStatus | None: + """Return the status stored for ``group_id``, or None when the learner has no row for it.""" + return _to_status( + StudentCompetencyCriteriaGroupStatus.objects.filter(user_id=user_id, group_id=group_id) + .values_list("status_id", flat=True) + .first() + ) + + 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.""" + """Return, per group, one status (or None for no row) for each of its non-archived child groups and criteria.""" child_groups = _child_groups(group_ids) child_criteria = _child_criteria(group_ids) group_status = dict( @@ -111,6 +131,71 @@ def _competency_level(status: MasteryStatus) -> MasteryStatus: return status +def _raise_group_status(user_id: int, group_id: int, status: MasteryStatus | None, now: datetime) -> None: + """Raise the learner's stored status for the group to ``status``; a None status writes nothing.""" + if status is None: + return + raise_status( + StudentCompetencyCriteriaGroupStatus.objects, + lookup={"user_id": user_id, "group_id": group_id}, + status=status, + now=now, + ) + + +def _raise_competency_status(user_id: int, tag_id: int, status: MasteryStatus | None, now: datetime) -> None: + """Raise the learner's stored status for the competency to ``status``; a None status writes nothing.""" + if status is None: + return + raise_status( + StudentCompetencyStatus.objects, + lookup={"user_id": user_id, "tag_id": tag_id}, + status=status, + now=now, + ) + + +def _raise_competency_from_root(user_id: int, root: CompetencyCriteriaGroup, now: datetime) -> None: + """Raise the competency's status from the status stored for its root group; no stored status writes nothing.""" + status = _stored_group_status(user_id, root.pk) + if status is not None: + _raise_competency_status(user_id, root.tag_id, _competency_level(status), now) + + +def _roll_up_group( + user_id: int, group: CompetencyCriteriaGroup, child_statuses: list[MasteryStatus | None], now: datetime, +) -> None: + """ + Recompute one group, and for a root group raise its competency's status from the root's stored status. + + An archived group is not recomputed. + """ + if not group.archived: + _raise_group_status(user_id, group.pk, combine_children(group.logic_operator, child_statuses), now) + # The stored status can be higher than this recomputation, and the competency has to match it. + if group.parent_id is None: + _raise_competency_from_root(user_id, group, now) + + +def _roll_up_group_level(user_id: int, group_ids: set[int], now: datetime) -> list[CompetencyCriteriaGroup]: + """Roll up each group in ``group_ids`` from its children's statuses, and return the groups.""" + child_statuses = _children_statuses(user_id, group_ids) + groups = list(CompetencyCriteriaGroup.objects.filter(pk__in=group_ids)) + for group in groups: + _roll_up_group(user_id, group, child_statuses[group.pk], now) + return groups + + +def _competency_tag_ids(groups: list[CompetencyCriteriaGroup]) -> set[int]: + """Return the ids of the competency tags that the root groups in ``groups`` belong to.""" + return {group.tag_id for group in groups if group.parent_id is None} + + +def _parent_group_ids(groups: list[CompetencyCriteriaGroup]) -> set[int]: + """Return the ids of the parent groups of the non-root groups in ``groups``.""" + return {group.parent_id for group in groups if group.parent_id is not None} + + 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. @@ -118,36 +203,17 @@ def _roll_up_groups(user_id: int, group_ids: set[int], now: datetime) -> set[int 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 + level = set(group_ids) + while level: + groups = _roll_up_group_level(user_id, level, now) + competency_tag_ids |= _competency_tag_ids(groups) + level = _parent_group_ids(groups) 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.""" + # Archived roots are counted because the walk raises a competency's row from an archived root's stored status. rows = ( StudentCompetencyCriteriaGroupStatus.objects.filter( user_id=user_id, group__tag_id__in=tag_ids, group__parent__isnull=True, @@ -174,39 +240,46 @@ def _parent_ids(tag_ids: set[int]) -> set[int]: return set(Tag.objects.filter(pk__in=tag_ids, parent_id__isnull=False).values_list("parent_id", flat=True)) +def _child_ids_by_parent(parent_ids: set[int]) -> dict[int, list[int]]: + """Return, per parent tag, the ids of its direct child tags.""" + children: dict[int, list[int]] = {} + for child_id, parent_id in Tag.objects.filter(parent_id__in=parent_ids).values_list("pk", "parent_id"): + children.setdefault(parent_id, []).append(child_id) + return children + + +def _child_competency_statuses(user_id: int, parent_ids: set[int]) -> dict[int, int]: + """Return the learner's stored competency status for each child tag of ``parent_ids`` that has one.""" + return dict( + StudentCompetencyStatus.objects.filter(user_id=user_id, tag__parent_id__in=parent_ids).values_list( + "tag_id", "status_id", + ) + ) + + +def _roll_up_parent_level(user_id: int, parent_ids: set[int], now: datetime) -> None: + """Recompute each competency in ``parent_ids`` from its own criteria and its direct sub-competencies.""" + child_ids = _child_ids_by_parent(parent_ids) + child_statuses = _child_competency_statuses(user_id, parent_ids) + own_statuses = _own_criteria_statuses(user_id, parent_ids) + for parent_id in sorted(parent_ids): + status = combine_parent_competency_status( + own_statuses.get(parent_id), _sub_competency_status(child_ids.get(parent_id, []), child_statuses), + ) + _raise_competency_status(user_id, parent_id, status, now) + + 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) + level = 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 = _parent_ids(frontier) - if not parent_ids: + level = _parent_ids(level) + if not level: return - parents = list(Tag.objects.filter(pk__in=parent_ids)) - children_by_parent: dict[int, list[int]] = {} - for child_id, parent_id in Tag.objects.filter(parent_id__in=parent_ids).values_list("pk", "parent_id"): - children_by_parent.setdefault(parent_id, []).append(child_id) - 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) - for parent in parents: - child_ids = 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 + _roll_up_parent_level(user_id, level, now) # Tag.save() enforces the cap, so only a tree built around it can still have parents here. - pending = _parent_ids(frontier) + pending = _parent_ids(level) if pending: log.error( "Competency rollup for user %s stopped at the taxonomy depth cap; parent tags %s were not recomputed", @@ -221,6 +294,7 @@ def roll_up_competency_statuses(*, user_id: int, object_ids: Sequence[str]) -> N 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. + Archived groups and criteria are left out of the combination, and a status already stored against them is kept. :raises TransactionManagementError: if called inside an atomic block. """ @@ -231,6 +305,7 @@ def roll_up_competency_statuses(*, user_id: int, object_ids: Sequence[str]) -> N ) if not object_ids: return + # Not filtered on archived: naming an archived criterion's object recomputes its group from non-archived children. group_ids = set( CompetencyCriterion.objects.filter( object_tag__object_id__in=list(set(object_ids)), diff --git a/tests/openedx_learning/applets/cbe/test_rollup_api.py b/tests/openedx_learning/applets/cbe/test_rollup_api.py index 1a913876c..9375a1feb 100644 --- a/tests/openedx_learning/applets/cbe/test_rollup_api.py +++ b/tests/openedx_learning/applets/cbe/test_rollup_api.py @@ -32,24 +32,33 @@ DEMONSTRATED = MasteryStatus.DEMONSTRATED PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED -FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED +NOT_DEMONSTRATED = 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: +def _make_group(tag: Tag, *, parent=None, logic_operator=None, archived: bool = False) -> 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) + return CompetencyCriteriaGroup.objects.create( + tag=tag, parent=parent, logic_operator=logic_operator, archived=archived, + ) def _make_criterion( - group: CompetencyCriteriaGroup, object_id: str, profile: CompetencyRuleProfile, + group: CompetencyCriteriaGroup, object_id: str, profile: CompetencyRuleProfile, *, archived: bool = False, ) -> 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) + return CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=profile, archived=archived, + ) + + +def _archive(row: CompetencyCriteriaGroup | CompetencyCriterion) -> None: + row.archived = True + row.save(update_fields=["archived"]) def _record(user, criterion: CompetencyCriterion, status: MasteryStatus) -> None: @@ -124,11 +133,11 @@ def test_completing_one_of_several_requirements_shows_progress(user, tag, and_tr 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) + _record(user, and_tree.second, NOT_DEMONSTRATED) _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) - assert _group_status(user, and_tree.leaf) == FAILED + assert _group_status(user, and_tree.leaf) == NOT_DEMONSTRATED assert _competency_status(user, tag) == PARTIAL @@ -138,7 +147,7 @@ def test_a_group_stays_partially_attempted_once_stored_even_when_the_last_requir """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) + _record(user, and_tree.second, NOT_DEMONSTRATED) _roll_up(user, ASSIGNMENT_2) @@ -157,7 +166,7 @@ def test_either_alternative_route_demonstrates_the_competency(user, tag, or_tree def test_attempting_some_routes_without_succeeding_shows_progress(user, tag, or_tree) -> None: - _record(user, or_tree.first, FAILED) + _record(user, or_tree.first, NOT_DEMONSTRATED) _roll_up(user, ASSIGNMENT_1) @@ -167,24 +176,24 @@ def test_attempting_some_routes_without_succeeding_shows_progress(user, tag, or_ 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) + _record(user, or_tree.first, NOT_DEMONSTRATED) + _record(user, or_tree.second, NOT_DEMONSTRATED) _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) - assert _group_status(user, or_tree.root) == FAILED + assert _group_status(user, or_tree.root) == NOT_DEMONSTRATED 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) + """Pins raise-only writes for the other grading order: a stored PARTIAL is never replaced by NOT_DEMONSTRATED.""" + _record(user, or_tree.first, NOT_DEMONSTRATED) _roll_up(user, ASSIGNMENT_1) - _record(user, or_tree.second, FAILED) + _record(user, or_tree.second, NOT_DEMONSTRATED) _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.route_1) == NOT_DEMONSTRATED + assert _group_status(user, or_tree.route_2) == NOT_DEMONSTRATED assert _group_status(user, or_tree.root) == PARTIAL assert _competency_status(user, tag) == PARTIAL @@ -425,3 +434,195 @@ def test_a_call_inside_a_transaction_raises(user) -> None: """Each level must commit before the next is read, so the walk refuses to run inside a transaction.""" with pytest.raises(TransactionManagementError), transaction.atomic(): _roll_up(user, ASSIGNMENT_1) + + +def test_an_archived_requirement_no_longer_has_to_be_met(user, tag, and_tree) -> None: + _archive(and_tree.first) + _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_a_requirement_failed_before_it_was_archived_no_longer_holds_the_learner_back(user, tag, and_tree) -> None: + _record(user, and_tree.first, NOT_DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1) + assert _group_status(user, and_tree.leaf) == NOT_DEMONSTRATED + _archive(and_tree.first) + _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_an_archived_branch_no_longer_has_to_be_completed(user, tag, default_rule_profile) -> None: + root = _make_group(tag, logic_operator=LogicOperator.AND) + archived_leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND, archived=True) + live_leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + in_archived = _make_criterion(archived_leaf, ASSIGNMENT_1, default_rule_profile) + in_live = _make_criterion(live_leaf, ASSIGNMENT_2, default_rule_profile) + _record(user, in_archived, DEMONSTRATED) + _record(user, in_live, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, archived_leaf) is None + assert _group_status(user, live_leaf) == DEMONSTRATED + assert _group_status(user, root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_an_or_group_whose_only_demonstrated_route_is_archived_is_not_demonstrated(user, tag, or_tree) -> None: + _record(user, or_tree.first, DEMONSTRATED) + _archive(or_tree.route_1) + _archive(or_tree.first) + _record(user, or_tree.second, NOT_DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + + assert _group_status(user, or_tree.route_1) is None + assert _group_status(user, or_tree.route_2) == NOT_DEMONSTRATED + assert _group_status(user, or_tree.root) == NOT_DEMONSTRATED + assert _competency_status(user, tag) == PARTIAL + + +def test_a_competency_whose_requirements_are_all_archived_reports_no_status(user, and_tree) -> None: + _archive(and_tree.first) + _archive(and_tree.second) + + _roll_up(user, ASSIGNMENT_1) + + assert not StudentCompetencyCriteriaGroupStatus.objects.filter(user=user).exists() + assert not StudentCompetencyStatus.objects.filter(user=user).exists() + + +def test_mastery_already_earned_through_a_requirement_later_archived_is_kept(user, tag, or_tree) -> None: + """The recomputed root is NOT_DEMONSTRATED, below the stored DEMONSTRATED, so a raise-only write keeps the row.""" + _record(user, or_tree.first, DEMONSTRATED) + with freeze_time("2026-02-01"): + _roll_up(user, ASSIGNMENT_1) + _archive(or_tree.route_1) + _archive(or_tree.first) + _record(user, or_tree.second, NOT_DEMONSTRATED) + + with freeze_time("2026-03-01"): + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, or_tree.route_2) == NOT_DEMONSTRATED + assert _group_status(user, or_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + assert StudentCompetencyStatus.objects.get(tag=tag).modified == datetime(2026, 2, 1, tzinfo=timezone.utc) + + +def test_a_recorded_unmet_group_status_is_kept_when_its_only_attempted_requirement_is_archived(user, and_tree) -> None: + _record(user, and_tree.first, NOT_DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1) + _archive(and_tree.first) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.leaf) == NOT_DEMONSTRATED + + +def test_a_previously_demonstrated_requirement_changes_nothing_until_the_rollup_runs(user, tag, and_tree) -> None: + _record(user, and_tree.first, DEMONSTRATED) + _record(user, and_tree.second, NOT_DEMONSTRATED) + _roll_up(user, ASSIGNMENT_1, ASSIGNMENT_2) + assert _group_status(user, and_tree.leaf) == NOT_DEMONSTRATED + + _archive(and_tree.second) + + _roll_up(user, ASSIGNMENT_2) + assert _group_status(user, and_tree.leaf) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_an_archived_root_group_writes_neither_its_own_nor_the_competency_status( + user, tag, default_rule_profile, +) -> None: + root = _make_group(tag, archived=True) + leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + criterion = _make_criterion(leaf, ASSIGNMENT_1, default_rule_profile) + _record(user, criterion, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, leaf) == DEMONSTRATED + assert _group_status(user, root) is None + assert _competency_status(user, tag) is None + + +def test_an_archived_root_raises_its_competency_from_the_stored_root_status(user, tag, and_tree) -> None: + """A rollup that stopped between the root write and the competency write is repaired after the root is archived.""" + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=and_tree.root, status_id=DEMONSTRATED, created=NOW, modified=NOW, + ) + _archive(and_tree.first) + _archive(and_tree.second) + _archive(and_tree.root) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, and_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_a_root_raises_its_competency_from_the_stored_root_status_after_the_earning_route_is_archived( + user, tag, or_tree, +) -> None: + """A rollup that stopped between the root write and the competency write is repaired from the stored root status.""" + _record(user, or_tree.first, DEMONSTRATED) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=or_tree.root, status_id=DEMONSTRATED, created=NOW, modified=NOW, + ) + _archive(or_tree.route_1) + _record(user, or_tree.second, NOT_DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_2) + + assert _group_status(user, or_tree.root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_the_walk_continues_past_an_archived_group_to_its_parent(user, tag, default_rule_profile) -> None: + root = _make_group(tag) + archived_leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND, archived=True) + live_leaf = _make_group(tag, parent=root, logic_operator=LogicOperator.AND) + in_archived = _make_criterion(archived_leaf, ASSIGNMENT_1, default_rule_profile) + _make_criterion(live_leaf, ASSIGNMENT_2, default_rule_profile) + _record(user, in_archived, DEMONSTRATED) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=live_leaf, status_id=DEMONSTRATED, created=NOW, modified=NOW, + ) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, archived_leaf) is None + assert _group_status(user, root) == DEMONSTRATED + assert _competency_status(user, tag) == DEMONSTRATED + + +def test_an_archived_intermediate_group_cuts_its_live_subtree_off_from_the_root( + user, tag, default_rule_profile, +) -> None: + """ + The archived middle group is dropped from the root's children, so the live leaf's result does not reach the root. + """ + root = _make_group(tag) + middle = _make_group(tag, parent=root, logic_operator=LogicOperator.AND, archived=True) + leaf = _make_group(tag, parent=middle, logic_operator=LogicOperator.AND) + criterion = _make_criterion(leaf, ASSIGNMENT_1, default_rule_profile) + _record(user, criterion, DEMONSTRATED) + + _roll_up(user, ASSIGNMENT_1) + + assert _group_status(user, leaf) == DEMONSTRATED + assert _group_status(user, middle) is None + assert _group_status(user, root) is None + assert _competency_status(user, tag) is None diff --git a/tests/openedx_learning/applets/cbe/test_rollup_rules.py b/tests/openedx_learning/applets/cbe/test_rollup_rules.py index 108aecf92..273a76743 100644 --- a/tests/openedx_learning/applets/cbe/test_rollup_rules.py +++ b/tests/openedx_learning/applets/cbe/test_rollup_rules.py @@ -10,7 +10,7 @@ DEMONSTRATED = MasteryStatus.DEMONSTRATED PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED -FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED +NOT_DEMONSTRATED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED AND = LogicOperator.AND OR = LogicOperator.OR @@ -22,18 +22,18 @@ (AND, [DEMONSTRATED, DEMONSTRATED], DEMONSTRATED), (AND, [DEMONSTRATED, None], PARTIAL), (AND, [None, None], None), - (AND, [DEMONSTRATED, FAILED], FAILED), - (AND, [None, FAILED], FAILED), + (AND, [DEMONSTRATED, NOT_DEMONSTRATED], NOT_DEMONSTRATED), + (AND, [None, NOT_DEMONSTRATED], NOT_DEMONSTRATED), (AND, [PARTIAL, DEMONSTRATED], PARTIAL), - (AND, [FAILED, FAILED], FAILED), + (AND, [NOT_DEMONSTRATED, NOT_DEMONSTRATED], NOT_DEMONSTRATED), (OR, [DEMONSTRATED, None], DEMONSTRATED), - (OR, [FAILED, DEMONSTRATED], DEMONSTRATED), - (OR, [FAILED, FAILED], FAILED), - (OR, [FAILED, None], PARTIAL), - (OR, [PARTIAL, FAILED], PARTIAL), + (OR, [NOT_DEMONSTRATED, DEMONSTRATED], DEMONSTRATED), + (OR, [NOT_DEMONSTRATED, NOT_DEMONSTRATED], NOT_DEMONSTRATED), + (OR, [NOT_DEMONSTRATED, None], PARTIAL), + (OR, [PARTIAL, NOT_DEMONSTRATED], PARTIAL), (OR, [None, None], None), - (None, [FAILED, DEMONSTRATED], DEMONSTRATED), - (None, [FAILED], FAILED), + (None, [NOT_DEMONSTRATED, DEMONSTRATED], DEMONSTRATED), + (None, [NOT_DEMONSTRATED], NOT_DEMONSTRATED), (AND, [], None), (OR, [], None), ], @@ -45,7 +45,12 @@ def test_combine_children(operator, children, expected) -> None: @pytest.mark.parametrize("operator", [AND, OR, None]) @pytest.mark.parametrize( - "children", [[DEMONSTRATED, FAILED, None], [None, FAILED, PARTIAL], [DEMONSTRATED, DEMONSTRATED, PARTIAL]], + "children", + [ + [DEMONSTRATED, NOT_DEMONSTRATED, None], + [None, NOT_DEMONSTRATED, 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.""" diff --git a/tests/openedx_learning/applets/cbe/test_tag_rollup.py b/tests/openedx_learning/applets/cbe/test_tag_rollup.py index 5d9cf2646..20a99e704 100644 --- a/tests/openedx_learning/applets/cbe/test_tag_rollup.py +++ b/tests/openedx_learning/applets/cbe/test_tag_rollup.py @@ -7,6 +7,7 @@ from unittest import mock import pytest +from django.contrib.auth import get_user_model from openedx_learning.api import roll_up_competency_statuses from openedx_learning.applets.cbe import rollup @@ -28,7 +29,7 @@ DEMONSTRATED = MasteryStatus.DEMONSTRATED PARTIAL = MasteryStatus.PARTIALLY_ATTEMPTED -FAILED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED +NOT_DEMONSTRATED = MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED NOW = datetime(2026, 1, 1, tzinfo=timezone.utc) @@ -42,17 +43,35 @@ def _object_id(name: str) -> str: 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) +def _add_requirement( + tag: Tag, object_id: str, profile: CompetencyRuleProfile, *, archived: bool = False, +) -> SimpleNamespace: + """Give ``tag`` a root group with one criterion on ``object_id``, both archived when ``archived`` is set.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag, archived=archived) 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) + criterion = CompetencyCriterion.objects.create( + group=root, object_tag=object_tag, rule_profile=profile, archived=archived, + ) return SimpleNamespace(root=root, criterion=criterion, object_id=object_id) -def _record(user, earnable: SimpleNamespace, status: MasteryStatus) -> None: +def _add_nested_requirement( + tag: Tag, object_id: str, profile: CompetencyRuleProfile, *, levels: int, +) -> SimpleNamespace: + """Give ``tag`` a root group and one criterion on ``object_id`` that sits ``levels`` groups below the root.""" + root = CompetencyCriteriaGroup.objects.create(tag=tag) + group = root + for _ in range(levels): + group = CompetencyCriteriaGroup.objects.create(tag=tag, parent=group) + object_tag = ObjectTag.objects.create(object_id=object_id, taxonomy=tag.taxonomy, tag=tag) + criterion = CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=profile) + return SimpleNamespace(root=root, group=group, criterion=criterion, object_id=object_id) + + +def _record(user, requirement: SimpleNamespace, status: MasteryStatus) -> None: + """Store the learner's status on the requirement's criterion as the grade transaction would have.""" StudentCompetencyCriterionStatus.objects.create( - user=user, criterion=earnable.criterion, status_id=status, created=NOW, modified=NOW, + user=user, criterion=requirement.criterion, status_id=status, created=NOW, modified=NOW, ) @@ -63,6 +82,11 @@ def _set_group_status(user, group: CompetencyCriteriaGroup, status: MasteryStatu ) +def _set_competency_status(user, tag: Tag, status: MasteryStatus) -> None: + """Store a competency status as an earlier rollup would have.""" + StudentCompetencyStatus.objects.create(user=user, tag=tag, 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 @@ -87,10 +111,10 @@ def test_demonstrating_the_only_sub_competency_raises_the_parent_and_grandparent 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) + requirement = _add_requirement(child, _object_id("c"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert [_competency_status(user, tag) for tag in (child, parent, grandparent)] == [DEMONSTRATED] * 3 @@ -98,50 +122,50 @@ def test_demonstrating_the_only_sub_competency_raises_the_parent_and_grandparent 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) + requirement = _add_requirement(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, requirement, NOT_DEMONSTRATED) + parent_requirement = _add_requirement(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, parent_requirement.root, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.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) + _add_requirement(family.parent, _object_id("p"), default_rule_profile) + requirements = [ + _add_requirement(child, _object_id(f"c{index}"), default_rule_profile) for index, child in enumerate(family.children) ] - for earnable in earnables: - _record(user, earnable, DEMONSTRATED) + for requirement in requirements: + _record(user, requirement, DEMONSTRATED) - _roll_up(user, *(earnable.object_id for earnable in earnables)) + _roll_up(user, *(requirement.object_id for requirement in requirements)) 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) + _add_requirement(family.parent, _object_id("p"), default_rule_profile) + requirement = _add_requirement(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) + _add_requirement(family.children[1], _object_id("c2"), default_rule_profile) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.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) + requirement = _add_requirement(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, requirement, NOT_DEMONSTRATED) + parent_requirement = _add_requirement(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, parent_requirement.root, NOT_DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, family.parent) == PARTIAL @@ -149,12 +173,12 @@ def test_a_failed_own_root_group_shows_progress_on_the_parent(user, family, defa def test_sub_competencies_without_criteria_do_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) + requirement = _add_requirement(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) + parent_requirement = _add_requirement(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, parent_requirement.root, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, family.parent) == DEMONSTRATED @@ -163,12 +187,12 @@ def test_a_sub_competency_without_criteria_caps_a_parent_that_has_no_requirement user, competency_taxonomy, default_rule_profile, ) -> None: parent = _make_tag(competency_taxonomy, "Parent") - earnable_child = _make_tag(competency_taxonomy, "Earnable", parent) - _make_tag(competency_taxonomy, "Unearnable", parent) - earnable = _earnable(earnable_child, _object_id("c"), default_rule_profile) - _record(user, earnable, DEMONSTRATED) + child_with_requirement = _make_tag(competency_taxonomy, "With requirement", parent) + _make_tag(competency_taxonomy, "Without requirement", parent) + requirement = _add_requirement(child_with_requirement, _object_id("c"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, parent) == PARTIAL @@ -179,10 +203,10 @@ def test_a_sub_competency_with_criteria_only_further_down_still_participates( 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) + requirement = _add_requirement(leaf, _object_id("c"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, leaf) == DEMONSTRATED assert _competency_status(user, middle) == DEMONSTRATED @@ -195,20 +219,20 @@ def test_a_stored_demonstrated_parent_is_not_lowered_when_only_some_sub_competen 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) + requirement = _add_requirement(family.children[0], _object_id("c1"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.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) + requirement = _add_requirement(root_tag, _object_id("r"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert list(StudentCompetencyStatus.objects.values_list("tag_id", flat=True)) == [root_tag.pk] @@ -217,10 +241,10 @@ def test_the_deepest_allowed_taxonomy_is_walked_to_the_top(user, competency_taxo 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) + requirement = _add_requirement(tags[-1], _object_id("deep"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, tags[0]) == DEMONSTRATED assert _competency_status(user, tags[1]) == DEMONSTRATED @@ -242,11 +266,11 @@ def test_a_taxonomy_deeper_than_the_cap_logs_the_parents_left_unrecomputed( parent=tags[-2], depth=depth, lineage=tags[-2].lineage + tags[-1].value + "\t", ) tags[-1].refresh_from_db() - earnable = _earnable(tags[-1], _object_id("too-deep"), default_rule_profile) - _record(user, earnable, DEMONSTRATED) + requirement = _add_requirement(tags[-1], _object_id("too-deep"), default_rule_profile) + _record(user, requirement, DEMONSTRATED) with caplog.at_level(logging.ERROR, logger=rollup.__name__): - _roll_up(user, earnable.object_id) + _roll_up(user, requirement.object_id) assert _competency_status(user, tags[1]) == DEMONSTRATED assert _competency_status(user, tags[0]) is None @@ -259,17 +283,17 @@ def test_a_taxonomy_deeper_than_the_cap_logs_the_parents_left_unrecomputed( 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) + requirements = [ + _add_requirement(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) + for requirement in requirements: + _record(user, requirement, 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)) + _roll_up(user, *(requirement.object_id for requirement in requirements)) assert combine.call_count == 2 # the parent once, then the grandparent once @@ -283,14 +307,14 @@ def test_parents_at_the_same_level_are_rolled_up_independently( 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), + _add_requirement(only_child, _object_id("only"), default_rule_profile), + _add_requirement(first, _object_id("first"), default_rule_profile), ] - _earnable(second, _object_id("second"), default_rule_profile) - for earnable in graded: - _record(user, earnable, DEMONSTRATED) + _add_requirement(second, _object_id("second"), default_rule_profile) + for requirement in graded: + _record(user, requirement, DEMONSTRATED) - _roll_up(user, *(earnable.object_id for earnable in graded)) + _roll_up(user, *(requirement.object_id for requirement in graded)) assert _competency_status(user, parent_one) == DEMONSTRATED assert _competency_status(user, parent_two) == PARTIAL @@ -304,15 +328,173 @@ def test_competencies_graded_at_different_depths_converge_on_the_shared_ancestor 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), + _add_requirement(deep, _object_id("deep"), default_rule_profile), + _add_requirement(shallow, _object_id("shallow"), default_rule_profile), ] - for earnable in graded: - _record(user, earnable, DEMONSTRATED) + for requirement in graded: + _record(user, requirement, DEMONSTRATED) - _roll_up(user, *(earnable.object_id for earnable in graded)) + _roll_up(user, *(requirement.object_id for requirement in graded)) assert _competency_status(user, deep) == DEMONSTRATED assert _competency_status(user, middle) == DEMONSTRATED assert _competency_status(user, shallow) == DEMONSTRATED assert _competency_status(user, top) == DEMONSTRATED + + +def test_a_parent_with_its_own_requirements_is_demonstrated_when_a_sub_competency_is_fully_archived( + user, family, default_rule_profile, +) -> None: + """The parent is demonstrated through its own requirement, whatever state its sub-competencies are in.""" + archived_child, graded_child, _child_without_requirements = family.children + parent_requirement = _add_requirement(family.parent, _object_id("p"), default_rule_profile) + _set_group_status(user, parent_requirement.root, DEMONSTRATED) + _add_requirement(archived_child, _object_id("c1"), default_rule_profile, archived=True) + graded_requirement = _add_requirement(graded_child, _object_id("c2"), default_rule_profile) + _record(user, graded_requirement, DEMONSTRATED) + + _roll_up(user, graded_requirement.object_id) + + assert _competency_status(user, graded_child) == DEMONSTRATED + assert _competency_status(user, family.parent) == DEMONSTRATED + + +def test_a_parent_with_no_requirements_of_its_own_stays_short_when_a_sub_competency_is_fully_archived( + user, competency_taxonomy, default_rule_profile, +) -> None: + """The archived sub-competency can never be demonstrated, so the parent cannot have every sub-competency.""" + parent = _make_tag(competency_taxonomy, "Parent") + archived_child = _make_tag(competency_taxonomy, "Archived", parent) + graded_child = _make_tag(competency_taxonomy, "Graded", parent) + _add_requirement(archived_child, _object_id("a"), default_rule_profile, archived=True) + graded_requirement = _add_requirement(graded_child, _object_id("g"), default_rule_profile) + _record(user, graded_requirement, DEMONSTRATED) + + _roll_up(user, graded_requirement.object_id) + + assert _competency_status(user, graded_child) == DEMONSTRATED + assert _competency_status(user, parent) == PARTIAL + + +def test_an_archived_root_group_still_starts_the_parent_competency_walk( + user, competency_taxonomy, default_rule_profile, +) -> None: + """A rollup for an archived requirement writes nothing for its own competency but still recomputes the parent.""" + parent = _make_tag(competency_taxonomy, "Parent") + archived_child = _make_tag(competency_taxonomy, "Archived", parent) + demonstrated_child = _make_tag(competency_taxonomy, "Demonstrated earlier", parent) + _set_competency_status(user, demonstrated_child, DEMONSTRATED) + archived_requirement = _add_requirement(archived_child, _object_id("a"), default_rule_profile, archived=True) + _record(user, archived_requirement, DEMONSTRATED) + + _roll_up(user, archived_requirement.object_id) + + assert _competency_status(user, archived_child) is None + assert _competency_status(user, parent) == PARTIAL + + +def test_a_parents_own_archived_root_still_counts_through_its_stored_status( + user, family, default_rule_profile, +) -> None: + """The parent demonstrated its own requirement before that requirement was archived, and keeps the result.""" + graded_child = family.children[0] + parent_requirement = _add_requirement(family.parent, _object_id("p"), default_rule_profile, archived=True) + _set_group_status(user, parent_requirement.root, DEMONSTRATED) + graded_requirement = _add_requirement(graded_child, _object_id("c1"), default_rule_profile) + _record(user, graded_requirement, NOT_DEMONSTRATED) + + _roll_up(user, graded_requirement.object_id) + + assert _competency_status(user, graded_child) == PARTIAL + assert _competency_status(user, family.parent) == DEMONSTRATED + + +def test_a_parent_whose_only_sub_competency_has_no_status_gets_no_status( + user, competency_taxonomy, default_rule_profile, +) -> None: + """The parent has no requirement of its own and no sub-competency with a status, so there is nothing to write.""" + parent = _make_tag(competency_taxonomy, "Parent") + archived_child = _make_tag(competency_taxonomy, "Archived", parent) + archived_requirement = _add_requirement(archived_child, _object_id("a"), default_rule_profile, archived=True) + _record(user, archived_requirement, DEMONSTRATED) + + _roll_up(user, archived_requirement.object_id) + + assert _competency_status(user, archived_child) is None + assert _competency_status(user, parent) is None + + +def test_root_groups_reached_at_different_depths_each_start_their_own_parent_walk( + user, competency_taxonomy, default_rule_profile, +) -> None: + """The shallow root is reached on the first level of the group walk and the deep root two levels later.""" + shallow_parent = _make_tag(competency_taxonomy, "Shallow parent") + shallow = _make_tag(competency_taxonomy, "Shallow", shallow_parent) + deep_parent = _make_tag(competency_taxonomy, "Deep parent") + deep = _make_tag(competency_taxonomy, "Deep", deep_parent) + graded = [ + _add_requirement(shallow, _object_id("shallow"), default_rule_profile), + _add_nested_requirement(deep, _object_id("deep"), default_rule_profile, levels=2), + ] + for requirement in graded: + _record(user, requirement, DEMONSTRATED) + + _roll_up(user, *(requirement.object_id for requirement in graded)) + + assert _competency_status(user, shallow_parent) == DEMONSTRATED + assert _competency_status(user, deep_parent) == DEMONSTRATED + + +def test_another_learners_statuses_do_not_count_toward_the_rolled_up_learner( + user, competency_taxonomy, default_rule_profile, +) -> None: + """The other learner holds a status at every level that the rollup reads: criterion, root group, competency.""" + other_learner = get_user_model().objects.create(username="other_learner") + parent = _make_tag(competency_taxonomy, "Parent") + graded_child = _make_tag(competency_taxonomy, "Graded", parent) + other_child = _make_tag(competency_taxonomy, "Other", parent) + parent_requirement = _add_requirement(parent, _object_id("p"), default_rule_profile) + graded_requirement = _add_requirement(graded_child, _object_id("graded"), default_rule_profile) + other_requirement = _add_requirement(other_child, _object_id("other"), default_rule_profile) + _record(user, graded_requirement, DEMONSTRATED) + _record(other_learner, other_requirement, DEMONSTRATED) + _set_group_status(other_learner, parent_requirement.root, DEMONSTRATED) + _set_competency_status(other_learner, other_child, DEMONSTRATED) + + _roll_up(user, graded_requirement.object_id, other_requirement.object_id) + + assert _competency_status(user, graded_child) == DEMONSTRATED + assert _competency_status(user, other_child) is None + assert _competency_status(user, parent) == PARTIAL + assert _competency_status(other_learner, parent) is None + + +def test_a_repeated_rollup_of_nested_groups_under_a_parent_competency_runs_a_fixed_number_of_queries( + user, competency_taxonomy, default_rule_profile, django_assert_num_queries, +) -> None: + """The first rollup creates every status row, so the repeated one only reads and compares.""" + parent = _make_tag(competency_taxonomy, "Parent") + child = _make_tag(competency_taxonomy, "Child", parent) + requirement = _add_nested_requirement(child, _object_id("c"), default_rule_profile, levels=1) + _record(user, requirement, DEMONSTRATED) + _roll_up(user, requirement.object_id) + + with django_assert_num_queries(23): + _roll_up(user, requirement.object_id) + + +def test_a_nested_group_in_a_parents_own_tree_does_not_count_as_the_parents_own_result( + user, competency_taxonomy, default_rule_profile, +) -> None: + """Only a root group's status is the parent's own result, and this learner has none on the parent's root.""" + parent = _make_tag(competency_taxonomy, "Parent") + child = _make_tag(competency_taxonomy, "Child", parent) + parent_requirement = _add_nested_requirement(parent, _object_id("p"), default_rule_profile, levels=1) + _set_group_status(user, parent_requirement.group, DEMONSTRATED) + child_requirement = _add_requirement(child, _object_id("c"), default_rule_profile) + _record(user, child_requirement, NOT_DEMONSTRATED) + + _roll_up(user, child_requirement.object_id) + + assert _competency_status(user, child) == PARTIAL + assert _competency_status(user, parent) == PARTIAL