diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 2d5847b8f..65b803cd4 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -79,6 +79,10 @@ openedx_content.UnitVersion: ".. no_pii:": "This model has no PII" openedx_learning.HistoricalCompetencyCriteriaGroup: ".. no_pii:": "This model has no PII" +openedx_learning.HistoricalCompetencyCriterion: + ".. 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..71b2bf501 100644 --- a/src/openedx_learning/applets/cbe/models/__init__.py +++ b/src/openedx_learning/applets/cbe/models/__init__.py @@ -2,11 +2,15 @@ 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, CompetencyCriterion, CompetencyRuleProfile, LogicOperator __all__ = [ "CompetencyCriteriaGroup", + "CompetencyCriterion", + "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 d987ef230..c69709722 100644 --- a/src/openedx_learning/applets/cbe/models/criteria.py +++ b/src/openedx_learning/applets/cbe/models/criteria.py @@ -1,22 +1,31 @@ """ -The CompetencyAchievementCriteria tree: CompetencyCriteriaGroup, the internal AND/OR node. +The CompetencyAchievementCriteria models: CompetencyCriteriaGroup, CompetencyRuleProfile, and +CompetencyCriterion. -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, 3 and 4 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 openedx_tagging.models import ObjectTag, Tag + +from ..rule_payloads import RuleType, validate_rule_payload +from .competency_taxonomy import CompetencyTaxonomy __all__ = [ "CompetencyCriteriaGroup", + "CompetencyCriterion", + "CompetencyRuleProfile", "LogicOperator", ] @@ -101,3 +110,245 @@ class Meta: # No constraint tying `logic_operator` to child count, and no UniqueConstraint on (parent, # ordering): a child group cannot be saved until its parent's primary key exists, so # neither has a single-row state to check at save time. See ADR-0002 Decision 2. + + +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 keyed by rule_type; see validate_rule_payload for the shape it must match.") + ) + 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) + + +class CompetencyCriterion(models.Model): + """ + A leaf node in a CompetencyAchievementCriteria tree: one tag/object association plus its rule. + + A null ``rule_profile`` does NOT mean "resolve the applicable profile at read time." ADR-0002 + Decision 4 resolves which profile (or override) applies at four specific write events + (creation, a more specific profile appearing later, an author setting a per-criterion + override, and an override being cleared back to matching the computed profile), and stores + the result. ``rule_profile`` is null only when an author has set a per-criterion override; in + every other case it holds the id of the profile that was resolved at the relevant write event + and is never re-resolved dynamically. Do not add a property, manager method, or other helper + that recomputes it; that would contradict the ADR. + + When ``rule_type_override`` is set, its ``rule_payload_override``'s shape (see + :func:`~openedx_learning.applets.cbe.rule_payloads.validate_rule_payload`) is validated from + ``clean()``, reached from both ``objects.create()`` and a plain ``instance.save()`` via + ``full_clean()``. A bulk ``QuerySet.update()``, ``bulk_create()``, and a DRF serializer that + writes straight to the database are NOT covered: none of them build or save a model instance, + so ``clean()`` never runs. + + .. no_pii: + """ + + uuid = immutable_uuid_field() + group = models.ForeignKey( + CompetencyCriteriaGroup, + db_column="competency_criteria_group_id", + on_delete=models.CASCADE, + related_name="criteria", + help_text=_("The CompetencyCriteriaGroup this leaf criterion belongs to."), + ) + object_tag = models.ForeignKey( + ObjectTag, + db_column="oel_tagging_objecttag_id", + on_delete=models.CASCADE, + related_name="competency_criteria", + help_text=_("The tag/object association that this criterion evaluates."), + ) + rule_profile = models.ForeignKey( + CompetencyRuleProfile, + null=True, + blank=True, + db_column="competency_rule_profile_id", + on_delete=models.RESTRICT, + related_name="criteria", + help_text=_("The profile this criterion uses by default. Null only when overrides are set instead."), + ) + rule_type_override = models.CharField(max_length=32, choices=RuleType, null=True, blank=True) + rule_payload_override = models.JSONField(null=True, blank=True) + + history = HistoricalRecords() + + class Meta: + # No db_table override: the table is Django's default, openedx_learning_competencycriterion. + # verbose_name/verbose_name_plural are set explicitly because Django's default pluralization + # of "CompetencyCriterion" is "competency criterions". See ADR-0002 Decision 4. + verbose_name = _("Competency Criterion") + verbose_name_plural = _("Competency Criteria") + constraints = [ + models.CheckConstraint( + condition=( + Q( + rule_profile__isnull=False, + rule_type_override__isnull=True, + rule_payload_override__isnull=True, + ) + | Q( + rule_profile__isnull=True, + rule_type_override__isnull=False, + rule_payload_override__isnull=False, + ) + ), + name="oel_cbe_criterion_profile_xor_override_check", + violation_error_message=_( + "A CompetencyCriterion must have either a rule_profile with no overrides, or both override " + "fields set with no rule_profile. Never both, never neither." + ), + ), + ] + + def clean(self): + """Validate the override rule_payload's shape, when a per-criterion override is set.""" + super().clean() + if self.rule_type_override is not None: + validate_rule_payload(self.rule_type_override, self.rule_payload_override) + + def save(self, *args, **kwargs): + """Persist this criterion, after full_clean() re-validates the override payload, if set.""" + self.full_clean(validate_unique=False, validate_constraints=False) + super().save(*args, **kwargs) diff --git a/src/openedx_learning/applets/cbe/rule_payloads.py b/src/openedx_learning/applets/cbe/rule_payloads.py new file mode 100644 index 000000000..24205c4c5 --- /dev/null +++ b/src/openedx_learning/applets/cbe/rule_payloads.py @@ -0,0 +1,88 @@ +""" +Rule payload shapes for CBE evaluation rules, and the validator that checks a raw payload against +the shape its rule_type defines. See :ref:`openedx-learning-adr-0002` Decision 3 for the payload +contract. ``RuleType`` declares exactly the rule types with a shape defined here, so a rule type +can never be offered as a choice without also being saveable. These messages reach an API caller +or admin form, so they must not leak internal class or function names. +""" +from __future__ import annotations + +from typing import Any, Callable + +from django.core.exceptions import ValidationError +from django.db import models +from django.utils.translation import gettext_lazy as _ + +__all__ = [ + "RuleType", + "validate_rule_payload", +] + + +class RuleType(models.TextChoices): + """ + The evaluation rule types a CompetencyRuleProfile or CompetencyCriterion override can use. + + Declares exactly the rule types with a defined rule_payload shape below, i.e. exactly the keys + of ``_RULE_PAYLOAD_SPECS``: see this module's own docstring for why the two are never allowed + to drift apart. + """ + + GRADE = "Grade", _("Grade") + + +_GRADE_OPERATORS = {"gte", "lte", "eq"} + + +def _validate_grade_payload(payload: dict) -> None: + """Validate a Grade payload's op, value, and scale. Keys are already checked.""" + if payload["op"] not in _GRADE_OPERATORS: + raise ValidationError(_("The 'op' in a 'Grade' rule_payload must be one of: gte, lte, eq.")) + value = payload["value"] + # isinstance(True, int) is True in Python, so a bool needs excluding explicitly. + if isinstance(value, bool) or not isinstance(value, (int, float)) or not 0.0 <= value <= 1.0: + raise ValidationError( + _( + "The 'value' in a 'Grade' rule_payload must be a fraction between 0.0 and 1.0 inclusive " + "(e.g. 0.8 for a passing grade of 80%%), not %(value)r." + ) + % {"value": value} + ) + if payload["scale"] != "percent": + raise ValidationError(_("The 'scale' in a 'Grade' rule_payload must be 'percent'.")) + + +# The required keys and validator for each rule type that has a defined payload shape. RuleType +# declares exactly these types, so a rule type can never be offered as a choice without being +# saveable. Adding one is an entry here, a validator, and the matching RuleType member. +_RULE_PAYLOAD_SPECS: dict[str, tuple[frozenset[str], Callable[[dict], None]]] = { + RuleType.GRADE: (frozenset({"op", "value", "scale"}), _validate_grade_payload), +} + + +def validate_rule_payload(rule_type: str, payload: Any) -> None: + """ + Raise ValidationError unless ``payload`` matches the shape ADR-0002 Decision 3 defines for + ``rule_type``, including when ``rule_type`` has no defined shape at all. + """ + spec = _RULE_PAYLOAD_SPECS.get(rule_type) + if spec is None: + raise ValidationError( + _("Rule type '%(rule_type)s' is not supported yet; only 'Grade' has a defined rule_payload shape.") + % {"rule_type": rule_type} + ) + expected_keys, validate_values = spec + if not isinstance(payload, dict): + raise ValidationError(_("A '%(rule_type)s' rule_payload must be a JSON object.") % {"rule_type": rule_type}) + missing = sorted(expected_keys - payload.keys()) + unexpected = sorted(payload.keys() - expected_keys) + if missing or unexpected: + raise ValidationError( + _("A '%(rule_type)s' rule_payload has the wrong keys: missing %(missing)s; unexpected %(unexpected)s.") + % { + "rule_type": rule_type, + "missing": ", ".join(missing) or _("none"), + "unexpected": ", ".join(unexpected) or _("none"), + } + ) + validate_values(payload) diff --git a/src/openedx_learning/migrations/0004_competencyruleprofile.py b/src/openedx_learning/migrations/0004_competencyruleprofile.py new file mode 100644 index 000000000..e351e3e2a --- /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 keyed by rule_type; see validate_rule_payload for the shape it must match.')), + ('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 keyed by rule_type; see validate_rule_payload for the shape it must match.')), + ('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/src/openedx_learning/migrations/0006_competencycriterion.py b/src/openedx_learning/migrations/0006_competencycriterion.py new file mode 100644 index 000000000..047118390 --- /dev/null +++ b/src/openedx_learning/migrations/0006_competencycriterion.py @@ -0,0 +1,61 @@ +# Generated by Django 5.2.16 on 2026-09-10 18:34 + +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 = [ + ('oel_tagging', '0021_remove_system_defined_add_read_only'), + ('openedx_learning', '0005_seed_default_rule_profile'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='HistoricalCompetencyCriterion', + 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_override', models.CharField(blank=True, choices=[('Grade', 'Grade')], max_length=32, null=True)), + ('rule_payload_override', models.JSONField(blank=True, null=True)), + ('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)), + ('group', models.ForeignKey(blank=True, db_column='competency_criteria_group_id', db_constraint=False, help_text='The CompetencyCriteriaGroup this leaf criterion belongs to.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_learning.competencycriteriagroup')), + ('history_user', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to=settings.AUTH_USER_MODEL)), + ('object_tag', models.ForeignKey(blank=True, db_column='oel_tagging_objecttag_id', db_constraint=False, help_text='The tag/object association that this criterion evaluates.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='oel_tagging.objecttag')), + ('rule_profile', models.ForeignKey(blank=True, db_column='competency_rule_profile_id', db_constraint=False, help_text='The profile this criterion uses by default. Null only when overrides are set instead.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_learning.competencyruleprofile')), + ], + options={ + 'verbose_name': 'historical Competency Criterion', + 'verbose_name_plural': 'historical Competency Criteria', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + migrations.CreateModel( + name='CompetencyCriterion', + 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')), + ('rule_type_override', models.CharField(blank=True, choices=[('Grade', 'Grade')], max_length=32, null=True)), + ('rule_payload_override', models.JSONField(blank=True, null=True)), + ('group', models.ForeignKey(db_column='competency_criteria_group_id', help_text='The CompetencyCriteriaGroup this leaf criterion belongs to.', on_delete=django.db.models.deletion.CASCADE, related_name='criteria', to='openedx_learning.competencycriteriagroup')), + ('object_tag', models.ForeignKey(db_column='oel_tagging_objecttag_id', help_text='The tag/object association that this criterion evaluates.', on_delete=django.db.models.deletion.CASCADE, related_name='competency_criteria', to='oel_tagging.objecttag')), + ('rule_profile', models.ForeignKey(blank=True, db_column='competency_rule_profile_id', help_text='The profile this criterion uses by default. Null only when overrides are set instead.', null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='criteria', to='openedx_learning.competencyruleprofile')), + ], + options={ + 'verbose_name': 'Competency Criterion', + 'verbose_name_plural': 'Competency Criteria', + 'constraints': [models.CheckConstraint(condition=models.Q(models.Q(('rule_payload_override__isnull', True), ('rule_profile__isnull', False), ('rule_type_override__isnull', True)), models.Q(('rule_payload_override__isnull', False), ('rule_profile__isnull', True), ('rule_type_override__isnull', False)), _connector='OR'), name='oel_cbe_criterion_profile_xor_override_check', violation_error_message='A CompetencyCriterion must have either a rule_profile with no overrides, or both override fields set with no rule_profile. Never both, never neither.')], + }, + ), + ] diff --git a/tests/openedx_learning/applets/cbe/conftest.py b/tests/openedx_learning/applets/cbe/conftest.py index 60fefbc79..58935b6f0 100644 --- a/tests/openedx_learning/applets/cbe/conftest.py +++ b/tests/openedx_learning/applets/cbe/conftest.py @@ -4,8 +4,8 @@ from organizations.models import Organization from openedx_catalog.models import CatalogCourse, CourseRun -from openedx_learning.models import CompetencyCriteriaGroup, CompetencyTaxonomy -from openedx_tagging.models import Tag +from openedx_learning.models import CompetencyCriteriaGroup, CompetencyRuleProfile, CompetencyTaxonomy +from openedx_tagging.models import ObjectTag, Tag @pytest.fixture(name="organization") @@ -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.""" @@ -34,7 +41,27 @@ def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: return Tag.objects.create(taxonomy=competency_taxonomy, value="Writing Poetry") +@pytest.fixture(name="object_tag") +def _object_tag(competency_taxonomy: CompetencyTaxonomy, tag: Tag) -> ObjectTag: + """An ObjectTag associating `tag` with a made-up content object, a criterion's target.""" + return ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+p1", + taxonomy=competency_taxonomy, + tag=tag, + ) + + @pytest.fixture(name="group") 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_criteria_deletion.py b/tests/openedx_learning/applets/cbe/test_criteria_deletion.py new file mode 100644 index 000000000..3c8b64e23 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criteria_deletion.py @@ -0,0 +1,544 @@ +""" +Delete-behavior tests for the three CompetencyAchievementCriteria models. + +Every test that deletes a row another row points at lives here. Each model declares its own +``on_delete`` values in the change that adds it; this module is where the behavior those values +produce, especially across more than one model, is pinned. + +ADR-0002 Decision 7 in one sentence: ``on_delete`` expresses containment rather than protection. +It governs deletion of the row a foreign key points *at*, never the row holding it, so the seven +cascading edges are how Django's collector walks *down* the tree, and the two remaining edges are +what refuse a delete outright. + +| Foreign key | Value | Why | +| CompetencyCriteriaGroup.parent | CASCADE | a subtree is meaningless without its parent | +| CompetencyCriteriaGroup.tag | CASCADE | a criteria tree is meaningless without its competency | +| CompetencyCriteriaGroup.course | CASCADE | a course-scoped tree is meaningless without its run | +| 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 | +| CompetencyCriterion.group | CASCADE | a leaf is meaningless without its group | +| CompetencyCriterion.object_tag | CASCADE | a leaf is meaningless without its content association | +| CompetencyCriterion.rule_profile | RESTRICT | a profile is never hard-deleted out from under a leaf | + +``rule_profile`` is RESTRICT rather than PROTECT because the two differ exactly where it matters +here. Both refuse a direct profile delete while a criterion is assigned to it. Only RESTRICT +ignores referencing rows that the same operation is already deleting, which is what lets a scope +owner's deletion carry its profile away instead of failing on a criterion that delete was about to +remove anyway. + +Only the cascade half of each case is asserted. Every matching "raises ProtectedError because a +learner status row exists" case needs #642's three Student*Status tables, and #642 is the change +that creates them, so those assertions belong there. Nothing here stubs or fakes a status model +to stand in for them. Until #642 merges, main carries a cascade chain with no PROTECT at the +bottom, so deleting a tag removes the whole authored tree and nothing objects. That window is +expected and harmless, because the learner status tables do not exist yet. + +Fixtures live in this directory's conftest.py. +""" +import pytest +from django.apps import apps +from django.db import connection +from django.db.models import ProtectedError, RestrictedError +from organizations.models import Organization + +from openedx_catalog.models import CourseRun +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + RuleType, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + + +# --------------------------------------------------------------------------------------------- +# CompetencyCriteriaGroup's three foreign keys +# Each CASCADE test asserts the referencing row existed beforehand and is gone afterward, not +# merely that no exception was raised. + + +# --------------------------------------------------------------------------------------------- + + +def test_deleting_a_group_also_deletes_its_child_groups(tag: Tag) -> None: + """ + Deleting a CompetencyCriteriaGroup cascades to any child group referencing it via `parent`: + the delete succeeds and the child row is gone too. + """ + root = CompetencyCriteriaGroup.objects.create(tag=tag) + child = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root) + assert CompetencyCriteriaGroup.objects.filter(pk=child.pk).exists() + + root.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=root.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=child.pk).exists() + + +def test_deleting_a_tag_also_deletes_its_competency_criteria_groups(tag: Tag, group: CompetencyCriteriaGroup) -> None: + """ + Deleting a Tag cascades to any CompetencyCriteriaGroup referencing it via `tag`: the delete + succeeds and the group row is gone. Also confirms django-simple-history records the cascaded + removal as its own historical row (history_type='-'), not silently: an author or auditor + reviewing history for a group that vanished this way still finds why it did. + """ + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + group_pk = group.pk + + tag.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group_pk).exists() + + historical_group = apps.get_model("openedx_learning", "HistoricalCompetencyCriteriaGroup") + assert historical_group.objects.filter(id=group_pk, history_type="-").exists() + + +def test_deleting_a_course_run_also_deletes_its_course_scoped_criteria_groups( + tag: Tag, course_run: CourseRun +) -> None: + """ + Deleting a CourseRun cascades to any CompetencyCriteriaGroup scoped to it via `course`: the + delete succeeds and the group row is gone too. A course-scoped criteria tree has no meaning + once the course run it evaluates against no longer exists. + """ + group = CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run) + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + course_run.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + +def test_a_cascaded_group_removal_is_recorded_in_history(tag: Tag) -> None: + """ + A group removed by a cascade, rather than by a direct delete, still gets its own historical + row with history_type '-'. An author or auditor reviewing history for a group that vanished + this way still finds why it did. + """ + historical_group = apps.get_model("openedx_learning", "HistoricalCompetencyCriteriaGroup") + group = CompetencyCriteriaGroup.objects.create(tag=tag) + group_pk = group.pk + + tag.delete() + + assert historical_group.objects.filter(id=group_pk, history_type="-").exists() + + +# --------------------------------------------------------------------------------------------- +# CompetencyRuleProfile's three foreign keys +# 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 and +# RESTRICT tests inspect 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. A CompetencyRuleProfile is never hard-deleted + by a *direct* delete of the profile itself (ADR-0002 Decision 7); that does not stop it being + cascaded away as a side effect of deleting the course it is scoped to, once nothing else (no + CompetencyCriterion still assigned to it) protects it -- a course is only ever hard-deleted + once nothing beneath it needs protecting. + """ + 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. A CompetencyRuleProfile is never hard-deleted by a *direct* delete of the profile itself + (ADR-0002 Decision 7); that does not stop it being cascaded away as a side effect of deleting + the taxonomy it is scoped to, once nothing else protects it. 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. See the "residual tension" section below for what happens instead when a + CompetencyCriterion is still assigned to the scoped profile being cascaded away. + """ + 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() + + +# --------------------------------------------------------------------------------------------- +# CompetencyCriterion's three foreign keys + + +# --------------------------------------------------------------------------------------------- + + +def test_deleting_a_group_also_deletes_its_criteria( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting a CompetencyCriteriaGroup cascades to any CompetencyCriterion referencing it via + `group`: the delete succeeds and the criterion row is gone too. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + group.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_deleting_an_object_tag_also_deletes_its_criteria( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting an ObjectTag cascades to any CompetencyCriterion referencing it via `object_tag`: the + delete succeeds and the criterion row is gone too. Doubles as the "OURS" half of #641's + Deletions criterion for oel_tagging_objecttag, since ObjectTag has only this one hop down to + CompetencyCriterion. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + object_tag.delete() + + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_deleting_a_rule_profile_referenced_by_a_criterion_raises_restricted_error( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting a CompetencyRuleProfile that a CompetencyCriterion references via `rule_profile` + raises RestrictedError, which is what holds ADR-0002 Decision 7's "a profile is never + hard-deleted by a direct delete" at the ORM layer. + + Nothing cascades from a profile down to a criterion, so the criterion is not part of this + delete and RESTRICT refuses, exactly as PROTECT would have. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + + with pytest.raises(RestrictedError) as exc_info: + default_rule_profile.delete() + + restricted = exc_info.value.restricted_objects + assert any(isinstance(obj, CompetencyCriterion) and obj.pk == criterion.pk for obj in restricted) + + +# --------------------------------------------------------------------------------------------- +# Transitive deletes required by issue #641 +# Deleting a Tag, a group at depth, or a Taxonomy takes the whole referencing criteria tree +# with it. Tag.taxonomy is already CASCADE in openedx_tagging, which is what makes the tag +# case hold transitively from a taxonomy. + + +# --------------------------------------------------------------------------------------------- + + +def test_tag_delete_with_no_status_cascades_whole_criteria_tree( + tag: Tag, group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting an oel_tagging.Tag with no learner status beneath it succeeds and cascades away + every CompetencyCriteriaGroup and CompetencyCriterion that references it, transitively: + Tag -> CompetencyCriteriaGroup.tag (CASCADE) -> CompetencyCriterion.group (CASCADE). + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + tag.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_group_delete_at_depth_cascades_descendants_and_their_criteria( + tag: Tag, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting a CompetencyCriteriaGroup that is not a root removes it, every descendant group, and + every CompetencyCriterion under any of them, while leaving the rest of the tree (here, the + root) alone. + + Builds a genuinely nested tree, root -> child -> grandchild, with criteria at two different + levels (on `child` and on `grandchild`), so "at depth" and "every descendant" both mean + something: a shallower tree could pass this by accident. + """ + root = CompetencyCriteriaGroup.objects.create(tag=tag) + child = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root) + grandchild = CompetencyCriteriaGroup.objects.create(tag=tag, parent=child) + child_criterion = CompetencyCriterion.objects.create( + group=child, object_tag=object_tag, rule_profile=default_rule_profile + ) + grandchild_criterion = CompetencyCriterion.objects.create( + group=grandchild, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert CompetencyCriteriaGroup.objects.filter(pk=root.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=child.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=grandchild.pk).exists() + assert CompetencyCriterion.objects.filter(pk=child_criterion.pk).exists() + assert CompetencyCriterion.objects.filter(pk=grandchild_criterion.pk).exists() + + child.delete() + + assert CompetencyCriteriaGroup.objects.filter(pk=root.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=child.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=grandchild.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=child_criterion.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=grandchild_criterion.pk).exists() + + +def test_taxonomy_delete_cascades_every_tag_and_its_criteria( + competency_taxonomy: CompetencyTaxonomy, + tag: Tag, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + Deleting an oel_tagging.Taxonomy collects every Tag beneath it (Tag.taxonomy is CASCADE), so + the tag-deletion cases above hold transitively through a taxonomy delete too. This asserts the + succeeding case (no learner status beneath the tag), which is what #641's Deletions criterion + for taxonomy-level deletion requires "at minimum". + + Chain exercised: CompetencyTaxonomy -> Tag (CASCADE) -> CompetencyCriteriaGroup.tag (CASCADE) + -> CompetencyCriterion.group (CASCADE). + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert Tag.objects.filter(pk=tag.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + competency_taxonomy.delete() + + assert not Tag.objects.filter(pk=tag.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +# --------------------------------------------------------------------------------------------- +# Scope-owner deletes that reach a profile a criterion is assigned to +# These are what RESTRICT on CompetencyCriterion.rule_profile buys, and what it still refuses. +# Both are unreachable until a taxonomy- or course-scoped profile can be authored, which no code +# path does yet. See ADR-0002 Decision 7. + + +# --------------------------------------------------------------------------------------------- + + +def test_taxonomy_delete_reaching_its_scoped_profile_through_a_criterion_succeeds( + competency_taxonomy: CompetencyTaxonomy, group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + Deleting a CompetencyTaxonomy whose taxonomy-scoped profile is itself assigned to a criterion + succeeds, and takes the profile and the criterion with it. + + This is the case RESTRICT exists for. The delete reaches the profile through + `competency_taxonomy` (CASCADE) and reaches the criterion through the tag chain + (Tag -> CompetencyCriteriaGroup.tag -> CompetencyCriterion.group, all CASCADE). RESTRICT then + finds nothing left restricting the profile, because the only row referencing it is one this + same operation is already deleting. Under PROTECT this raised ProtectedError naming that + criterion, which was a spurious failure: an author deleting a taxonomy was told a criterion + was in the way, when nothing about that criterion survived the delete either. + """ + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + criterion = CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=profile) + + competency_taxonomy.delete() + + assert not CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + +def test_course_run_delete_is_refused_by_a_criterion_outside_its_scope( + course_run: CourseRun, group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + Deleting a CourseRun whose course-scoped profile is assigned to a criterion that the same + delete does NOT reach raises RestrictedError, and nothing is removed. + + A criterion's profile assignment is independent of its tree's `course` scope (ADR-0002 + Decision 4), so a criterion in a tree with `course=None`, which `group` is, can still be + assigned a course-scoped profile. Deleting that run collects the profile but not the + criterion, so RESTRICT correctly refuses: unlike the taxonomy case above, this criterion + really would have been left pointing at a deleted profile. ADR-0002 Decision 7 records this + as the residual case, whose fix is a fifth reassignment event on Decision 4. + """ + profile = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + criterion = CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=profile) + assert group.course is None + + with pytest.raises(RestrictedError) as exc_info: + course_run.delete() + + restricted = exc_info.value.restricted_objects + assert any(isinstance(obj, CompetencyCriterion) and obj.pk == criterion.pk for obj in restricted) + # Nothing was removed: the whole operation raised before any DELETE executed. + assert CompetencyRuleProfile.objects.filter(pk=profile.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + assert CourseRun.objects.filter(pk=course_run.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 CompetencyRuleProfile.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_course_run_delete_cascades_its_course_scoped_criteria_group_under_mysql_collector_semantics( + monkeypatch: pytest.MonkeyPatch, tag: Tag, course_run: CourseRun +) -> None: + """ + Deleting a CourseRun with a course-scoped CompetencyCriteriaGroup succeeds and cascades the + group away even under MySQL's non-deferred constraint semantics. `course` is one of the two + nullable cascading foreign keys, so it shares the exact pre-delete-nulling collector path the + taxonomy case below does; unlike scope_code, + CompetencyCriteriaGroup carries no uniqueness constraint a null `course_id` could collide with, + so this path is expected to just succeed. Pinned here anyway, alongside the taxonomy case, + since a future fix to one foreign key without the other would otherwise go unnoticed. + """ + monkeypatch.setattr(type(connection.features), "can_defer_constraint_checks", False, raising=False) + group = CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run) + + course_run.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + +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 newly-CASCADE foreign key, and shares the + same pre-delete-nulling collector path and the same scope_code collision this fix removes. + """ + 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() diff --git a/tests/openedx_learning/applets/cbe/test_criteria_trees.py b/tests/openedx_learning/applets/cbe/test_criteria_trees.py new file mode 100644 index 000000000..1b1988744 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criteria_trees.py @@ -0,0 +1,144 @@ +""" +Integrative tests for CompetencyAchievementCriteria trees. + +test_criteria_deletion.py proves each foreign key cascades or protects correctly in isolation. +That is not the same claim as "deleting somewhere in the middle of a realistic tree leaves exactly +the right rows behind and nothing else": a per-foreign-key test can pass while a wider tree still +ends up with an orphaned group, a criterion pointing at nothing, or a sibling branch disturbed by +a delete that should not have touched it. The tests here build a wider tree on purpose and assert +the full surviving/removed row set, not just that a cascade fired somewhere. + +Fixtures shared with test_criteria_models.py and test_criteria_deletion.py live in this directory's +conftest.py. +""" +import pytest + +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + RuleType, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + + +def test_object_tag_delete_leaves_a_childless_criteria_group_behind( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting an ObjectTag cascades away the CompetencyCriterion that references it, but leaves the + CompetencyCriteriaGroup that housed that criterion in place, even when it was the group's only + criterion and the group now has no children of any kind (no criteria, no child groups). + + This is a deliberately accepted outcome, not a bug: CompetencyCriteriaGroup does not reference + ObjectTag at all (only CompetencyCriterion does), so nothing about deleting an ObjectTag gives + the collector a reason to reach the group. A childless group left behind this way is inert (it + evaluates no criteria and contributes nothing to its parent's logic_operator combination) and + is exactly the state authoring tooling must already handle for a group edited down to zero + children, so no additional cleanup path exists for this narrower case either. Pinned here so a + future change one way or the other (cascading the now-childless group away, or continuing to + leave it) is a deliberate decision, not an accidental side effect of something else. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + object_tag.delete() + + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert not CompetencyCriteriaGroup.objects.get(pk=group.pk).criteria.exists() + + +def test_deleting_a_middle_group_removes_its_subtree_but_leaves_the_rest_of_the_tree_untouched( + tag: Tag, competency_taxonomy: CompetencyTaxonomy, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting a CompetencyCriteriaGroup partway down a realistic tree removes exactly that group, + every descendant beneath it, and every criterion under any of them -- and nothing else. A + sibling branch of the deleted group, with its own criterion, survives completely unchanged. + + Tree built here, all under one root: + + root + |-- branch_to_delete (criterion: profile-assigned, via default_rule_profile) + | `-- grandchild (criterion: override, no rule_profile) + `-- surviving_sibling (criterion: profile-assigned, via a taxonomy-scoped profile) + + `branch_to_delete` is deleted. This exercises criteria at two different tree depths (on + `branch_to_delete` itself and on its child `grandchild`) with a genuine mix of the two ways a + criterion can be governed (a stored `rule_profile` vs. per-criterion overrides), and confirms + `surviving_sibling` and its own criterion are byte-for-byte untouched: same primary keys, still + present, in a tree that shares a root with the subtree that just got removed. A test that only + checks "the deleted branch is gone" cannot tell a correct cascade apart from one that + over-deletes into a sibling it should never have reached; this test can. + """ + root = CompetencyCriteriaGroup.objects.create(tag=tag, name="root") + branch_to_delete = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root, name="branch_to_delete") + grandchild = CompetencyCriteriaGroup.objects.create(tag=tag, parent=branch_to_delete, name="grandchild") + surviving_sibling = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root, name="surviving_sibling") + + branch_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+branch", taxonomy=competency_taxonomy, tag=tag + ) + grandchild_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+grandchild", taxonomy=competency_taxonomy, tag=tag + ) + sibling_object_tag = ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+sibling", taxonomy=competency_taxonomy, tag=tag + ) + + taxonomy_scoped_profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + branch_criterion = CompetencyCriterion.objects.create( + group=branch_to_delete, object_tag=branch_object_tag, rule_profile=default_rule_profile + ) + grandchild_criterion = CompetencyCriterion.objects.create( + group=grandchild, + object_tag=grandchild_object_tag, + rule_type_override=RuleType.GRADE, + rule_payload_override=_GRADE_PAYLOAD, + ) + sibling_criterion = CompetencyCriterion.objects.create( + group=surviving_sibling, object_tag=sibling_object_tag, rule_profile=taxonomy_scoped_profile + ) + + all_group_pks = {root.pk, branch_to_delete.pk, grandchild.pk, surviving_sibling.pk} + all_criterion_pks = {branch_criterion.pk, grandchild_criterion.pk, sibling_criterion.pk} + existing_group_pks = set(CompetencyCriteriaGroup.objects.filter(pk__in=all_group_pks).values_list("pk", flat=True)) + existing_criterion_pks = set( + CompetencyCriterion.objects.filter(pk__in=all_criterion_pks).values_list("pk", flat=True) + ) + assert existing_group_pks == all_group_pks + assert existing_criterion_pks == all_criterion_pks + + branch_to_delete.delete() + + remaining_group_pks = set( + CompetencyCriteriaGroup.objects.filter(pk__in=all_group_pks).values_list("pk", flat=True) + ) + remaining_criterion_pks = set( + CompetencyCriterion.objects.filter(pk__in=all_criterion_pks).values_list("pk", flat=True) + ) + + # Exactly the root and the surviving sibling remain; the deleted branch and its child are gone. + assert remaining_group_pks == {root.pk, surviving_sibling.pk} + # Exactly the sibling's criterion remains; both criteria under the deleted branch are gone, + # regardless of whether they were profile-assigned or override-governed. + assert remaining_criterion_pks == {sibling_criterion.pk} + + # The surviving sibling and its criterion are not merely "still present somewhere" but the + # exact same rows, untouched by the delete of an unrelated branch under the same root. + surviving_sibling.refresh_from_db() + sibling_criterion.refresh_from_db() + assert surviving_sibling.parent_id == root.pk + assert sibling_criterion.group_id == surviving_sibling.pk + assert sibling_criterion.rule_profile_id == taxonomy_scoped_profile.pk diff --git a/tests/openedx_learning/applets/cbe/test_criterion.py b/tests/openedx_learning/applets/cbe/test_criterion.py new file mode 100644 index 000000000..5acb4b236 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criterion.py @@ -0,0 +1,244 @@ +""" +Tests for CompetencyCriterion, a leaf of a CompetencyAchievementCriteria tree. + +Each test name states the behavior it pins. A leaf points at one ObjectTag, meaning one specific +piece of tagged content, and takes its pass rule either from a shared CompetencyRuleProfile or +from its own inline override pair, never from both and never from neither. + +Reading top to bottom gives the model's contract: its columns, the either-profile-or-overrides +invariant and every way it can be violated, that an override payload is validated on save, that +the stored profile is never re-resolved at read time, and its indexes and history. + +Delete behavior is not covered here and not decided here. Nothing in this module deletes a row +that another row points at. See the change that settles delete behavior for those tests, which +is also where the tree-wide integration tests live. + +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 openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + RuleType, +) +from openedx_tagging.models import ObjectTag + +pytestmark = pytest.mark.django_db + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + +# One (rule_type, payload) pair per way ADR-0002 Decision 3 says a rule_payload can be invalid. +# test_rule_payloads.py covers these shapes directly; here they only have to reach clean(). +_INVALID_GRADE_PAYLOADS = [ + pytest.param(RuleType.GRADE, {"op": "startswith", "value": 0.8, "scale": "percent"}, id="bad_op"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": 80, "scale": "percent"}, id="value_80_not_0_8"), + pytest.param(RuleType.GRADE, {"op": "gte", "scale": "percent"}, id="missing_key"), + pytest.param(RuleType.GRADE, ["not", "a", "dict"], id="non_dict"), +] + + +# --------------------------------------------------------------------------------------------- +# Schema + + +# --------------------------------------------------------------------------------------------- + + +def test_criterion_has_exactly_the_columns_adr_0002_decision_4_lists() -> None: + """ + CompetencyCriterion's columns are exactly the ones ADR-0002 Decision 4 lists, with + `rule_profile`, `rule_type_override`, and `rule_payload_override` optional and the rest + required. No `archived` column yet; that arrives with #642. Carries no Meta.db_table + override, so the table is Django's default name for the class. + """ + fields = [f for f in CompetencyCriterion._meta.get_fields() if f.concrete] + assert {f.name for f in fields} == { + "id", "uuid", "group", "object_tag", "rule_profile", "rule_type_override", "rule_payload_override", + } + assert {f.name for f in fields if f.null} == {"rule_profile", "rule_type_override", "rule_payload_override"} + assert CompetencyCriterion._meta.get_field("group").db_column == "competency_criteria_group_id" + assert CompetencyCriterion._meta.get_field("object_tag").db_column == "oel_tagging_objecttag_id" + assert CompetencyCriterion._meta.get_field("rule_profile").db_column == "competency_rule_profile_id" + assert CompetencyCriterion._meta.db_table == "openedx_learning_competencycriterion" + + +# --------------------------------------------------------------------------------------------- +# Either a rule_profile or both overrides. Never both, never neither. +# ADR-0002 Decision 4. Three of the four invalid states reach the database check constraint +# and raise IntegrityError. The fourth, rule_type_override set with no payload, is caught +# earlier by save()'s payload validation and raises ValidationError instead. + + +# --------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "invalid_kwargs", + [ + pytest.param( + {"rule_type_override": RuleType.GRADE, "rule_payload_override": _GRADE_PAYLOAD, "use_profile": True}, + id="both_set", + ), + pytest.param({"use_profile": False}, id="neither_set"), + pytest.param({"rule_payload_override": _GRADE_PAYLOAD, "use_profile": False}, id="only_payload_override_set"), + ], +) +def test_criterion_profile_xor_override_check_constraint_rejects_invalid_states( + invalid_kwargs: dict, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """ + A CompetencyCriterion must have either a rule_profile with no overrides, or both override + fields set with no rule_profile, never both and never neither. See ADR-0002 Decision 4. + + Covers the three invalid states that reach the database's check constraint: both set, neither + set, and only rule_payload_override set. The fourth invalid state, only rule_type_override set, + is caught earlier by save()'s own validation instead and raises ValidationError before the + database is ever touched; see test_setting_a_rule_type_override_without_a_payload_is_rejected_by_save + below for that case, and why it raises a different exception type than these three. + """ + use_profile = invalid_kwargs.pop("use_profile") + kwargs = dict(invalid_kwargs) + if use_profile: + kwargs["rule_profile"] = default_rule_profile + + with pytest.raises(IntegrityError): + with transaction.atomic(): + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, **kwargs) + + +def test_criterion_accepts_either_a_rule_profile_or_both_overrides( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Both valid states of the profile-xor-overrides check constraint save successfully: a + rule_profile with no overrides, and both override fields set with no rule_profile. + See ADR-0002 Decision 4. + """ + with_profile = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + assert with_profile.pk is not None + + with_overrides = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_type_override=RuleType.GRADE, rule_payload_override=_GRADE_PAYLOAD + ) + assert with_overrides.pk is not None + + +def test_setting_a_rule_type_override_without_a_payload_is_rejected_by_save( + group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + Setting only rule_type_override, leaving rule_payload_override null, is caught by save()'s + own validation before it ever reaches the database: save() validates rule_payload_override's + shape whenever rule_type_override is set, and None is not a valid shape for any rule type, so + this raises ValidationError. The database's check constraint would also reject this same row, + for the same underlying reason (an override with no real payload), but save() never lets it + get there. This is why two similar-looking invalid override states raise different exception + types: this one is caught by save()'s validate_rule_payload call, while the other three (see + test_criterion_profile_xor_override_check_constraint_rejects_invalid_states above) reach the + database's check constraint, because the payload save() inspects for them is either valid or, + when rule_type_override itself is null, not inspected at all. + """ + with pytest.raises(ValidationError): + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_type_override=RuleType.GRADE) + + +# --------------------------------------------------------------------------------------------- +# Override payload validation, and the profile that is never re-resolved + + +# --------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize("rule_type, payload", _INVALID_GRADE_PAYLOADS) +def test_criterion_full_clean_rejects_invalid_override_payload( + rule_type: str, payload: object, group: CompetencyCriteriaGroup, object_tag: ObjectTag +) -> None: + """ + full_clean() raises ValidationError for a CompetencyCriterion's rule_payload_override on the + same invalid shapes as CompetencyRuleProfile.rule_payload. See ADR-0002 Decision 3. + """ + criterion = CompetencyCriterion( + group=group, object_tag=object_tag, rule_type_override=rule_type, rule_payload_override=payload + ) + with pytest.raises(ValidationError): + criterion.full_clean() + + +def test_criterion_rule_profile_is_not_recomputed_once_a_more_specific_profile_appears( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + A criterion's stored rule_profile is not resolved dynamically at read time: creating a new, + more specific profile later does not silently re-govern a criterion that already resolved to a + less specific one. See ADR-0002 Decision 4, which lists the specific write events that DO + reassign a criterion (not exercised here) and states that no other path may recompute it. This + guards against a property, manager method, or signal handler being added that would violate + that rule by resolving the FK on every read instead of only at those write events. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + + CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + criterion.refresh_from_db() + assert criterion.rule_profile_id == default_rule_profile.pk + + +# --------------------------------------------------------------------------------------------- +# Indexes 4 and 5, and history + + +# --------------------------------------------------------------------------------------------- + + +def test_the_database_carries_adr_0002_decision_5_indexes_4_and_5() -> None: + """ + The real table carries ADR-0002 Decision 5's index 4 on object_tag and index 5 on group. Both + come from Django's automatic per-ForeignKey index rather than an explicit models.Index, so + this introspects the database rather than the model and holds either way. + """ + with connection.cursor() as cursor: + constraints = connection.introspection.get_constraints(cursor, CompetencyCriterion._meta.db_table) + + def is_indexed(columns: list[str]) -> bool: + return any(c["columns"] == columns and c["index"] for c in constraints.values()) + + assert is_indexed(["oel_tagging_objecttag_id"]) + assert is_indexed(["competency_criteria_group_id"]) + + +def test_editing_a_criterion_writes_a_historical_row( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + HistoricalRecords() is applied to CompetencyCriterion: creating a criterion and then switching + it from a profile to overrides leaves two rows in the Historical model. See ADR-0003 + Decision 1, and Decision 4 for why that switch is an authoring event worth recording. + """ + historical_criterion = apps.get_model("openedx_learning", "HistoricalCompetencyCriterion") + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + + criterion.rule_profile = None + criterion.rule_type_override = RuleType.GRADE + criterion.rule_payload_override = _GRADE_PAYLOAD + criterion.save() + + assert historical_criterion.objects.filter(id=criterion.pk).count() == 2 diff --git a/tests/openedx_learning/applets/cbe/test_rule_payloads.py b/tests/openedx_learning/applets/cbe/test_rule_payloads.py new file mode 100644 index 000000000..705d1a204 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rule_payloads.py @@ -0,0 +1,130 @@ +""" +Tests for the CBE rule payload contract: RuleType and validate_rule_payload. + +These need no database. validate_rule_payload is a plain function over a dict, and the models +that call it from clean() arrive in later changes, so every behavior here is exercised directly +rather than through a model save. + +ADR-0002 Decision 3 defines one payload shape per rule_type. The single supported type is +"Grade", whose payload is {"op": ..., "value": ..., "scale": ...} where op is one of gte, lte or +eq, value is a fraction from 0.0 to 1.0 rather than a number out of 100, and scale is "percent". +""" +import pytest +from django.core.exceptions import ValidationError + +# Private: the payload-spec registry, compared against RuleType's declared choices below. +from openedx_learning.applets.cbe.rule_payloads import _RULE_PAYLOAD_SPECS, RuleType, validate_rule_payload + +_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} + + +def test_a_well_formed_grade_payload_is_accepted() -> None: + """A Grade payload with a valid op, a fraction value, and the percent scale raises nothing.""" + validate_rule_payload(RuleType.GRADE, _GRADE_PAYLOAD) + + +@pytest.mark.parametrize( + "op", + [pytest.param("gte", id="gte"), pytest.param("lte", id="lte"), pytest.param("eq", id="eq")], +) +def test_every_documented_comparison_operator_is_accepted(op: str) -> None: + """All three operators ADR-0002 Decision 3 lists are accepted, not just the seeded gte.""" + validate_rule_payload(RuleType.GRADE, {**_GRADE_PAYLOAD, "op": op}) + + +@pytest.mark.parametrize( + "value", + [pytest.param(0.0, id="lower_bound"), pytest.param(1.0, id="upper_bound"), pytest.param(1, id="int_one")], +) +def test_the_ends_of_the_zero_to_one_range_are_accepted(value: float) -> None: + """0.0 and 1.0 are both inside the range, and an int is a number as far as this rule cares.""" + validate_rule_payload(RuleType.GRADE, {**_GRADE_PAYLOAD, "value": value}) + + +# One (rule_type, payload) pair per way ADR-0002 Decision 3 says a rule_payload can be invalid. +_INVALID_PAYLOADS = [ + pytest.param(RuleType.GRADE, {"op": "startswith", "value": 0.8, "scale": "percent"}, id="bad_op"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": 80, "scale": "percent"}, id="value_80_not_0_8"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": 1.5, "scale": "percent"}, id="value_above_range"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": -0.1, "scale": "percent"}, id="value_below_range"), + pytest.param(RuleType.GRADE, {"op": "gte", "scale": "percent"}, id="missing_key"), + pytest.param(RuleType.GRADE, {**_GRADE_PAYLOAD, "extra": 1}, id="extra_key"), + pytest.param(RuleType.GRADE, ["not", "a", "dict"], id="non_dict"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": 0.8, "scale": "raw"}, id="wrong_scale"), + pytest.param(RuleType.GRADE, {"op": "gte", "value": True, "scale": "percent"}, id="boolean_value"), + # "View" is a plain string, not RuleType.VIEW: RuleType declares only rule types that have a + # payload spec, so an unsupported rule type is by construction not a RuleType member at all. + pytest.param("View", _GRADE_PAYLOAD, id="unsupported_rule_type"), +] + + +@pytest.mark.parametrize("rule_type, payload", _INVALID_PAYLOADS) +def test_every_documented_way_a_payload_can_be_wrong_raises_validation_error( + rule_type: str, payload: object +) -> None: + """ + Each invalid shape raises ValidationError rather than passing or raising something the caller + would not expect: a bad op, a value given out of 100 instead of as a fraction, a value outside + the range at either end, a missing or extra key, a non-dict payload, a wrong scale, a boolean + masquerading as a number, and a rule_type with no defined payload shape. + """ + with pytest.raises(ValidationError): + validate_rule_payload(rule_type, payload) + + +def test_a_boolean_value_is_rejected_even_though_python_calls_it_an_int() -> None: + """ + True is rejected. isinstance(True, int) is True in Python, so a bool would slip through a + plain numeric check, and True would then read as the fraction 1.0, silently meaning "100%". + """ + with pytest.raises(ValidationError): + validate_rule_payload(RuleType.GRADE, {**_GRADE_PAYLOAD, "value": True}) + + +def test_an_out_of_range_value_message_names_the_fraction_convention() -> None: + """ + The message for a value given out of 100 (for example 80) names the 0.0 to 1.0 fraction + convention, so an author who wrote 80 meaning 80% is told what to write instead. This is the + single most likely authoring mistake for this payload. + """ + with pytest.raises(ValidationError) as exc_info: + validate_rule_payload(RuleType.GRADE, {"op": "gte", "value": 80, "scale": "percent"}) + + assert "fraction between 0.0 and 1.0" in " ".join(exc_info.value.messages) + + +def test_a_wrong_keys_message_names_the_offending_keys() -> None: + """ + The message for a wrong key set names both what is missing and what is unexpected, so an + author can see which key to fix rather than being told only that the payload is invalid. + """ + with pytest.raises(ValidationError) as exc_info: + validate_rule_payload(RuleType.GRADE, {"op": "gte", "extra": 1}) + + message = " ".join(exc_info.value.messages) + assert "extra" in message + assert "value" in message and "scale" in message + + +def test_an_unsupported_rule_type_says_only_grade_is_defined() -> None: + """ + A rule_type with no payload shape is rejected with a message saying so, rather than being + silently accepted. ADR-0002 Decision 3 lists View and MasteryLevel as future types; neither + has a defined shape yet. + """ + with pytest.raises(ValidationError) as exc_info: + validate_rule_payload("MasteryLevel", {"level": 3}) + + assert "not supported yet" in " ".join(exc_info.value.messages) + + +def test_rule_type_declares_exactly_the_types_that_have_a_payload_spec() -> None: + """ + RuleType's choices, which is what a serializer or admin form offers an author, contain exactly + the rule types that can actually be saved. + + A rule_type with no payload spec is always rejected regardless of payload content, so + declaring a RuleType member without its spec would offer an author a dead-end choice. This + pins the invariant so adding one without the other fails a test instead of shipping. + """ + assert {value for value, _label in RuleType.choices} == set(_RULE_PAYLOAD_SPECS) 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..bf52132c8 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_rule_profile.py @@ -0,0 +1,446 @@ +""" +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 and not decided here. Nothing in this module deletes a row +that another row points at, except where a test frees the seeded system-default scope, which +nothing references. See the change that settles delete behavior for those tests. + +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.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 0003 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