diff --git a/src/openedx_learning/applets/cbe/README.rst b/src/openedx_learning/applets/cbe/README.rst index 869474767..1d20f4fd0 100644 --- a/src/openedx_learning/applets/cbe/README.rst +++ b/src/openedx_learning/applets/cbe/README.rst @@ -15,7 +15,7 @@ taxonomy; put a queryset through ``select_competency_taxonomies()`` first when c list, so the whole list costs one query instead of one per row. Keeping the check here is what lets ``openedx_tagging`` stay a generic tagging library that never learns CBE exists. -The criteria, rule profile, and learner status tables this applet still needs are designed -in ``docs/openedx_learning/decisions/``. ``openedx_tagging`` ADR 0013 covers how +The criteria, rule profile, and learner status tables are designed in +``docs/openedx_learning/decisions/``. ``openedx_tagging`` ADR 0013 covers how ``openedx-platform`` calls ``is_competency_taxonomy()`` to report a taxonomy's type to Studio. diff --git a/src/openedx_learning/applets/cbe/models/__init__.py b/src/openedx_learning/applets/cbe/models/__init__.py index 2e6bad99b..8b525773c 100644 --- a/src/openedx_learning/applets/cbe/models/__init__.py +++ b/src/openedx_learning/applets/cbe/models/__init__.py @@ -5,7 +5,13 @@ from ..rule_payloads import RuleType from .competency_taxonomy import CompetencyTaxonomy from .criteria import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, LogicOperator -from .learner_status import CompetencyMasteryStatus, MasteryStatus, StudentCompetencyStatus +from .learner_status import ( + CompetencyMasteryStatus, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriterionStatus, + StudentCompetencyStatus, +) __all__ = [ "CompetencyCriteriaGroup", @@ -16,5 +22,7 @@ "LogicOperator", "MasteryStatus", "RuleType", + "StudentCompetencyCriteriaGroupStatus", + "StudentCompetencyCriterionStatus", "StudentCompetencyStatus", ] diff --git a/src/openedx_learning/applets/cbe/models/learner_status.py b/src/openedx_learning/applets/cbe/models/learner_status.py index 958253654..4db925a3e 100644 --- a/src/openedx_learning/applets/cbe/models/learner_status.py +++ b/src/openedx_learning/applets/cbe/models/learner_status.py @@ -1,5 +1,16 @@ """ -Models tracking a learner's mastery status for a competency. +Models tracking a learner's mastery status at each level of a criteria tree. + +:class:`StudentCompetencyCriterionStatus` covers one leaf :class:`~openedx_learning.models.CompetencyCriterion`, +:class:`StudentCompetencyCriteriaGroupStatus` covers one +:class:`~openedx_learning.models.CompetencyCriteriaGroup`, and :class:`StudentCompetencyStatus` covers one +:class:`~openedx_tagging.models.Tag`. Each holds one row per learner per node, updated in place (ADR-0003 +Decision 5). Which writes may raise or lower a status is decided in the API layer, not here (ADR-0004 +Decisions 4 and 6). ``created`` and ``modified`` are caller-supplied because ``auto_now`` does not fire on +``QuerySet.update()``, which is how conditional raises are written. ``user`` is ``CASCADE`` so that this +library never vetoes ``User.delete()`` platform-wide. The node foreign keys are ``PROTECT`` because every +foreign key above them in the criteria tree is ``CASCADE``: without ``PROTECT``, deleting a tag, taxonomy, +group or ``ObjectTag`` would silently delete the learner status beneath it (ADR-0002 Decision 7). """ from django.conf import settings from django.db import models @@ -7,10 +18,14 @@ from openedx_django_lib.fields import manual_date_time_field from openedx_tagging.models import Tag +from .criteria import CompetencyCriteriaGroup, CompetencyCriterion + __all__ = [ "MasteryStatus", "CompetencyMasteryStatus", "StudentCompetencyStatus", + "StudentCompetencyCriterionStatus", + "StudentCompetencyCriteriaGroupStatus", ] @@ -97,3 +112,79 @@ class Meta: name="oex_learning_studentcompetencystatus_status_allowed", ), ] + + +class StudentCompetencyCriterionStatus(models.Model): + """ + A learner's current mastery status for one leaf ``CompetencyCriterion``. + + One row per learner per criterion, updated in place (ADR-0003 Decision 5). + + .. no_pii: + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="+", + ) + criterion = models.ForeignKey( + CompetencyCriterion, + db_column="competency_criteria_id", + on_delete=models.PROTECT, + related_name="+", + ) + status = models.ForeignKey( + CompetencyMasteryStatus, + on_delete=models.PROTECT, + related_name="+", + ) + created = manual_date_time_field() + modified = manual_date_time_field() + + class Meta: + constraints = [ + # ADR-0002 Decision 5 index 6. + models.UniqueConstraint( + fields=("user", "criterion"), + name="oex_learning_studentcriterionstatus_user_criterion_uniq", + ), + ] + + +class StudentCompetencyCriteriaGroupStatus(models.Model): + """ + A learner's current mastery status for one ``CompetencyCriteriaGroup``. + + One row per learner per group, updated in place (ADR-0003 Decision 5). + + .. no_pii: + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="+", + ) + group = models.ForeignKey( + CompetencyCriteriaGroup, + db_column="competency_criteria_group_id", + on_delete=models.PROTECT, + related_name="+", + ) + status = models.ForeignKey( + CompetencyMasteryStatus, + on_delete=models.PROTECT, + related_name="+", + ) + created = manual_date_time_field() + modified = manual_date_time_field() + + class Meta: + constraints = [ + # ADR-0002 Decision 5 index 7. + models.UniqueConstraint( + fields=("user", "group"), + name="oex_learning_studentcriteriagroupstatus_user_group_uniq", + ), + ] diff --git a/src/openedx_learning/migrations/0011_studentcompetencycriterionstatus.py b/src/openedx_learning/migrations/0011_studentcompetencycriterionstatus.py new file mode 100644 index 000000000..beb518787 --- /dev/null +++ b/src/openedx_learning/migrations/0011_studentcompetencycriterionstatus.py @@ -0,0 +1,32 @@ +# Generated by Django 5.2.16 on 2026-09-28 16:09 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import openedx_django_lib.validators + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_learning', '0010_studentcompetencystatus'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='StudentCompetencyCriterionStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('modified', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('criterion', models.ForeignKey(db_column='competency_criteria_id', on_delete=django.db.models.deletion.PROTECT, related_name='+', to='openedx_learning.competencycriterion')), + ('status', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='+', to='openedx_learning.competencymasterystatus')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'constraints': [models.UniqueConstraint(fields=('user', 'criterion'), name='oex_learning_studentcriterionstatus_user_criterion_uniq')], + }, + ), + ] diff --git a/src/openedx_learning/migrations/0012_studentcompetencycriteriagroupstatus.py b/src/openedx_learning/migrations/0012_studentcompetencycriteriagroupstatus.py new file mode 100644 index 000000000..cb06d5650 --- /dev/null +++ b/src/openedx_learning/migrations/0012_studentcompetencycriteriagroupstatus.py @@ -0,0 +1,32 @@ +# Generated by Django 5.2.16 on 2026-09-28 16:26 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import openedx_django_lib.validators + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_learning', '0011_studentcompetencycriterionstatus'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='StudentCompetencyCriteriaGroupStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('modified', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('group', models.ForeignKey(db_column='competency_criteria_group_id', on_delete=django.db.models.deletion.PROTECT, related_name='+', to='openedx_learning.competencycriteriagroup')), + ('status', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='+', to='openedx_learning.competencymasterystatus')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'constraints': [models.UniqueConstraint(fields=('user', 'group'), name='oex_learning_studentcriteriagroupstatus_user_group_uniq')], + }, + ), + ] diff --git a/tests/openedx_learning/applets/cbe/conftest.py b/tests/openedx_learning/applets/cbe/conftest.py index 58935b6f0..8053fda18 100644 --- a/tests/openedx_learning/applets/cbe/conftest.py +++ b/tests/openedx_learning/applets/cbe/conftest.py @@ -1,5 +1,8 @@ """Shared fixtures for the CBE criteria test modules.""" +from datetime import datetime, timezone + import pytest +from django.contrib.auth import get_user_model from organizations.api import ensure_organization from organizations.models import Organization @@ -65,3 +68,15 @@ def _default_rule_profile() -> CompetencyRuleProfile: course__isnull=True, competency_taxonomy__isnull=True, ) + + +@pytest.fixture(name="user") +def _user(): + """Create a single learner for use in these tests.""" + return get_user_model().objects.create(username="learner") + + +@pytest.fixture(name="now") +def _now() -> datetime: + """A single UTC timestamp shared by writes in a test.""" + return datetime.now(timezone.utc) diff --git a/tests/openedx_learning/applets/cbe/test_criteria_group_status.py b/tests/openedx_learning/applets/cbe/test_criteria_group_status.py new file mode 100644 index 000000000..7d169d085 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criteria_group_status.py @@ -0,0 +1,178 @@ +"""Tests for StudentCompetencyCriteriaGroupStatus, a learner's status for one criteria group.""" +from datetime import datetime + +import pytest +from django.contrib.auth import get_user_model +from django.db import IntegrityError, transaction +from django.db.models import ProtectedError + +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriterionStatus, + StudentCompetencyStatus, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + + +def test_second_row_for_the_same_user_and_group_is_rejected( + user, group: CompetencyCriteriaGroup, now: datetime +) -> None: + """ + Learner status rows are updated in place, one row per learner and node under a unique constraint: + a second row for the same learner and group is rejected. + """ + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.PARTIALLY_ATTEMPTED, created=now, modified=now, + ) + + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + +def test_attempted_not_demonstrated_is_accepted(user, group: CompetencyCriteriaGroup, now: datetime) -> None: + """ + The models accept any status value the caller writes, including `AttemptedNotDemonstrated`, which + only `StudentCompetencyStatus` rejects. + """ + row = StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, created=now, modified=now, + ) + + row.refresh_from_db() + assert row.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + +def test_group_status_accepts_a_lower_status_written_over_a_higher_one( + user, group: CompetencyCriteriaGroup, now: datetime +) -> None: + """ + The model accepts any status value the caller writes, so a lower status written over a higher one takes + effect. Staff edits may lower a status, and the monotone-write rule (a status is only ever raised) applies + to the default tree calculation alone, which enforces it in the API layer. + """ + row = StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + changed = StudentCompetencyCriteriaGroupStatus.objects.filter(pk=row.pk).update( + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, modified=now, + ) + + assert changed == 1 + row.refresh_from_db() + assert row.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + +def test_group_delete_is_protected_by_its_status(user, group: CompetencyCriteriaGroup, now: datetime) -> None: + """ + The foreign key from `StudentCompetencyCriteriaGroupStatus` to its definition row, + `CompetencyCriteriaGroup`, is `PROTECT`: deleting a group that holds a learner status raises `ProtectedError`. + """ + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + group.delete() + + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + +def test_root_group_delete_is_protected_by_a_descendant_group_status( + user, group: CompetencyCriteriaGroup, now: datetime +) -> None: + """ + The transitive cases are tested, not only the direct ones: deleting a root `CompetencyCriteriaGroup` + above a descendant group that holds a learner status row raises `ProtectedError`. + """ + child_group = CompetencyCriteriaGroup.objects.create(tag=group.tag, parent=group) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=child_group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + group.delete() + + assert CompetencyCriteriaGroup.objects.filter(pk__in=[group.pk, child_group.pk]).count() == 2 + + +def test_tag_delete_is_protected_by_a_group_status_with_no_criterion_status_beneath( + user, tag: Tag, group: CompetencyCriteriaGroup, now: datetime +) -> None: + """ + Deleting an `oel_tagging_tag` with a learner status row anywhere beneath it raises `ProtectedError`, + even when that row is a group status with no criterion status beneath it. + """ + # #642 wants the database to fail closed here, although ADR-0002 Decision 7's predicate reads only + # the criterion table. + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + assert not StudentCompetencyCriterionStatus.objects.exists() + + with pytest.raises(ProtectedError), transaction.atomic(): + tag.delete() + + assert Tag.objects.filter(pk=tag.pk).exists() + + +def test_mastery_status_row_delete_is_protected_by_a_group_status( + user, group: CompetencyCriteriaGroup, now: datetime +) -> None: + """ + The `status_id` foreign key to `CompetencyMasteryStatuses` is `PROTECT`. + """ + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + CompetencyMasteryStatus.objects.get(pk=MasteryStatus.DEMONSTRATED).delete() + + assert CompetencyMasteryStatus.objects.filter(pk=MasteryStatus.DEMONSTRATED).exists() + + +def test_user_delete_removes_only_that_users_rows_from_all_three_status_tables( + user, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, + now: datetime, +) -> None: + """ + Deleting a user row removes that user's status rows across all three `Student*Status` models, and + leaves other users' rows in place. + """ + other_user = get_user_model().objects.create(username="other_learner") + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + StudentCompetencyStatus.objects.create( + user=user, tag=group.tag, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=other_user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + user_id = user.pk + + user.delete() + + assert not StudentCompetencyCriterionStatus.objects.filter(user_id=user_id).exists() + assert not StudentCompetencyCriteriaGroupStatus.objects.filter(user_id=user_id).exists() + assert not StudentCompetencyStatus.objects.filter(user_id=user_id).exists() + assert StudentCompetencyCriteriaGroupStatus.objects.filter(user=other_user).exists() diff --git a/tests/openedx_learning/applets/cbe/test_criterion_status.py b/tests/openedx_learning/applets/cbe/test_criterion_status.py new file mode 100644 index 000000000..9ad513389 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criterion_status.py @@ -0,0 +1,371 @@ +"""Tests for StudentCompetencyCriterionStatus, a learner's status for one leaf criterion.""" +from datetime import datetime + +import pytest +from django.contrib.auth import get_user_model +from django.db import IntegrityError, transaction +from django.db.models import ProtectedError + +from openedx_catalog.models import CourseRun +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + StudentCompetencyCriterionStatus, +) +from openedx_tagging import api as tagging_api +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + + +@pytest.fixture(name="criterion") +def _criterion( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> CompetencyCriterion: + """A leaf CompetencyCriterion directly under the root `group`.""" + return CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + + +def test_second_row_for_the_same_user_and_criterion_is_rejected( + user, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Learner status rows are updated in place, one row per learner and node under a unique constraint: + a second row for the same learner and criterion is rejected. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.PARTIALLY_ATTEMPTED, created=now, modified=now, + ) + + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + +def test_same_user_can_hold_status_for_a_second_criterion( + user, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + One row per learner and node: the unique constraint is on the learner and the criterion together, so + the same learner holds a separate row for each criterion. + """ + second_criterion = CompetencyCriterion.objects.create( + group=criterion.group, object_tag=criterion.object_tag, rule_profile=criterion.rule_profile + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=second_criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + assert StudentCompetencyCriterionStatus.objects.filter(user=user).count() == 2 + + +def test_attempted_not_demonstrated_is_accepted(user, criterion: CompetencyCriterion, now: datetime) -> None: + """ + The models accept any status value the caller writes, including `AttemptedNotDemonstrated`, which + only `StudentCompetencyStatus` rejects. + """ + row = StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, created=now, modified=now, + ) + + row.refresh_from_db() + assert row.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + +def test_criterion_status_accepts_a_lower_status_written_over_a_higher_one( + user, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + The model accepts any status value the caller writes, so a lower status written over a higher one takes + effect. Staff may downward-correct a wrong status, and the monotone-write rule (a status is only ever + raised) applies to the default tree calculation alone, which enforces it in the API layer. + """ + row = StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + changed = StudentCompetencyCriterionStatus.objects.filter(pk=row.pk).update( + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, modified=now, + ) + + assert changed == 1 + row.refresh_from_db() + assert row.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + +def test_criterion_delete_is_protected_by_its_status(user, criterion: CompetencyCriterion, now: datetime) -> None: + """ + The foreign key from `StudentCompetencyCriterionStatus` to its definition row, `CompetencyCriterion`, + is `PROTECT`: deleting a criterion that holds a learner status raises `ProtectedError`. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + criterion.delete() + + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_object_tag_delete_is_protected_by_a_criterion_status( + user, object_tag: ObjectTag, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_objecttag` whose criterion has a learner status row raises `ProtectedError`. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + object_tag.delete() + + assert ObjectTag.objects.filter(pk=object_tag.pk).exists() + + +def test_tagging_api_object_tag_delete_is_protected_by_a_criterion_status( + user, object_tag: ObjectTag, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_objecttag` whose criterion has a learner status row raises `ProtectedError`, + also through `openedx_tagging.api.delete_object_tags()`. + """ + # delete_object_tags() deletes through a QuerySet, a different call path from Model.delete(). + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + tagging_api.delete_object_tags(object_tag.object_id) + + assert ObjectTag.objects.filter(pk=object_tag.pk).exists() + + +def test_object_tag_delete_with_no_status_beneath_it_cascades_its_criterion_away( + user, competency_taxonomy: CompetencyTaxonomy, tag: Tag, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_objecttag` with no status rows beneath it succeeds and cascades its criterion + away, while a criterion on another object tag holds status. + """ + criterion_with_status = CompetencyCriterion.objects.create( + group=criterion.group, + object_tag=ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+p4", taxonomy=competency_taxonomy, tag=tag, + ), + rule_profile=criterion.rule_profile, + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion_with_status, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + criterion.object_tag.delete() + + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion_with_status.pk).exists() + + +def test_root_group_delete_is_protected_by_a_criterion_status_two_levels_down( + user, group: CompetencyCriteriaGroup, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + The transitive cases are tested, not only the direct ones: deleting a root `CompetencyCriteriaGroup` + with a learner status row two levels beneath it raises `ProtectedError` and removes nothing. + """ + child_group = CompetencyCriteriaGroup.objects.create(tag=group.tag, parent=group) + criterion.group = child_group + criterion.save() + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + group.delete() + + assert CompetencyCriteriaGroup.objects.filter(pk__in=[group.pk, child_group.pk]).count() == 2 + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_tag_delete_is_protected_by_a_criterion_status( + user, tag: Tag, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_tag` with a learner status row anywhere beneath it raises `ProtectedError`. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + tag.delete() + + assert Tag.objects.filter(pk=tag.pk).exists() + + +def test_tag_delete_with_no_status_beneath_it_cascades_the_whole_criteria_tree_away( + user, competency_taxonomy: CompetencyTaxonomy, tag: Tag, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_tag` with no status rows beneath it succeeds and cascades the whole criteria + tree away, while another tag's tree holds status. + """ + other_tag = Tag.objects.create(taxonomy=competency_taxonomy, value="Decimals") + other_criterion = CompetencyCriterion.objects.create( + group=CompetencyCriteriaGroup.objects.create(tag=other_tag), + object_tag=ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+p3", taxonomy=competency_taxonomy, tag=other_tag, + ), + rule_profile=criterion.rule_profile, + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=other_criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + tag.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=criterion.group_id).exists() + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + assert CompetencyCriterion.objects.filter(pk=other_criterion.pk).exists() + + +def test_tagging_api_parent_tag_delete_is_protected_by_a_criterion_status_under_a_subtag( + user, + competency_taxonomy: CompetencyTaxonomy, + tag: Tag, + default_rule_profile: CompetencyRuleProfile, + now: datetime, +) -> None: + """ + Deleting an `oel_tagging_tag` with a learner status row anywhere beneath it raises `ProtectedError`, + including a row beneath one of its subtags, deleted through `openedx_tagging.api.delete_tags_from_taxonomy()`. + """ + subtag = Tag.objects.create(taxonomy=competency_taxonomy, parent=tag, value="Sonnets") + subtag_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+p2", taxonomy=competency_taxonomy, tag=subtag, + ) + subtag_group = CompetencyCriteriaGroup.objects.create(tag=subtag) + subtag_criterion = CompetencyCriterion.objects.create( + group=subtag_group, object_tag=subtag_object_tag, rule_profile=default_rule_profile + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=subtag_criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + count_before = Tag.objects.filter(pk__in=[tag.pk, subtag.pk]).count() + + with pytest.raises(ProtectedError), transaction.atomic(): + tagging_api.delete_tags_from_taxonomy(competency_taxonomy, [tag.value], with_subtags=True) + + assert count_before == 2 + assert Tag.objects.filter(pk__in=[tag.pk, subtag.pk]).count() == count_before + + +def test_taxonomy_delete_is_protected_by_a_criterion_status( + user, competency_taxonomy: CompetencyTaxonomy, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + Deleting an `oel_tagging_taxonomy` raises `ProtectedError` when a status row exists beneath any of its + tags, here deleted through its `CompetencyTaxonomy` row. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + competency_taxonomy.delete() + + assert CompetencyTaxonomy.objects.filter(pk=competency_taxonomy.pk).exists() + + +def test_course_run_delete_is_protected_by_a_criterion_status_in_its_course_scoped_group( + user, group: CompetencyCriteriaGroup, course_run: CourseRun, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + The transitive cases are tested, not only the direct ones: deleting a `CourseRun` whose course-scoped + `CompetencyCriteriaGroup` holds a criterion with a learner status row raises `ProtectedError`. + """ + course_group = CompetencyCriteriaGroup.objects.create(tag=group.tag, parent=group, course=course_run) + criterion.group = course_group + criterion.save() + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + course_run.delete() + + assert CourseRun.objects.filter(pk=course_run.pk).exists() + + +def test_deleting_a_branch_with_no_status_succeeds_while_a_sibling_branch_holds_status( + user, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, + now: datetime, +) -> None: + """ + Deleting a `CompetencyCriteriaGroup` at depth with no status rows beneath it succeeds and cascades its + criteria away, while a sibling branch of the same tree holds status. + """ + branch_with_status = CompetencyCriteriaGroup.objects.create(tag=group.tag, parent=group) + branch_without_status = CompetencyCriteriaGroup.objects.create(tag=group.tag, parent=group) + criterion_with_status = CompetencyCriterion.objects.create( + group=branch_with_status, object_tag=object_tag, rule_profile=default_rule_profile + ) + criterion_without_status = CompetencyCriterion.objects.create( + group=branch_without_status, object_tag=object_tag, rule_profile=default_rule_profile + ) + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion_with_status, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + branch_without_status.delete() + + assert not CompetencyCriterion.objects.filter(pk=criterion_without_status.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion_with_status.pk).exists() + + +def test_mastery_status_row_delete_is_protected_by_a_criterion_status( + user, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + The `status_id` foreign key to `CompetencyMasteryStatuses` is `PROTECT`. + """ + StudentCompetencyCriterionStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + CompetencyMasteryStatus.objects.get(pk=MasteryStatus.DEMONSTRATED).delete() + + assert CompetencyMasteryStatus.objects.filter(pk=MasteryStatus.DEMONSTRATED).exists() + + +def test_user_delete_removes_only_that_users_criterion_status( + user, criterion: CompetencyCriterion, now: datetime +) -> None: + """ + The `user_id` foreign key on `StudentCompetencyCriterionStatus` is `CASCADE`: deleting a user row + removes that user's status rows, and only that user's. + """ + other_user = get_user_model().objects.create(username="other_learner") + for learner in (user, other_user): + StudentCompetencyCriterionStatus.objects.create( + user=learner, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + user_id = user.pk + + user.delete() + + assert not StudentCompetencyCriterionStatus.objects.filter(user_id=user_id).exists() + assert StudentCompetencyCriterionStatus.objects.filter(user=other_user).exists() diff --git a/tests/openedx_learning/applets/cbe/test_learner_status.py b/tests/openedx_learning/applets/cbe/test_learner_status.py index 64364344b..0720da1fe 100644 --- a/tests/openedx_learning/applets/cbe/test_learner_status.py +++ b/tests/openedx_learning/applets/cbe/test_learner_status.py @@ -10,7 +10,6 @@ from datetime import datetime, timezone import pytest -from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.db import IntegrityError, transaction from django.db.models import ProtectedError @@ -21,41 +20,12 @@ pytestmark = pytest.mark.django_db -@pytest.fixture(name="competency_taxonomy") -def _competency_taxonomy() -> CompetencyTaxonomy: - """A CompetencyTaxonomy for use as a scope, and as the home taxonomy for `tag`.""" - return CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") - - -@pytest.fixture(name="tag") -def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: - """A Tag, from `competency_taxonomy`, for use as the competency a learner is assessed on.""" - return Tag.objects.create(taxonomy=competency_taxonomy, value="Writing Poetry") - - @pytest.fixture(name="other_tag") def _other_tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: """A second Tag, from the same taxonomy as `tag`, for a second competency-level status.""" return Tag.objects.create(taxonomy=competency_taxonomy, value="Decimals") -@pytest.fixture(name="user") -def _user(): - """ - Create a single learner for use in these tests. - - Deliberately unannotated: the user model is swappable, so this library must not - name a concrete one (edx-lint enforces that as `imported-auth-user`). - """ - return get_user_model().objects.create(username="learner") - - -@pytest.fixture(name="now") -def _now() -> datetime: - """A single UTC timestamp shared by writes in a test.""" - return datetime.now(timezone.utc) - - # ============================================================================================== # The lookup table and its rank ordering. # ==============================================================================================