diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 2d5847b8f..662feb441 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -79,6 +79,8 @@ openedx_content.UnitVersion: ".. no_pii:": "This model has no PII" openedx_learning.HistoricalCompetencyCriteriaGroup: ".. no_pii:": "This model has no PII" +openedx_learning.HistoricalCompetencyRuleProfile: + ".. no_pii:": "This model has no PII" social_django.Association: ".. no_pii:": "This model has no PII" social_django.Code: diff --git a/src/openedx_learning/applets/cbe/models/__init__.py b/src/openedx_learning/applets/cbe/models/__init__.py index fe57c998e..6775df566 100644 --- a/src/openedx_learning/applets/cbe/models/__init__.py +++ b/src/openedx_learning/applets/cbe/models/__init__.py @@ -2,11 +2,14 @@ Models for Competency-Based Education (CBE). """ +from ..rule_payloads import RuleType from .competency_taxonomy import CompetencyTaxonomy -from .criteria import CompetencyCriteriaGroup, LogicOperator +from .criteria import CompetencyCriteriaGroup, CompetencyRuleProfile, LogicOperator __all__ = [ "CompetencyCriteriaGroup", + "CompetencyRuleProfile", "CompetencyTaxonomy", "LogicOperator", + "RuleType", ] diff --git a/src/openedx_learning/applets/cbe/models/criteria.py b/src/openedx_learning/applets/cbe/models/criteria.py index f06adb680..cfad99f39 100644 --- a/src/openedx_learning/applets/cbe/models/criteria.py +++ b/src/openedx_learning/applets/cbe/models/criteria.py @@ -1,22 +1,30 @@ """ -The CompetencyAchievementCriteria tree: CompetencyCriteriaGroup, the internal AND/OR node. +The CompetencyAchievementCriteria models: CompetencyCriteriaGroup, the internal AND/OR node, +and CompetencyRuleProfile, the reusable evaluation rule its leaves draw from. -See :ref:`openedx-learning-adr-0002` Decision 2 for the design and Decision 7 for why every -foreign key here cascades, and :ref:`openedx-learning-adr-0003` Decisions 1 and 2 for why this -model carries ``django-simple-history`` tracking and CompetencyTaxonomy does not. +See :ref:`openedx-learning-adr-0002` Decisions 2 and 3 for the design and Decision 7 for each +foreign key's delete behavior, and :ref:`openedx-learning-adr-0003` Decisions 1 and 2 for why +these models carry ``django-simple-history`` tracking and CompetencyTaxonomy does not. """ from __future__ import annotations +from django.core.exceptions import ValidationError from django.db import models +from django.db.models import Q from django.utils.translation import gettext_lazy as _ +from organizations.models import Organization from simple_history.models import HistoricalRecords from openedx_catalog.models import CourseRun from openedx_django_lib.fields import case_insensitive_char_field, immutable_uuid_field from openedx_tagging.models import Tag +from ..rule_payloads import RuleType, validate_rule_payload +from .competency_taxonomy import CompetencyTaxonomy + __all__ = [ "CompetencyCriteriaGroup", + "CompetencyRuleProfile", "LogicOperator", ] @@ -98,3 +106,156 @@ class Meta: # indexes every ForeignKey column by default, so a second explicit one here would only # cost write throughput without adding any read benefit. ] + + +class CompetencyRuleProfile(models.Model): + """ + A reusable default evaluation rule, optionally scoped to a taxonomy, course, or organization. + + Each row is scoped by at most one of ``organization``, ``course``, and ``competency_taxonomy``, + enforced by the check constraint below; the row with all three null is the system default, + seeded once by migration and never created or deleted through the profile API. See ADR-0002 + Decision 3 for how a :class:`CompetencyCriterion` is assigned one of these, and Decision 4 for + what happens when more than one scope's profile could apply to the same criterion. + + A profile's scope is immutable after creation; only ``rule_type``, ``rule_payload`` and + ``archived`` may change. + + .. no_pii: + """ + + uuid = immutable_uuid_field() + organization = models.ForeignKey( + Organization, + null=True, + blank=True, + on_delete=models.PROTECT, + related_name="competency_rule_profiles", + help_text=_("The organization this profile is scoped to, if any."), + ) + course = models.ForeignKey( + CourseRun, + null=True, + blank=True, + on_delete=models.CASCADE, + related_name="competency_rule_profiles", + help_text=_("The course run this profile is scoped to, if any."), + ) + competency_taxonomy = models.ForeignKey( + CompetencyTaxonomy, + null=True, + blank=True, + on_delete=models.CASCADE, + related_name="rule_profiles", + help_text=_("The competency taxonomy this profile is scoped to, if any."), + ) + # Recomputed in save(), never set directly: null while archived, so any number of archived + # rows may share a scope while exactly one live row holds it, which is what lets an archived + # profile be replaced. See ADR-0002 Decision 3. + scope_code = models.CharField( + max_length=255, + null=True, + editable=False, + help_text=_( + "Derived from organization/course/competency_taxonomy; null while archived, otherwise " + "\"org:X,course:Y,taxonomy:Z\" with each segment blank when that scope column is null." + ), + ) + rule_type = models.CharField(max_length=32, choices=RuleType) + rule_payload = models.JSONField( + help_text=_( + 'Structured payload whose keys are set by rule_type. A "Grade" payload is ' + '{"op": "gte" | "lte" | "eq", "value": a fraction from 0.0 to 1.0, "scale": "percent"}.' + ) + ) + archived = models.BooleanField( + default=False, + help_text=_( + "Hides a profile from authoring and from new associations while keeping it queryable, so " + "criteria already assigned to it stay resolvable." + ), + ) + + # scope_code is excluded from history: it is a derived, non-editable bookkeeping column (see + # above), not an author-facing fact worth its own historical row -- the columns it derives + # from (organization, course, competency_taxonomy, archived) are already tracked, and are what + # an audit trail actually needs. + history = HistoricalRecords(excluded_fields=["scope_code"]) + + class Meta: + constraints = [ + # Unconditional, over the derived scope_code column rather than the raw nullable + # scope columns: MySQL has no partial unique indexes and Django silently skips + # creating one there. See ADR-0002 Rejected Alternative 6. + models.UniqueConstraint(fields=["scope_code"], name="oel_cbe_ruleprofile_scope_code_uniq"), + models.CheckConstraint( + # Expressed as "at least two of the three scope columns are null", i.e. at most one + # is non-null. + condition=( + Q(organization__isnull=True, course__isnull=True) + | Q(organization__isnull=True, competency_taxonomy__isnull=True) + | Q(course__isnull=True, competency_taxonomy__isnull=True) + ), + name="oel_cbe_ruleprofile_scope_check", + violation_error_message=_( + "A CompetencyRuleProfile may be scoped to at most one of organization, course, and " + "competency_taxonomy." + ), + ), + models.CheckConstraint( + # Keeps scope_code's invariant honest against QuerySet.update(), which bypasses + # save(): the database refuses the row rather than letting this get out of sync + # behind save()'s back. + condition=( + Q(archived=True, scope_code__isnull=True) | Q(archived=False, scope_code__isnull=False) + ), + name="oel_cbe_ruleprofile_archived_scope_code_check", + violation_error_message=_( + "An archived CompetencyRuleProfile must have a null scope_code; a live one must not." + ), + ), + ] + + def _check_scope_immutable(self) -> None: + """Raise ValidationError if the scope columns no longer match what is persisted for this row.""" + if self.pk is None: + # A new, unsaved instance: there's no persisted scope yet to compare against. + return + # Queried rather than compared against a value cached at load time, so a deferred load or + # a refresh_from_db() cannot bypass the check. + persisted_scope = ( + CompetencyRuleProfile.objects.filter(pk=self.pk) + .values_list("organization_id", "course_id", "competency_taxonomy_id") + .first() + ) + if persisted_scope is None: + return + current_scope = (self.organization_id, self.course_id, self.competency_taxonomy_id) + if current_scope != persisted_scope: + raise ValidationError( + _( + "A CompetencyRuleProfile's scope (organization, course, competency_taxonomy) cannot be " + "changed after creation." + ) + ) + + def clean(self): + """Validate scope immutability and the rule_payload shape for rule_type.""" + super().clean() + self._check_scope_immutable() + validate_rule_payload(self.rule_type, self.rule_payload) + + def _compute_scope_code(self) -> str | None: + """Return this profile's scope_code, or None while it is archived.""" + if self.archived: + return None + # A blank segment, not "None", for an unset scope: ADR-0002 Decision 3 fixes this format. + org, course, taxonomy = self.organization_id, self.course_id, self.competency_taxonomy_id + return f"org:{org or ''},course:{course or ''},taxonomy:{taxonomy or ''}" + + def save(self, *args, **kwargs): + """On save: recompute and validate scope_code.""" + self.scope_code = self._compute_scope_code() + # validate_unique() is already enforced by the database. + self.full_clean(validate_unique=False, validate_constraints=False) + super().save(*args, **kwargs) diff --git a/src/openedx_learning/migrations/0004_competencyruleprofile.py b/src/openedx_learning/migrations/0004_competencyruleprofile.py new file mode 100644 index 000000000..61dde047f --- /dev/null +++ b/src/openedx_learning/migrations/0004_competencyruleprofile.py @@ -0,0 +1,63 @@ +# Generated by Django 5.2.16 on 2026-09-10 18:33 + +import uuid + +import django.db.models.deletion +import simple_history.models +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_catalog', '0001_initial'), + ('openedx_learning', '0003_competencycriteriagroup'), + ('organizations', '0005_competencyruleprofile'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalCompetencyRuleProfile', + fields=[ + ('id', models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('uuid', models.UUIDField(db_index=True, default=uuid.uuid4, editable=False, verbose_name='UUID')), + ('rule_type', models.CharField(choices=[('Grade', 'Grade')], max_length=32)), + ('rule_payload', models.JSONField(help_text='Structured payload whose keys are set by rule_type. A "Grade" payload is {"op": "gte" | "lte" | "eq", "value": a fraction from 0.0 to 1.0, "scale": "percent"}.')), + ('archived', models.BooleanField(default=False, help_text='Hides a profile from authoring and from new associations while keeping it queryable, so criteria already assigned to it stay resolvable.')), + ('history_id', models.AutoField(primary_key=True, serialize=False)), + ('history_date', models.DateTimeField(db_index=True)), + ('history_change_reason', models.CharField(max_length=100, null=True)), + ('history_type', models.CharField(choices=[('+', 'Created'), ('~', 'Changed'), ('-', 'Deleted')], max_length=1)), + ('competency_taxonomy', models.ForeignKey(blank=True, db_constraint=False, help_text='The competency taxonomy this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_learning.competencytaxonomy')), + ('course', models.ForeignKey(blank=True, db_constraint=False, help_text='The course run this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_catalog.courserun')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('organization', models.ForeignKey(blank=True, db_constraint=False, help_text='The organization this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='organizations.organization')), + ], + options={ + 'verbose_name': 'historical competency rule profile', + 'verbose_name_plural': 'historical competency rule profiles', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='CompetencyRuleProfile', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, editable=False, unique=True, verbose_name='UUID')), + ('scope_code', models.CharField(editable=False, help_text='Derived from organization/course/competency_taxonomy; null while archived, otherwise "org:X,course:Y,taxonomy:Z" with each segment blank when that scope column is null.', max_length=255, null=True)), + ('rule_type', models.CharField(choices=[('Grade', 'Grade')], max_length=32)), + ('rule_payload', models.JSONField(help_text='Structured payload whose keys are set by rule_type. A "Grade" payload is {"op": "gte" | "lte" | "eq", "value": a fraction from 0.0 to 1.0, "scale": "percent"}.')), + ('archived', models.BooleanField(default=False, help_text='Hides a profile from authoring and from new associations while keeping it queryable, so criteria already assigned to it stay resolvable.')), + ('competency_taxonomy', models.ForeignKey(blank=True, help_text='The competency taxonomy this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='rule_profiles', to='openedx_learning.competencytaxonomy')), + ('course', models.ForeignKey(blank=True, help_text='The course run this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='competency_rule_profiles', to='openedx_catalog.courserun')), + ('organization', models.ForeignKey(blank=True, help_text='The organization this profile is scoped to, if any.', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='competency_rule_profiles', to='organizations.organization')), + ], + options={ + 'constraints': [models.UniqueConstraint(fields=('scope_code',), name='oel_cbe_ruleprofile_scope_code_uniq'), models.CheckConstraint(condition=models.Q(models.Q(('course__isnull', True), ('organization__isnull', True)), models.Q(('competency_taxonomy__isnull', True), ('organization__isnull', True)), models.Q(('competency_taxonomy__isnull', True), ('course__isnull', True)), _connector='OR'), name='oel_cbe_ruleprofile_scope_check', violation_error_message='A CompetencyRuleProfile may be scoped to at most one of organization, course, and competency_taxonomy.'), models.CheckConstraint(condition=models.Q(models.Q(('archived', True), ('scope_code__isnull', True)), models.Q(('archived', False), ('scope_code__isnull', False)), _connector='OR'), name='oel_cbe_ruleprofile_archived_scope_code_check', violation_error_message='An archived CompetencyRuleProfile must have a null scope_code; a live one must not.')], + }, + ), + ] diff --git a/src/openedx_learning/migrations/0005_seed_default_rule_profile.py b/src/openedx_learning/migrations/0005_seed_default_rule_profile.py new file mode 100644 index 000000000..3414b015e --- /dev/null +++ b/src/openedx_learning/migrations/0005_seed_default_rule_profile.py @@ -0,0 +1,49 @@ +""" +Seed the system-default CompetencyRuleProfile: the one row where every scope column is null. + +Per ADR-0002 Decision 3, this is the rule every CompetencyCriterion falls back to when nothing +more specific applies, so a deployment that adds no profiles of its own still gets an 80% +threshold. +""" +from django.db import migrations + +# Fixed rather than uuid.uuid4(), so this shared system-default row has the same external +# identifier in every deployment, not a fresh random one each time this migration runs. +_DEFAULT_RULE_PROFILE_UUID = "5b3e8f5c-3b0e-4b1a-9b1e-6b6b6b6b6b6b" + + +def seed_default_rule_profile(apps, schema_editor): + """Create the all-null-scope CompetencyRuleProfile.""" + CompetencyRuleProfile = apps.get_model("openedx_learning", "CompetencyRuleProfile") + CompetencyRuleProfile.objects.create( + uuid=_DEFAULT_RULE_PROFILE_UUID, + rule_type="Grade", + rule_payload={"op": "gte", "value": 0.8, "scale": "percent"}, + archived=False, + # apps.get_model() returns a historical model reconstructed from migration state, which + # does not carry CompetencyRuleProfile's custom save() (and so never computes this). + # organization_id/course_id/competency_taxonomy_id are all null for this row, so every + # segment of the "org:X,course:Y,taxonomy:Z" format is blank. + scope_code="org:,course:,taxonomy:", + ) + + +def remove_default_rule_profile(apps, schema_editor): + """Delete the all-null-scope CompetencyRuleProfile, reversing seed_default_rule_profile.""" + CompetencyRuleProfile = apps.get_model("openedx_learning", "CompetencyRuleProfile") + CompetencyRuleProfile.objects.filter( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + ).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ("openedx_learning", "0004_competencyruleprofile"), + ] + + operations = [ + migrations.RunPython(seed_default_rule_profile, remove_default_rule_profile), + ] diff --git a/tests/openedx_learning/applets/cbe/conftest.py b/tests/openedx_learning/applets/cbe/conftest.py index 60fefbc79..fb25fd213 100644 --- a/tests/openedx_learning/applets/cbe/conftest.py +++ b/tests/openedx_learning/applets/cbe/conftest.py @@ -4,7 +4,7 @@ from organizations.models import Organization from openedx_catalog.models import CatalogCourse, CourseRun -from openedx_learning.models import CompetencyCriteriaGroup, CompetencyTaxonomy +from openedx_learning.models import CompetencyCriteriaGroup, CompetencyRuleProfile, CompetencyTaxonomy from openedx_tagging.models import Tag @@ -15,6 +15,13 @@ def _organization() -> Organization: return Organization.objects.get(short_name="Org1") +@pytest.fixture(name="organization2") +def _organization2() -> Organization: + """A second Organization, distinct from `organization`, for use as a scope in these tests.""" + ensure_organization("Org2") + return Organization.objects.get(short_name="Org2") + + @pytest.fixture(name="course_run") def _course_run(organization: Organization) -> CourseRun: """A CourseRun for use as a scope in these tests.""" @@ -38,3 +45,13 @@ def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: def _group(tag: Tag) -> CompetencyCriteriaGroup: """A root CompetencyCriteriaGroup for `tag`, for use as a criterion's parent group.""" return CompetencyCriteriaGroup.objects.create(tag=tag) + + +@pytest.fixture(name="default_rule_profile") +def _default_rule_profile() -> CompetencyRuleProfile: + """The system-default CompetencyRuleProfile seeded by migration 0005.""" + return CompetencyRuleProfile.objects.get( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + ) diff --git a/tests/openedx_learning/applets/cbe/test_rule_profile.py b/tests/openedx_learning/applets/cbe/test_rule_profile.py new file mode 100644 index 000000000..13c461323 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rule_profile.py @@ -0,0 +1,466 @@ +""" +Tests for CompetencyRuleProfile, the reusable evaluation rule a CompetencyCriterion draws from. + +Each test name states the behavior it pins. Reading top to bottom gives the model's contract: +its columns, the at-most-one-scope rule, how scope_code encodes that scope and what archiving +does to it, that the payload validator is wired into save(), that a profile's scope can never +change after creation, and the index, history and seeded row. + +The payload shapes themselves are covered exhaustively and without a database in +test_rule_payloads.py. What matters here is only that a model save reaches that validator. + +Delete behavior is not covered here, except where a test frees the seeded system-default scope, +which nothing references. Nothing else in this module deletes a row that another row points at. +See test_rule_profile_deletion.py, in this same change, for this model's own `on_delete` values +and the tests that exercise them. + +Fixtures live in this directory's conftest.py. +""" +import pytest +from django.apps import apps +from django.core.exceptions import ValidationError +from django.db import connection, transaction +from django.db.utils import IntegrityError +from organizations.models import Organization + +from openedx_catalog.models import CatalogCourse, CourseRun +from openedx_learning.applets.cbe.rule_payloads import validate_rule_payload +from openedx_learning.models import CompetencyRuleProfile, CompetencyTaxonomy, RuleType + +pytestmark = pytest.mark.django_db + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + + +# --------------------------------------------------------------------------------------------- +# Schema + + +# --------------------------------------------------------------------------------------------- + + +def test_rule_profile_has_exactly_the_columns_adr_0002_decision_3_lists() -> None: + """ + CompetencyRuleProfile's columns are exactly the ones ADR-0002 Decision 3 lists, with + `organization`, `course`, `competency_taxonomy`, and `scope_code` nullable and the rest + required. `scope_code` is nullable, not "never null": it is null exactly while a profile is + archived, which is what frees that scope's unique slot for a replacement. See ADR-0002 + Decision 3. + """ + fields = [f for f in CompetencyRuleProfile._meta.get_fields() if f.concrete] + assert {f.name for f in fields} == { + "id", "uuid", "organization", "course", "competency_taxonomy", "scope_code", "rule_type", + "rule_payload", "archived", + } + assert {f.name for f in fields if f.null} == {"organization", "course", "competency_taxonomy", "scope_code"} + assert CompetencyRuleProfile._meta.get_field("organization").remote_field.model is Organization + assert CompetencyRuleProfile._meta.get_field("course").remote_field.model is CourseRun + assert CompetencyRuleProfile._meta.get_field("competency_taxonomy").remote_field.model is CompetencyTaxonomy + + +# --------------------------------------------------------------------------------------------- +# Scope: at most one of organization, course, competency_taxonomy + + +# --------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "scope_kwargs", + [ + pytest.param({"organization": True}, id="organization_only"), + pytest.param({"course": True}, id="course_only"), + pytest.param({"competency_taxonomy": True}, id="competency_taxonomy_only"), + pytest.param({}, id="no_scope_system_default"), + ], +) +def test_rule_profile_scope_check_constraint_accepts_at_most_one_scope_field( + scope_kwargs: dict, + organization: Organization, + course_run: CourseRun, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + The scope check constraint accepts a CompetencyRuleProfile scoped to at most one of + organization, course, or competency_taxonomy, including none of them (the system default). + See ADR-0002 Decision 3. + """ + # Free the all-null slot the seed migration (0003) occupies, so the "no scope" case can be + # tested in isolation from scope_code's own uniqueness constraint, which has its own tests. + CompetencyRuleProfile.objects.filter( + organization__isnull=True, course__isnull=True, competency_taxonomy__isnull=True + ).delete() + + resolved_kwargs: dict[str, object] = {} + if scope_kwargs.get("organization"): + resolved_kwargs["organization"] = organization + if scope_kwargs.get("course"): + resolved_kwargs["course"] = course_run + if scope_kwargs.get("competency_taxonomy"): + resolved_kwargs["competency_taxonomy"] = competency_taxonomy + + profile = CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD, **resolved_kwargs + ) + assert profile.pk is not None + + +@pytest.mark.parametrize( + "scoped_fields", + [ + pytest.param(("organization", "course"), id="organization_and_course"), + pytest.param(("organization", "competency_taxonomy"), id="organization_and_taxonomy"), + pytest.param(("course", "competency_taxonomy"), id="course_and_taxonomy"), + pytest.param(("organization", "course", "competency_taxonomy"), id="all_three"), + ], +) +def test_rule_profile_scope_check_constraint_rejects_more_than_one_scope_field( + scoped_fields: tuple[str, ...], + organization: Organization, + course_run: CourseRun, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + The scope check constraint rejects a CompetencyRuleProfile scoped to any two of organization, + course, and competency_taxonomy, or to all three. See ADR-0002 Decision 3. + """ + available_values = {"organization": organization, "course": course_run, "competency_taxonomy": competency_taxonomy} + scope_kwargs = {field_name: available_values[field_name] for field_name in scoped_fields} + + with pytest.raises(IntegrityError): + with transaction.atomic(): + CompetencyRuleProfile.objects.create(rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD, **scope_kwargs) + + +# --------------------------------------------------------------------------------------------- +# scope_code: how a scope is encoded, and what archiving does to it +# ADR-0002 Decision 3. scope_code is a plain column recomputed in save(), and it goes null +# while a profile is archived. SQL never treats two NULLs as equal, so any number of archived +# rows may share a scope while exactly one live row holds it, which is what lets an archived +# profile be replaced. + + +# --------------------------------------------------------------------------------------------- + + +def test_scope_code_matches_org_course_taxonomy_format_for_each_scope_shape( + organization: Organization, course_run: CourseRun, competency_taxonomy: CompetencyTaxonomy +) -> None: + """ + A live (non-archived) profile's scope_code is "org:X,course:Y,taxonomy:Z", with each segment + left blank when the corresponding scope column is null. See ADR-0002 Decision 3. + """ + CompetencyRuleProfile.objects.filter( + organization__isnull=True, course__isnull=True, competency_taxonomy__isnull=True + ).delete() + + all_null = CompetencyRuleProfile.objects.create(rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD) + org_only = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + course_only = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + taxonomy_only = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + for profile in (all_null, org_only, course_only, taxonomy_only): + profile.refresh_from_db() + + assert all_null.scope_code == "org:,course:,taxonomy:" + assert org_only.scope_code == f"org:{organization.pk},course:,taxonomy:" + assert course_only.scope_code == f"org:,course:{course_run.pk},taxonomy:" + assert taxonomy_only.scope_code == f"org:,course:,taxonomy:{competency_taxonomy.pk}" + + +def test_scope_code_is_null_once_archived_and_non_null_while_live(organization: Organization) -> None: + """ + scope_code is non-null while a profile is live, and becomes null once it is archived. An + archived profile no longer holds its scope's unique slot, which is what lets a replacement be + created for that same scope (see test_archiving_a_profile_frees_its_scope_for_a_replacement + below); a profile that stayed occupying a non-null scope_code after archiving would block that + forever. This is a deliberate design point, not an oversight: a plain nullable column, written + explicitly whenever a profile is saved, rather than a database-computed value that can never + tell "archived" apart from "live" on its own. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.refresh_from_db() + assert profile.scope_code == f"org:{organization.pk},course:,taxonomy:" + + profile.archived = True + profile.save() + profile.refresh_from_db() + assert profile.scope_code is None + + +def test_archiving_a_profile_frees_its_scope_for_a_replacement(organization: Organization) -> None: + """ + Once a profile scoped to a given organization/course/taxonomy is archived, a brand new profile + may be created for that exact same scope: the archived row's scope_code goes to null and stops + occupying the unique slot, so it no longer collides with the replacement's non-null scope_code. + """ + original = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + original.archived = True + original.save() + + replacement = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + replacement.refresh_from_db() + original.refresh_from_db() + + assert original.scope_code is None + assert replacement.scope_code == f"org:{organization.pk},course:,taxonomy:" + + +def test_two_live_profiles_cannot_share_the_same_scope(organization: Organization) -> None: + """ + Two live CompetencyRuleProfile rows cannot share the same scope. In particular, two rows that + both set only `organization` (leaving course and competency_taxonomy null) collide, which is + exactly the case a plain UniqueConstraint on the three raw nullable columns would not catch, + since SQL never treats two NULLs as equal. See ADR-0002 Decision 3. + """ + CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + with pytest.raises(IntegrityError): + with transaction.atomic(): + CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + +# --------------------------------------------------------------------------------------------- +# Payload validation is wired into save() + + +# --------------------------------------------------------------------------------------------- + + +def test_saving_a_profile_with_an_invalid_payload_raises_validation_error() -> None: + """ + A profile whose rule_payload does not match its rule_type is rejected by full_clean(), which + save() calls, so objects.create() raises rather than writing a rule nothing can evaluate. + + This proves only the wiring. test_rule_payloads.py covers every way a payload can be wrong. + """ + with pytest.raises(ValidationError): + CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload={"op": "gte", "value": 80, "scale": "percent"} + ) + + +def test_rule_profile_full_clean_value_message_names_the_fraction_convention(organization: Organization) -> None: + """ + full_clean()'s error for a rule_payload 'value' given on a 0-100 scale (e.g. 80) names the + 0.0-1.0 fraction convention. Every other invalid-payload test here only asserts the exception + type; this one asserts the message content. + """ + profile = CompetencyRuleProfile( + organization=organization, rule_type=RuleType.GRADE, rule_payload={"op": "gte", "value": 80, "scale": "percent"} + ) + with pytest.raises(ValidationError) as exc_info: + profile.full_clean() + + message = " ".join(exc_info.value.messages) + assert "fraction between 0.0 and 1.0" in message + + +def test_rule_profile_full_clean_extra_key_message_names_the_key(organization: Organization) -> None: + """ + full_clean()'s error for an unrecognized rule_payload key names that key in our own domain + language (e.g. "unexpected extra"). + """ + profile = CompetencyRuleProfile( + organization=organization, + rule_type=RuleType.GRADE, + rule_payload={**_GRADE_PAYLOAD, "extra": 1}, + ) + with pytest.raises(ValidationError) as exc_info: + profile.full_clean() + + message = " ".join(exc_info.value.messages) + assert "extra" in message + + +# --------------------------------------------------------------------------------------------- +# Scope immutability +# Editing a profile may change rule_type, rule_payload and archived only. Its scope is fixed +# at creation, so criteria already resolved to that scope are never silently re-governed. + + +# --------------------------------------------------------------------------------------------- + + +def test_scope_immutability_rejects_organization_change( + organization: Organization, organization2: Organization +) -> None: + """ + Changing a CompetencyRuleProfile's `organization` after creation raises ValidationError on + save(). See ADR-0002 Decision 3. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.organization = organization2 + with pytest.raises(ValidationError): + profile.save() + + +def test_scope_immutability_rejects_course_change(organization: Organization, course_run: CourseRun) -> None: + """ + Changing a CompetencyRuleProfile's `course` after creation raises ValidationError on save(). + See ADR-0002 Decision 3. + """ + other_catalog_course = CatalogCourse.objects.create(org=organization, course_code="Python200") + other_course_run = CourseRun.objects.create(catalog_course=other_catalog_course, run_code="Spring2027") + + profile = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.course = other_course_run + with pytest.raises(ValidationError): + profile.save() + + +def test_scope_immutability_rejects_taxonomy_change(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + Changing a CompetencyRuleProfile's `competency_taxonomy` after creation raises ValidationError + on save(). See ADR-0002 Decision 3. + """ + other_taxonomy = CompetencyTaxonomy.objects.create(name="Welding", export_id="welding-v1") + + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.competency_taxonomy = other_taxonomy + with pytest.raises(ValidationError): + profile.save() + + +def test_scope_immutability_allows_rule_type_rule_payload_and_archived_to_change(organization: Organization) -> None: + """ + Only rule_type, rule_payload, and archived may change after creation; changing any of them (as + opposed to a scope field) succeeds. See ADR-0002 Decision 3. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.rule_type = RuleType.GRADE + profile.rule_payload = {"op": "lte", "value": 0.5, "scale": "percent"} + profile.archived = True + profile.save() + + profile.refresh_from_db() + assert profile.rule_payload == {"op": "lte", "value": 0.5, "scale": "percent"} + assert profile.archived is True + + +def test_scope_immutability_enforced_after_deferred_load( + organization: Organization, organization2: Organization +) -> None: + """ + Scope immutability is enforced even when the profile was loaded with .only()/.defer() and so + never loaded the scope columns into this instance in the first place. + _check_scope_immutable() always queries the persisted scope directly (see its docstring), so a + partial load is not a way to bypass this check. + + Uses a second organization rather than setting the scope to None: a null scope would collide + with the seeded system-default row, so the unique constraint would raise IntegrityError and + the scope guard would never be reached. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + deferred = CompetencyRuleProfile.objects.only("id", "rule_type").get(pk=profile.pk) + + deferred.organization = organization2 + with pytest.raises(ValidationError): + deferred.save() + + +# --------------------------------------------------------------------------------------------- +# Index 9, history, and the seeded system default + + +# --------------------------------------------------------------------------------------------- + + +def test_the_database_carries_adr_0002_decision_5_index_9_as_unique() -> None: + """ + The real table carries ADR-0002 Decision 5's index 9 on scope_code, and it is unique. A plain + index there would not enforce one profile per scope. + + The constraint is unconditional on purpose. A conditional UniqueConstraint compiles to a + partial index, which MySQL does not support: Django raises only a models.W036 warning and + silently skips creating it, while SQLite does support partial indexes and would hide the gap + in a local run. See ADR-0002 Rejected Alternative 6. + """ + with connection.cursor() as cursor: + constraints = connection.introspection.get_constraints( + cursor, CompetencyRuleProfile._meta.db_table + ) + + assert any(set(c["columns"]) == {"scope_code"} and c["unique"] for c in constraints.values()) + + +def test_editing_a_profile_writes_a_historical_row(organization: Organization) -> None: + """ + HistoricalRecords() is applied to CompetencyRuleProfile: creating then editing a profile + leaves two rows in the Historical model. See ADR-0003 Decision 1. + """ + historical_profile = apps.get_model("openedx_learning", "HistoricalCompetencyRuleProfile") + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + profile.rule_payload = {"op": "gte", "value": 0.9, "scale": "percent"} + profile.save() + + assert historical_profile.objects.filter(id=profile.pk).count() == 2 + + +def test_scope_code_is_excluded_from_history() -> None: + """ + The Historical model does not track scope_code. It is a derived bookkeeping column, and the + columns it derives from (the three scope fields and archived) are tracked instead, which is + what an audit trail actually needs. + """ + historical_profile = apps.get_model("openedx_learning", "HistoricalCompetencyRuleProfile") + + assert "scope_code" not in {f.name for f in historical_profile._meta.get_fields()} + + +def test_migration_seeds_exactly_one_system_default_rule_profile() -> None: + """ + Migration 0005 seeds exactly one system-default CompetencyRuleProfile: all three scope + columns null, not archived, Grade >= 0.8 (80%). See ADR-0002 Decision 3. + """ + profile = CompetencyRuleProfile.objects.get( + organization__isnull=True, course__isnull=True, competency_taxonomy__isnull=True + ) + assert profile.archived is False + assert profile.rule_type == RuleType.GRADE + assert profile.rule_payload == _GRADE_PAYLOAD + + +def test_the_seeded_rule_payload_satisfies_the_payload_contract() -> None: + """ + The seeded system-default row's rule_payload passes validate_rule_payload. + + 0005_seed_default_rule_profile writes that payload as a literal and cannot check it itself: a + historical migration must not import rule_payloads, because that module changes while the + migration must not, and apps.get_model() returns a model reconstructed without the custom + clean(). This test is therefore the only place the seeded literal and the validator meet. + Without it, tightening _validate_grade_payload would leave the default row that every + deployment ships with invalid, and no test would fail. + """ + profile = CompetencyRuleProfile.objects.get( + organization__isnull=True, course__isnull=True, competency_taxonomy__isnull=True + ) + + validate_rule_payload(profile.rule_type, profile.rule_payload) diff --git a/tests/openedx_learning/applets/cbe/test_rule_profile_deletion.py b/tests/openedx_learning/applets/cbe/test_rule_profile_deletion.py new file mode 100644 index 000000000..e252d9df9 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rule_profile_deletion.py @@ -0,0 +1,182 @@ +""" +Delete-behavior tests for CompetencyRuleProfile's own foreign keys. + +| Foreign key | Value | Why | +| CompetencyRuleProfile.organization | PROTECT | an Organization is not a competency record | +| CompetencyRuleProfile.course | CASCADE | a course-scoped profile goes with its run | +| CompetencyRuleProfile.competency_taxonomy | CASCADE | a taxonomy-scoped profile goes with its taxonomy | + +``on_delete`` expresses containment rather than protection (ADR-0002 Decision 7): it governs +deletion of the row a foreign key points *at*, never the row holding it. A CompetencyRuleProfile +is never hard-deleted by a *direct* delete of the profile itself; retirement is archive-only. +That does not stop it being cascaded away as a side effect of deleting the course or taxonomy it +is scoped to. + +Fixtures live in this directory's conftest.py. +""" +import pytest +from django.db import connection +from django.db.models import ProtectedError +from organizations.models import Organization + +from openedx_catalog.models import CourseRun +from openedx_learning.models import CompetencyRuleProfile, CompetencyTaxonomy, RuleType + +pytestmark = pytest.mark.django_db + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + + +# --------------------------------------------------------------------------------------------- +# A profile is never hard-deleted by a direct delete; retirement is an archive. That does not +# stop a profile being cascaded away with the course or taxonomy it is scoped to. The PROTECT +# test inspects the exception's collected objects rather than only catching the exception, +# because several such relationships can fire on one delete. + + +# --------------------------------------------------------------------------------------------- + + +def test_deleting_an_organization_with_a_scoped_profile_raises_protected_error_naming_the_profile( + organization2: Organization, +) -> None: + """ + Deleting an Organization that a CompetencyRuleProfile references via `organization` raises + ProtectedError naming the profile. + + Uses `organization2`, which this test never attaches a CatalogCourse to, instead of + `organization` (the one `course_run` uses elsewhere in this module): CatalogCourse.org is + itself PROTECT, so deleting an organization with a CatalogCourse attached raises + ProtectedError regardless of whether a CompetencyRuleProfile references it too, and this + test would pass for the wrong reason. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization2, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + with pytest.raises(ProtectedError) as exc_info: + organization2.delete() + + protected = exc_info.value.protected_objects + assert any(isinstance(obj, CompetencyRuleProfile) and obj.pk == profile.pk for obj in protected) + + +def test_deleting_a_course_run_with_a_scoped_rule_profile_also_deletes_the_profile( + course_run: CourseRun, +) -> None: + """ + Deleting a CourseRun cascades to any CompetencyRuleProfile scoped to it via `course`: the + delete succeeds and the profile row is gone too. + """ + profile = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + assert CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + course_run.delete() + + assert not CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + +def test_deleting_a_taxonomy_with_a_scoped_rule_profile_also_deletes_the_profile( + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + Deleting a CompetencyTaxonomy cascades to any CompetencyRuleProfile scoped to it via + `competency_taxonomy`: the delete succeeds and the profile row is gone too, as #641 + requires. Nothing changes behaviorally in this MVP, since only the all-null system-default + profile exists otherwise, so this scenario cannot arise until a taxonomy-scoped profile is + actually created, which no authoring screen does yet. + """ + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + assert CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + competency_taxonomy.delete() + + assert not CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + +# --------------------------------------------------------------------------------------------- +# MySQL collector semantics, reproduced on SQLite +# MySQL cannot defer foreign-key constraint checks, and Django's CASCADE handler reads that +# flag directly: it nulls a nullable cascading foreign key before the DELETE. On SQLite that +# nulling never happens, so the tests below monkeypatch the flag to reproduce it. Without the +# monkeypatch they pass against broken and correct code alike, so do not drop it. This is also +# why `scope_code` is a plain column rather than a `GeneratedField`: a generated column would +# recompute from the nulled scope foreign key mid-cascade and collide with whichever row already +# holds the resulting blank scope. + + +# --------------------------------------------------------------------------------------------- + + +def test_taxonomy_delete_cascades_its_scoped_profile_under_mysql_collector_semantics( + monkeypatch: pytest.MonkeyPatch, competency_taxonomy: CompetencyTaxonomy +) -> None: + """ + Deleting a CompetencyTaxonomy with a taxonomy-scoped profile succeeds and cascades the profile + away even under MySQL's non-deferred constraint semantics, the same as it does under ordinary + SQLite semantics (see test_deleting_a_taxonomy_with_a_scoped_rule_profile_also_deletes_the_ + profile above). Nulling the profile's `competency_taxonomy_id` before deleting it leaves + `scope_code` alone, so it cannot collide with the seeded system-default profile's identical + blank scope and raise IntegrityError instead of completing the cascade. + """ + monkeypatch.setattr(type(connection.features), "can_defer_constraint_checks", False, raising=False) + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + competency_taxonomy.delete() + + assert not CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + +def test_course_run_delete_cascades_its_scoped_rule_profile_under_mysql_collector_semantics( + monkeypatch: pytest.MonkeyPatch, course_run: CourseRun +) -> None: + """ + Deleting a CourseRun with a course-scoped CompetencyRuleProfile succeeds and cascades the + profile away even under MySQL's non-deferred constraint semantics, the same as the taxonomy + case above: `course` is CompetencyRuleProfile's other CASCADE foreign key, and shares the same + pre-delete-nulling collector path and the same scope_code collision this design avoids. + """ + monkeypatch.setattr(type(connection.features), "can_defer_constraint_checks", False, raising=False) + profile = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + course_run.delete() + + assert not CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + + +def test_deleting_two_taxonomies_together_cascades_both_their_scoped_profiles_away( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ + Deleting two CompetencyTaxonomy rows in one `.delete()` call, each with its own taxonomy-scoped + profile, succeeds and cascades both profiles away -- neither profile's scope_code collides with + the other's, even though both get their `competency_taxonomy_id` nulled in the same collector + batch under MySQL's non-deferred constraint semantics. + + Same path as the single-taxonomy MySQL case above, but confirms it does not get worse when two + scope owners are collected in the same collector pass: before scope_code became a plain column, + nulling both profiles' `competency_taxonomy_id` in the same batch drove both scope_code values + to the identical blank "org:,course:,taxonomy:" string and raised IntegrityError on whichever + row the database processed second. + """ + monkeypatch.setattr(type(connection.features), "can_defer_constraint_checks", False, raising=False) + taxonomy1 = CompetencyTaxonomy.objects.create(name="Nursing Two Taxonomy Delete", export_id="nursing-two-del") + taxonomy2 = CompetencyTaxonomy.objects.create(name="Welding Two Taxonomy Delete", export_id="welding-two-del") + profile1 = CompetencyRuleProfile.objects.create( + competency_taxonomy=taxonomy1, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile2 = CompetencyRuleProfile.objects.create( + competency_taxonomy=taxonomy2, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + CompetencyTaxonomy.objects.filter(pk__in=[taxonomy1.pk, taxonomy2.pk]).delete() + + assert not CompetencyRuleProfile.objects.filter(pk__in=[profile1.pk, profile2.pk]).exists()