From ea5afa27f4b039cb43816967be1638ff7a3df097 Mon Sep 17 00:00:00 2001 From: Felipe Montoya Date: Tue, 10 Nov 2026 20:01:20 -0500 Subject: [PATCH] fix: send post_save when a learner competency status is raised Co-Authored-By: Claude Opus 5.5 --- src/openedx_learning/applets/cbe/statuses.py | 13 ++++ .../applets/cbe/test_statuses.py | 68 ++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/openedx_learning/applets/cbe/statuses.py b/src/openedx_learning/applets/cbe/statuses.py index e30a05f34..68e76cfbd 100644 --- a/src/openedx_learning/applets/cbe/statuses.py +++ b/src/openedx_learning/applets/cbe/statuses.py @@ -8,6 +8,7 @@ from typing import TypeVar from django.db import models +from django.db.models.signals import post_save from .models import ( MasteryStatus, @@ -32,6 +33,9 @@ def raise_status( 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``. + + ``post_save`` fires for a created row (from ``get_or_create``) and for a raised one (sent here, because the raise + is a bulk ``UPDATE``), so listeners such as analytics sinks see every change. It does not fire when nothing changed. """ row, created = manager.get_or_create(**lookup, defaults={"status_id": status, "created": now, "modified": now}) if created: @@ -41,4 +45,13 @@ def raise_status( changed = manager.filter(pk=row.pk, status_id__lt=status).update(status_id=status, modified=now) if changed: log.info("Raised %s %s: status %s -> %s", manager.model.__name__, lookup, row.status_id, status) + row.status_id, row.modified = status, now + post_save.send( + sender=manager.model, + instance=row, + created=False, + update_fields=frozenset({"status_id", "modified"}), + raw=False, + using=manager.db, + ) return changed diff --git a/tests/openedx_learning/applets/cbe/test_statuses.py b/tests/openedx_learning/applets/cbe/test_statuses.py index 5542f4157..95c7b1c6a 100644 --- a/tests/openedx_learning/applets/cbe/test_statuses.py +++ b/tests/openedx_learning/applets/cbe/test_statuses.py @@ -5,10 +5,11 @@ from datetime import datetime, timezone import pytest +from django.db.models.signals import post_save from openedx_learning.applets.cbe import statuses from openedx_learning.applets.cbe.statuses import raise_status -from openedx_learning.models import MasteryStatus, StudentCompetencyStatus +from openedx_learning.models import MasteryStatus, StudentCompetencyCriteriaGroupStatus, StudentCompetencyStatus pytestmark = pytest.mark.django_db @@ -93,3 +94,68 @@ def test_an_unchanged_status_is_not_logged(user, tag, caplog: pytest.LogCaptureF _write(user, tag, MasteryStatus.DEMONSTRATED, SECOND) assert not caplog.records + + +@pytest.fixture(name="saves") +def fixture_saves(): + """Record every post_save sent for StudentCompetencyStatus as (created, status_id, modified).""" + received = [] + + def receiver(sender, instance, created, **kwargs): # pylint: disable=unused-argument + received.append((created, instance.status_id, instance.modified)) + + post_save.connect(receiver, sender=StudentCompetencyStatus, weak=False) + yield received + post_save.disconnect(receiver, sender=StudentCompetencyStatus) + + +def test_creating_a_row_sends_post_save(user, tag, saves) -> None: + """A created row sends post_save with created=True.""" + _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, FIRST) + + assert saves == [(True, MasteryStatus.PARTIALLY_ATTEMPTED, FIRST)] + + +def test_raising_a_status_sends_post_save(user, tag, saves) -> None: + """A raise is a bulk UPDATE, so raise_status sends post_save itself, with the raised values.""" + _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, FIRST) + + _write(user, tag, MasteryStatus.DEMONSTRATED, SECOND) + + assert saves[1:] == [(False, MasteryStatus.DEMONSTRATED, SECOND)] + + +def test_an_unchanged_status_sends_no_post_save(user, tag, saves) -> None: + """A write that leaves the stored status as it was sends nothing.""" + _write(user, tag, MasteryStatus.DEMONSTRATED, FIRST) + + _write(user, tag, MasteryStatus.PARTIALLY_ATTEMPTED, SECOND) + _write(user, tag, MasteryStatus.DEMONSTRATED, SECOND) + + assert len(saves) == 1 + + +def test_raising_a_group_status_sends_post_save(user, group) -> None: + """Group status raises are bulk UPDATEs too, so they send post_save like competency status raises.""" + received = [] + + def receiver(sender, instance, created, **kwargs): # pylint: disable=unused-argument + received.append((created, instance.status_id, instance.modified)) + + post_save.connect(receiver, sender=StudentCompetencyCriteriaGroupStatus, weak=False) + try: + lookup = {"user_id": user.id, "group_id": group.id} + raise_status( + StudentCompetencyCriteriaGroupStatus.objects, lookup=lookup, + status=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, now=FIRST, + ) + raise_status( + StudentCompetencyCriteriaGroupStatus.objects, lookup=lookup, status=MasteryStatus.DEMONSTRATED, now=SECOND, + ) + finally: + post_save.disconnect(receiver, sender=StudentCompetencyCriteriaGroupStatus) + + assert received == [ + (True, MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, FIRST), + (False, MasteryStatus.DEMONSTRATED, SECOND), + ]