diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index 6b9f74d07..65b803cd4 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -77,6 +77,12 @@ openedx_content.Unit: ".. no_pii:": "This model has no PII" 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/.importlinter b/.importlinter index 17dd176f6..844575b91 100644 --- a/.importlinter +++ b/.importlinter @@ -7,6 +7,7 @@ root_packages = openedx_learning openedx_content + openedx_catalog openedx_tagging openedx_django_lib openedx_core @@ -26,6 +27,11 @@ layers = # Content: authoring-side models and APIs. openedx_content + # Catalog: CatalogCourse/CourseRun. CompetencyCriteriaGroup and CompetencyRuleProfile + # (openedx_learning) scope to a CourseRun, so this must sit below openedx_learning; it doesn't + # depend on tagging or content, so it can sit above openedx_tagging. + openedx_catalog + # Tagging is very simple & fundamental. Should probably not depend on any other Django apps. openedx_tagging diff --git a/mypy.ini b/mypy.ini index b383a8816..665714903 100644 --- a/mypy.ini +++ b/mypy.ini @@ -12,5 +12,8 @@ files = [mypy-organizations.*] follow_untyped_imports = True +[mypy-simple_history.*] +follow_untyped_imports = True + [mypy.plugins.django-stubs] django_settings_module = "projects.dev" diff --git a/requirements/base.in b/requirements/base.in index 626a551be..7292e10c3 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -17,3 +17,5 @@ rules<4.0 # Django extension for rules-based authorization check tomlkit # Parses and writes TOML configuration files edx-organizations # Implemented the "Organization" model that CatalogCourse/CourseRun are keyed to + +django-simple-history # History tracking for CBE criteria definitions, per ADR-0003 diff --git a/requirements/base.txt b/requirements/base.txt index c7dac57c3..c89949f77 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -58,7 +58,9 @@ django-crum==0.7.9 django-model-utils==5.0.0 # via edx-organizations django-simple-history==3.13.0 - # via edx-organizations + # via + # -r requirements/base.in + # edx-organizations django-waffle==5.0.0 # via # edx-django-utils diff --git a/src/openedx_learning/applets/cbe/admin.py b/src/openedx_learning/applets/cbe/admin.py index 71f7cf583..847dd88ba 100644 --- a/src/openedx_learning/applets/cbe/admin.py +++ b/src/openedx_learning/applets/cbe/admin.py @@ -3,10 +3,22 @@ """ from django.contrib import admin -from .models import CompetencyTaxonomy +from openedx_django_lib.admin_utils import ReadOnlyModelAdmin + +from .models import ( + CompetencyMasteryStatus, + CompetencyTaxonomy, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriteriaStatus, + StudentCompetencyStatus, +) __all__ = [ "CompetencyTaxonomyAdmin", + "CompetencyMasteryStatusAdmin", + "StudentCompetencyCriteriaStatusAdmin", + "StudentCompetencyCriteriaGroupStatusAdmin", + "StudentCompetencyStatusAdmin", ] @@ -18,4 +30,54 @@ class CompetencyTaxonomyAdmin(admin.ModelAdmin): list_filter = ["enabled"] +class CompetencyMasteryStatusAdmin(ReadOnlyModelAdmin): + """ + The CompetencyMasteryStatus model admin. + """ + list_display = ["id", "status"] + + +class StudentCompetencyCriteriaStatusAdmin(ReadOnlyModelAdmin): + """ + The StudentCompetencyCriteriaStatus model admin. + + Deliberately read-only: an editable page would be the staff-correction + path, which ADR-0004 Decision 6 requires to take a row lock and recompute + every ancestor status, and none of that machinery exists yet. + """ + list_display = ["user", "criterion", "status", "created", "modified"] + list_filter = ["status"] + list_select_related = ["user", "criterion", "status"] + + +class StudentCompetencyCriteriaGroupStatusAdmin(ReadOnlyModelAdmin): + """ + The StudentCompetencyCriteriaGroupStatus model admin. + + Deliberately read-only: an editable page would be the staff-correction + path, which ADR-0004 Decision 6 requires to take a row lock and recompute + every ancestor status, and none of that machinery exists yet. + """ + list_display = ["user", "group", "status", "created", "modified"] + list_filter = ["status"] + list_select_related = ["user", "group", "status"] + + +class StudentCompetencyStatusAdmin(ReadOnlyModelAdmin): + """ + The StudentCompetencyStatus model admin. + + Deliberately read-only: an editable page would be the staff-correction + path, which ADR-0004 Decision 6 requires to take a row lock and recompute + every ancestor status, and none of that machinery exists yet. + """ + list_display = ["user", "tag", "status", "created", "modified"] + list_filter = ["status"] + list_select_related = ["user", "tag", "status"] + + admin.site.register(CompetencyTaxonomy, CompetencyTaxonomyAdmin) +admin.site.register(CompetencyMasteryStatus, CompetencyMasteryStatusAdmin) +admin.site.register(StudentCompetencyCriteriaStatus, StudentCompetencyCriteriaStatusAdmin) +admin.site.register(StudentCompetencyCriteriaGroupStatus, StudentCompetencyCriteriaGroupStatusAdmin) +admin.site.register(StudentCompetencyStatus, StudentCompetencyStatusAdmin) diff --git a/src/openedx_learning/applets/cbe/models/__init__.py b/src/openedx_learning/applets/cbe/models/__init__.py new file mode 100644 index 000000000..da5159dc7 --- /dev/null +++ b/src/openedx_learning/applets/cbe/models/__init__.py @@ -0,0 +1,35 @@ +""" +Models for Competency-Based Education (CBE). +""" + +from .competency_taxonomy import CompetencyTaxonomy +from .criteria import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + LogicOperator, + RuleType, + validate_rule_payload, +) +from .learner_status import ( + CompetencyMasteryStatus, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriteriaStatus, + StudentCompetencyStatus, +) + +__all__ = [ + "CompetencyTaxonomy", + "CompetencyCriteriaGroup", + "CompetencyCriterion", + "CompetencyRuleProfile", + "LogicOperator", + "RuleType", + "validate_rule_payload", + "MasteryStatus", + "CompetencyMasteryStatus", + "StudentCompetencyCriteriaStatus", + "StudentCompetencyCriteriaGroupStatus", + "StudentCompetencyStatus", +] diff --git a/src/openedx_learning/applets/cbe/models.py b/src/openedx_learning/applets/cbe/models/competency_taxonomy.py similarity index 60% rename from src/openedx_learning/applets/cbe/models.py rename to src/openedx_learning/applets/cbe/models/competency_taxonomy.py index 7cbd8cb3a..a0623fa6e 100644 --- a/src/openedx_learning/applets/cbe/models.py +++ b/src/openedx_learning/applets/cbe/models/competency_taxonomy.py @@ -1,6 +1,9 @@ """ -Models for Competency-Based Education (CBE). +The CompetencyTaxonomy model. """ +from django.db import models +from django.utils.translation import gettext_lazy as _ + from openedx_tagging.models import Taxonomy __all__ = [ @@ -35,6 +38,19 @@ class CompetencyTaxonomy(Taxonomy): .. no_pii: """ + taxonomy_overrides_org = models.BooleanField( + default=False, + help_text=_( + "Resolves a tie when assigning a CompetencyRuleProfile to a CompetencyCriterion (ADR-0002 " + "Decision 4): if both an organization-scoped profile and a taxonomy-scoped profile from this " + "taxonomy apply to the same criterion, False (the default) assigns the organization-scoped " + "profile, and True assigns this taxonomy's own profile instead, so it cannot be locally " + "weakened by an organization. Nothing reads this field yet: organization-scoped " + "CompetencyRuleProfile rows do not exist in this phase, so the tie it resolves cannot arise " + "until they do." + ), + ) + class Meta: verbose_name = "Competency Taxonomy" verbose_name_plural = "Competency Taxonomies" diff --git a/src/openedx_learning/applets/cbe/models/criteria.py b/src/openedx_learning/applets/cbe/models/criteria.py new file mode 100644 index 000000000..265ec4431 --- /dev/null +++ b/src/openedx_learning/applets/cbe/models/criteria.py @@ -0,0 +1,450 @@ +""" +Models for CompetencyAchievementCriteria: CompetencyCriteriaGroup, CompetencyRuleProfile, CompetencyCriterion. + +See :ref:`openedx-learning-adr-0002` for the design this module implements, and +:ref:`openedx-learning-adr-0003` for why these three models (and not CompetencyTaxonomy) carry +``django-simple-history`` tracking. + +Four of the nine foreign keys here are ``on_delete=models.CASCADE``: ``CompetencyCriteriaGroup.parent``, +``CompetencyCriteriaGroup.tag``, ``CompetencyCriterion.group``, and ``CompetencyCriterion.object_tag``. +The other five stay ``models.PROTECT``: ``CompetencyCriterion.rule_profile``, +``CompetencyCriteriaGroup.course``, ``CompetencyRuleProfile.course``, +``CompetencyRuleProfile.organization``, and ``CompetencyRuleProfile.competency_taxonomy``. + +The four are CASCADE because deleting a Tag nobody holds mastery against must succeed, and #655 +forbids ``openedx_tagging`` from knowing CBE exists, so the tagging side cannot clear the +criteria tree first; CASCADE lets the tag's delete take the tree with it. ``parent`` and ``group`` +also need CASCADE because Django's collector looks up referencing rows in the database rather +than in the set it has already decided to delete, so even a parent and child reached in the same +batch would trip ``PROTECT`` and abort the walk partway down. + +This is not a relaxation of ADR-0002 Decision 7: these four CASCADE links are what carries the +collector down to the ``PROTECT`` that enforces it, on #642's three ``Student*Status`` foreign +keys one and two levels below the tag, reached only by walking CASCADE edges. Turning any link in +that chain to ``SET_NULL`` would let a tag delete succeed while learner statuses for it still exist. + +``on_delete`` governs the row a foreign key points AT, never the row holding it, and fires on +every row the collector reaches, not only the row passed to ``delete()``. So ``rule_profile`` +staying PROTECT does not block a cascading tag delete; it only stops a CompetencyRuleProfile from +being deleted while a criterion references it, Decision 7's archive-only rule at the ORM layer. + +The other four: both ``course`` fields match ``openedx_catalog``'s own convention +(``CourseRun.catalog_course`` and ``CatalogCourse.org`` are PROTECT too), and ``SET_NULL`` would +make a course-level group read as a root group, breaking #675's root-group rejection. +``organization`` is PROTECT because ``edx-organizations`` deactivates orgs rather than deleting +them (``remove_organization()``). ``competency_taxonomy`` is PROTECT because a profile's scope is +immutable, so ``SET_NULL`` is forbidden and CASCADE would only move the failure one hop. +""" +from __future__ import annotations + +from typing import Any + +from django.core.exceptions import ValidationError +from django.db import models +from django.db.models import F, Q, Value +from django.db.models.functions import Cast, Coalesce, Concat +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 ObjectTag, Tag + +from .competency_taxonomy import CompetencyTaxonomy + +__all__ = [ + "CompetencyCriteriaGroup", + "CompetencyCriterion", + "CompetencyRuleProfile", + "LogicOperator", + "RuleType", + "validate_rule_payload", +] + + +class RuleType(models.TextChoices): + """The evaluation rule types a CompetencyRuleProfile or CompetencyCriterion override can use.""" + + VIEW = "View", _("View") + GRADE = "Grade", _("Grade") + MASTERY_LEVEL = "MasteryLevel", _("Mastery Level") + + +class LogicOperator(models.TextChoices): + """How a CompetencyCriteriaGroup combines its child nodes.""" + + AND = "AND", _("And") + OR = "OR", _("Or") + + +def validate_rule_payload(rule_type: str, payload: Any) -> None: + """ + Validate ``payload`` against the shape ADR-0002 Decision 3 defines for ``rule_type``. + + Only ``RuleType.GRADE`` has a defined payload shape in this phase. ``RuleType.VIEW`` and + ``RuleType.MASTERY_LEVEL`` are valid choices elsewhere but are rejected here, since no + payload contract exists for them yet. Raises ``django.core.exceptions.ValidationError`` on + any mismatch; never returns a value. + """ + if rule_type != RuleType.GRADE: + raise ValidationError( + _("Rule type '%(rule_type)s' is not supported yet; only 'Grade' has a defined rule_payload shape.") + % {"rule_type": rule_type} + ) + if not isinstance(payload, dict): + raise ValidationError(_("A 'Grade' rule_payload must be a JSON object.")) + + allowed_keys = {"op", "value", "scale"} + if set(payload.keys()) != allowed_keys: + raise ValidationError( + _("A 'Grade' rule_payload must have exactly these keys, no more and no fewer: op, value, scale.") + ) + + if payload.get("op") not in {"gte", "lte", "eq"}: + raise ValidationError(_("The 'op' in a 'Grade' rule_payload must be one of: gte, lte, eq.")) + + value = payload.get("value") + # isinstance(True, int) is True in Python, so a bool would otherwise pass the numeric check below. + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise ValidationError(_("The 'value' in a 'Grade' rule_payload must be a number, not a boolean.")) + if 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.get("scale") != "percent": + raise ValidationError(_("The 'scale' in a 'Grade' rule_payload must be 'percent'.")) + + +class CompetencyCriteriaGroup(models.Model): + """ + An internal AND/OR node in a CompetencyAchievementCriteria expression tree. + + A single CompetencyAchievementCriteria is one root CompetencyCriteriaGroup plus all of its + descendant groups and leaf :class:`CompetencyCriterion` rows. ``logic_operator`` says how + this group's children combine; ``ordering`` gives their deterministic evaluation sequence, + which read-time evaluation and event-driven recomputation both rely on for short-circuiting. + See ADR-0002 Decision 2. + + .. no_pii: + """ + + uuid = immutable_uuid_field() + parent = models.ForeignKey( + "self", + null=True, + blank=True, + on_delete=models.CASCADE, + related_name="child_groups", + help_text=_("The parent CompetencyCriteriaGroup. Null means this group is a tree root."), + ) + tag = models.ForeignKey( + Tag, + db_column="oel_tagging_tag_id", + on_delete=models.CASCADE, + related_name="competency_criteria_groups", + help_text=_("The competency (tag) that this criteria tree evaluates mastery of."), + ) + course = models.ForeignKey( + CourseRun, + null=True, + blank=True, + on_delete=models.PROTECT, + related_name="competency_criteria_groups", + help_text=_("The course run that scopes this criteria tree for evaluation windowing, if any."), + ) + name = case_insensitive_char_field(max_length=255, blank=True, default="") + ordering = models.PositiveIntegerField( + default=0, + help_text=_( + "Deterministic sibling evaluation sequence. Used to short-circuit evaluation and to order " + "child scans during event-driven recomputation." + ), + ) + logic_operator = models.CharField( + max_length=3, + choices=LogicOperator, + null=True, + blank=True, + help_text=_("How this group's children combine. Null until the group has children to combine."), + ) + + history = HistoricalRecords() + + class Meta: + indexes = [ + # ADR-0002 Decision 5, index 1: lookups by competency tag and course scope. + models.Index(fields=["tag", "course"]), + # ADR-0002 Decision 5 also lists an index on `parent` (index 2), but Django already + # indexes every ForeignKey column by default, so a second explicit one here would only + # cost write throughput without adding any read benefit. + ] + # ADR-0002 Decision 2 explicitly excludes two constraints here, both for the same reason: + # a child group cannot be saved until its parent's primary key exists, so at the moment a + # parent group is being validated/saved, its clean() always sees zero children, whether or + # not more are about to be attached. There's no single-row state at save time to check either + # of these against: + # - A constraint tying `logic_operator` to child count. + # - A UniqueConstraint on (parent, ordering), which would need to see all siblings, not just + # the row being saved. + + +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. + + Editing a profile may change ``rule_type``/``rule_payload`` only: the scope fields + (``organization``, ``course``, ``competency_taxonomy``) are immutable after creation, so that + criteria already resolved to this profile's scope are never silently re-governed. This is + enforced in ``clean()`` and ``save()`` by comparing against the scope this row had when + loaded. That comparison covers every ``instance.save()``, including one loaded with + ``.only()``/``.defer()`` that skipped some scope columns, in which case the comparison falls + back to reading the persisted scope directly rather than skipping the check. It does not + cover a bulk ``QuerySet.update()``, since that path never loads or constructs a model + instance at all. + + .. no_pii: + """ + + # Set at from_db() time to the scope this row had when it was loaded from the database, so + # clean()/save() can detect an attempt to change it. None for a newly-constructed instance, + # meaning there's nothing yet to compare against. Deliberately not underscore-prefixed: + # from_db() is a classmethod, so it sets this through a local `instance` variable rather than + # `self`, which pylint's protected-access check can't tell apart from reaching into another + # object's internals. + loaded_scope: tuple[int | None, int | None, int | None] | None = None + + 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.PROTECT, + 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.PROTECT, + related_name="rule_profiles", + help_text=_("The competency taxonomy this profile is scoped to, if any."), + ) + # Always non-null, including for the system-default row (all three scope columns null), so a + # plain UniqueConstraint on this one column enforces "at most one profile row per distinct + # scope" identically on every backend. SQL never treats two NULLs as equal, so a unique + # constraint directly on the three nullable scope columns would let e.g. two rows that both + # set only organization_id=5 both exist. See ADR-0002 Decision 3. + scope_code = models.GeneratedField( + expression=Concat( + Value("org:"), + Coalesce(Cast(F("organization_id"), output_field=models.CharField(max_length=20)), Value("")), + Value(",course:"), + Coalesce(Cast(F("course_id"), output_field=models.CharField(max_length=20)), Value("")), + Value(",taxonomy:"), + Coalesce(Cast(F("competency_taxonomy_id"), output_field=models.CharField(max_length=20)), Value("")), + ), + output_field=models.CharField(max_length=255), + db_persist=True, + ) + 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=_( + "Set instead of deleting a profile that's no longer wanted. Archived profiles are hidden from " + "authoring and new associations but remain queryable, so existing criteria stay resolvable." + ), + ) + uuid = immutable_uuid_field() + + history = HistoricalRecords(excluded_fields=["scope_code"]) + + class Meta: + constraints = [ + # Do NOT add `condition=` here. A conditional UniqueConstraint compiles to a partial + # index, which MySQL (this project's tested and production database) does not support: + # Django only raises a non-fatal system-check warning (models.W036) and silently skips + # creating the constraint, leaving uniqueness completely unenforced there, while SQLite + # (used for quick local test runs) does support partial indexes and would mask the gap + # in that environment. See ADR-0002 Rejected Alternative 6. The generated `scope_code` + # column above exists specifically so a plain, unconditional UniqueConstraint works + # identically on every backend. + 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." + ), + ), + ] + + @classmethod + def from_db(cls, db, field_names, values): + """Capture the scope this row had when loaded, so clean()/save() can detect an edit to it.""" + instance = super().from_db(db, field_names, values) + # field_names holds attnames (e.g. "organization_id"), not field names. Only capture when + # all three are present and unloaded (not deferred), so this never triggers extra queries. + scope_attnames = {"organization_id", "course_id", "competency_taxonomy_id"} + if scope_attnames.issubset(field_names): + instance.loaded_scope = ( + instance.organization_id, + instance.course_id, + instance.competency_taxonomy_id, + ) + return instance + + def _check_scope_immutable(self) -> None: + """Raise ValidationError if the scope columns no longer match what was loaded from the database.""" + loaded_scope = self.loaded_scope + if loaded_scope is None: + if self.pk is None: + # A new, unsaved instance: there's no persisted scope yet to compare against. + return + # from_db() didn't capture the scope, because this instance came from a deferred/ + # only() load that skipped one or more scope columns. Read the persisted scope back + # from the database directly, rather than silently skipping the check: a deferred + # load must not be a way to bypass immutability. This costs one extra query, but only + # on this rare path, which is already paying for extra field-loading queries anyway. + # Guarded against the row having since been deleted, in which case there's nothing + # left to compare against either. + row = ( + CompetencyRuleProfile.objects + .filter(pk=self.pk) + .values_list("organization_id", "course_id", "competency_taxonomy_id") + .first() + ) + if row is None: + return + loaded_scope = row + current_scope = (self.organization_id, self.course_id, self.competency_taxonomy_id) + if current_scope != loaded_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 save(self, *args, **kwargs): + """Persist this profile, after re-checking scope immutability.""" + self._check_scope_immutable() + super().save(*args, **kwargs) + self.loaded_scope = (self.organization_id, self.course_id, self.competency_taxonomy_id) + + +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. + + .. 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.PROTECT, + 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: + db_table = "openedx_learning_competencycriteria" + # Django's default pluralization of "CompetencyCriterion" is the ungrammatical + # "competency criterions"; set both explicitly, matching ADR-0002 Decision 4's + # terminology (one leaf is a criterion, the collection is CompetencyCriteria) and + # following CompetencyTaxonomy, which sets both for the same reason. + 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) diff --git a/src/openedx_learning/applets/cbe/models/learner_status.py b/src/openedx_learning/applets/cbe/models/learner_status.py new file mode 100644 index 000000000..8e4f09df8 --- /dev/null +++ b/src/openedx_learning/applets/cbe/models/learner_status.py @@ -0,0 +1,329 @@ +""" +Models tracking a learner's mastery status at each level of a competency criteria tree. + +:class:`StudentCompetencyCriteriaStatus`, :class:`StudentCompetencyCriteriaGroupStatus`, and +:class:`StudentCompetencyStatus` track the same fact -- a learner's current mastery rank -- at +the leaf (:class:`~openedx_learning.applets.cbe.models.criteria.CompetencyCriterion`), group +(:class:`~openedx_learning.applets.cbe.models.criteria.CompetencyCriteriaGroup`), and top +(:class:`~openedx_tagging.models.Tag`) levels of a criteria tree, respectively. All three share +one shape: a user, a foreign key to the node they track, a status, and caller-supplied +``created``/``modified`` timestamps. + +There is one row per learner per node, updated in place: finding a learner's current status at +any level is a lookup of that single row, not a query for the most recent of several (ADR-0003 +Decision 5). Each model accepts any status value its constraints permit; +the rules that decide *which* writes are allowed, that an automatic write may raise a status but +never lower it (ADR-0004 Decision 4), and that a staff correction may lower one (ADR-0004 +Decision 6), are enforced in the API layer, not here: by the time a write reaches these models +there is no caller context left to tell those cases apart. + +``created`` and ``modified`` are caller-supplied UTC datetimes, not automatic. A caller +performing a conditional raise must pass ``modified`` in the same ``update()`` call; there is no +``auto_now`` to do it for them, deliberately, because ``auto_now`` does not fire on +``QuerySet.update()`` and would silently leave the column stale on exactly that path. + +Each model's ``user`` foreign key is ``on_delete=models.CASCADE``, not ``PROTECT``: ``PROTECT`` +would let this library veto ``User.delete()`` platform-wide, from openedx-platform code that has +no reason to know CBE rows exist. A learner's status is a derived fact about that learner, so it +goes when they do. ``SET_NULL`` is not an option, because a null ``user_id`` would break the +``(user_id, node_id)`` uniqueness each model's in-place updates rest on. + +Each model's foreign key to the node it tracks (``criterion``, ``group``, or ``tag``) is +``on_delete=models.PROTECT``, and it is load-bearing, not defensive. #641's four foreign keys +that carry Django's collector down the criteria tree -- ``CompetencyCriteriaGroup.parent``, +``CompetencyCriteriaGroup.tag``, ``CompetencyCriterion.group``, and +``CompetencyCriterion.object_tag`` -- are ``CASCADE``, so deleting a ``Tag`` walks into its +groups and then their criteria. These ``PROTECT`` values are the only thing that stops that +walk, and they are what turns ADR-0002 Decision 7's guarantee into behavior: the delete succeeds +when no learner holds status beneath the row and raises ``ProtectedError`` when one does. Django +evaluates ``PROTECT`` on every row the collector reaches, not only the row passed to +``delete()``, which is why transitive cases work too. #675 re-implements the same predicate at +the API layer for a clean status code; this is the backstop for paths that never reach it. + +These three ``PROTECT`` values are deliberately stricter than the application-layer predicate. +ADR-0002 Decision 7, as amended for #655, names only the leaf table +``StudentCompetencyCriteriaStatus`` as determining whether a record is protected, and treats the +two roll-up tables as derived and not independently checked. The database makes no such +distinction, so a roll-up row with no leaf beneath it would also block a delete. That should not +occur, and failing closed is the right default for a backstop. + +Each model's ``status`` foreign key is also ``on_delete=models.PROTECT``, because the lookup +table it points to (:class:`CompetencyMasteryStatus`) is system-owned immutable data, seeded by +migration and never deleted. +""" +from django.conf import settings +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from openedx_django_lib.fields import manual_date_time_field +from openedx_tagging.models import Tag + +from .criteria import CompetencyCriteriaGroup, CompetencyCriterion + +__all__ = [ + "MasteryStatus", + "CompetencyMasteryStatus", + "StudentCompetencyCriteriaStatus", + "StudentCompetencyCriteriaGroupStatus", + "StudentCompetencyStatus", +] + + +class MasteryStatus(models.IntegerChoices): + """ + Ranks of competency mastery. + + Each member's value is the pinned primary key of its ``CompetencyMasteryStatus`` + row, seeded by the ``seed_competency_mastery_statuses`` data migration. + + The integer value of each member is also its rank, lowest to highest mastery. + Pinning the rank order into the stored id is what lets raising a learner's + status be written as one conditional ``UPDATE`` + (``... WHERE status_id < new_status_id``) instead of a read, a comparison in + Python, and a write: ADR-0004 Decision 4 requires that, because two concurrent + writers doing read-compare-write can each read the same old value, and the + later of the two writes then lowers what the earlier one had already raised. + + Because the ids are pinned, a new status can be added above or below the + existing three, but never between them. + + This subclasses ``IntegerChoices`` rather than ``enum.IntEnum`` so that + Django's migration serializer writes a member as a bare integer instead of an + import of this module. That keeps an already-applied migration's meaning + independent of later edits to this enum. + """ + + ATTEMPTED_NOT_DEMONSTRATED = 1, "AttemptedNotDemonstrated" + PARTIALLY_ATTEMPTED = 2, "PartiallyAttempted" + DEMONSTRATED = 3, "Demonstrated" + + +class CompetencyMasteryStatus(models.Model): + """ + Lookup table of the mastery statuses a competency can be assigned. + + This table is system-owned lookup data, seeded by the + ``seed_competency_mastery_statuses`` data migration, and is + treated as immutable configuration rather than user-authored rows + (ADR-0002 Decision 6.1). See :class:`MasteryStatus` for the pinned ids and + names of its rows. + + .. no_pii: + """ + + # ADR-0002 Decision 5 index 10. + status = models.CharField(max_length=64, unique=True) + + def __str__(self) -> str: + """User-facing string representation of a CompetencyMasteryStatus.""" + return self.status + + class Meta: + verbose_name = "Competency Mastery Status" + verbose_name_plural = "Competency Mastery Statuses" + # id order is rank order (see MasteryStatus), so a default listing of + # this table already reads lowest to highest mastery. + ordering = ("id",) + + +class StudentCompetencyCriteriaStatus(models.Model): + """ + A learner's current mastery status for one leaf ``CompetencyCriterion``. + + There is one row per learner per criterion, updated in place: finding a + learner's current status is a lookup of that single row, not a query for the + most recent of several (ADR-0003 Decision 5). + + This model accepts any status value the constraints below permit. The rules + that decide *which* writes are allowed, that an automatic write may raise a + status but never lower it, and that a staff correction may lower one, are + enforced in the API layer, not here: by the time a write reaches this model + there is no caller context left to tell those two cases apart. + + ``created`` and ``modified`` are caller-supplied UTC datetimes, not automatic. + A caller performing a conditional raise must pass ``modified`` in the same + ``update()`` call; there is no ``auto_now`` to do it for them, deliberately, + because ``auto_now`` does not fire on ``QuerySet.update()`` and would silently + leave the column stale on exactly that path. + + This table stores a user foreign key and a status value, and no personal + data of its own. + + .. no_pii: + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="competency_criteria_statuses", + ) + criterion = models.ForeignKey( + CompetencyCriterion, + db_column="competency_criteria_id", + on_delete=models.PROTECT, + related_name="student_statuses", + ) + status = models.ForeignKey( + CompetencyMasteryStatus, + on_delete=models.PROTECT, + related_name="student_criteria_statuses", + ) + created = manual_date_time_field() + modified = manual_date_time_field() + + def __str__(self) -> str: + """User-facing string representation of a StudentCompetencyCriteriaStatus.""" + return f"{self.user}: {self.criterion} = {self.status}" + + class Meta: + verbose_name = "Student Competency Criteria Status" + verbose_name_plural = "Student Competency Criteria Statuses" + constraints = [ + # ADR-0002 Decision 5 index 6. This is what makes "one row per + # learner and criterion" true, which is the precondition for the + # in-place conditional update described above: it is load-bearing, + # not a lookup optimisation. + models.UniqueConstraint( + fields=("user", "criterion"), + name="oex_learning_studentcriteriastatus_user_criterion_uniq", + ), + ] + + +class StudentCompetencyCriteriaGroupStatus(models.Model): + """ + A learner's current mastery status for one ``CompetencyCriteriaGroup`` node. + + There is one row per learner per group, updated in place: finding a + learner's current status is a lookup of that single row, not a query for the + most recent of several (ADR-0003 Decision 5). + + This model accepts any status value the constraints below permit. The rules + that decide *which* writes are allowed, that an automatic write may raise a + status but never lower it, and that a staff correction may lower one, are + enforced in the API layer, not here: by the time a write reaches this model + there is no caller context left to tell those two cases apart. + + ``created`` and ``modified`` are caller-supplied UTC datetimes, not automatic. + A caller performing a conditional raise must pass ``modified`` in the same + ``update()`` call; there is no ``auto_now`` to do it for them, deliberately, + because ``auto_now`` does not fire on ``QuerySet.update()`` and would silently + leave the column stale on exactly that path. + + This table stores a user foreign key and a status value, and no personal + data of its own. + + .. no_pii: + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="competency_criteria_group_statuses", + ) + group = models.ForeignKey( + CompetencyCriteriaGroup, + db_column="competency_criteria_group_id", + on_delete=models.PROTECT, + related_name="student_statuses", + ) + status = models.ForeignKey( + CompetencyMasteryStatus, + on_delete=models.PROTECT, + related_name="student_criteria_group_statuses", + ) + created = manual_date_time_field() + modified = manual_date_time_field() + + def __str__(self) -> str: + """User-facing string representation of a StudentCompetencyCriteriaGroupStatus.""" + return f"{self.user}: {self.group} = {self.status}" + + class Meta: + verbose_name = "Student Competency Criteria Group Status" + verbose_name_plural = "Student Competency Criteria Group Statuses" + constraints = [ + # ADR-0002 Decision 5 index 7. This is what makes "one row per + # learner and group" true, which is the precondition for the + # in-place conditional update described above: it is load-bearing, + # not a lookup optimisation. + models.UniqueConstraint( + fields=("user", "group"), + name="oex_learning_studentcriteriagroupstatus_user_group_uniq", + ), + ] + + +class StudentCompetencyStatus(models.Model): + """ + A learner's current mastery status for one competency (``Tag``). + + There is one row per learner per competency, updated in place: finding a + learner's current status is a lookup of that single row, not a query for the + most recent of several (ADR-0003 Decision 5). + + This model accepts any status value the allow-list constraint below permits. + The rules that decide *which* writes are allowed, that an automatic write may + raise a status but never lower it, and that a staff correction may lower one, + are enforced in the API layer, not here: by the time a write reaches this + model there is no caller context left to tell those two cases apart. + + ``created`` and ``modified`` are caller-supplied UTC datetimes, not automatic. + A caller performing a conditional raise must pass ``modified`` in the same + ``update()`` call; there is no ``auto_now`` to do it for them, deliberately, + because ``auto_now`` does not fire on ``QuerySet.update()`` and would silently + leave the column stale on exactly that path. + + This table stores a user foreign key and a status value, and no personal + data of its own. + + .. no_pii: + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE, + related_name="competency_mastery_statuses", + ) + tag = models.ForeignKey( + Tag, + db_column="oel_tagging_tag_id", + on_delete=models.PROTECT, + related_name="student_competency_statuses", + ) + status = models.ForeignKey( + CompetencyMasteryStatus, + on_delete=models.PROTECT, + related_name="student_competency_statuses", + ) + created = manual_date_time_field() + modified = manual_date_time_field() + + def __str__(self) -> str: + """User-facing string representation of a StudentCompetencyStatus.""" + return f"{self.user}: {self.tag} = {self.status}" + + class Meta: + verbose_name = "Student Competency Status" + verbose_name_plural = "Student Competency Statuses" + constraints = [ + # ADR-0002 Decision 5 index 8. This is what makes "one row per + # learner and competency" true, which is the precondition for the + # in-place conditional update above: it is load-bearing, not a + # lookup optimisation. + models.UniqueConstraint( + fields=("user", "tag"), + name="oex_learning_studentcompetencystatus_user_tag_uniq", + ), + # Allow list, not a negation of the excluded value: a future fourth + # status should be rejected here by default rather than silently + # permitted. + models.CheckConstraint( + condition=models.Q(status__in=(MasteryStatus.PARTIALLY_ATTEMPTED, MasteryStatus.DEMONSTRATED)), + name="oex_learning_studentcompetencystatus_status_allowed", + violation_error_message=_( + "A competency-level status may only be PartiallyAttempted or Demonstrated, " + "since it represents overall demonstration state, not an in-progress state." + ), + ), + ] diff --git a/src/openedx_learning/migrations/0002_competency_criteria.py b/src/openedx_learning/migrations/0002_competency_criteria.py new file mode 100644 index 000000000..fcdd2c817 --- /dev/null +++ b/src/openedx_learning/migrations/0002_competency_criteria.py @@ -0,0 +1,165 @@ +# Generated by Django 5.2.16 on 2026-09-01 19:00 + +import uuid + +import django.db.models.deletion +import django.db.models.functions.comparison +import django.db.models.functions.text +import simple_history.models +from django.conf import settings +from django.db import migrations, models + +import openedx_django_lib.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_tagging', '0021_remove_system_defined_add_read_only'), + ('openedx_catalog', '0001_initial'), + ('openedx_learning', '0001_initial'), + ('organizations', '0004_auto_20230727_2054'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name='competencytaxonomy', + name='taxonomy_overrides_org', + field=models.BooleanField(default=False, help_text="Resolves a tie when assigning a CompetencyRuleProfile to a CompetencyCriterion (ADR-0002 Decision 4): if both an organization-scoped profile and a taxonomy-scoped profile from this taxonomy apply to the same criterion, False (the default) assigns the organization-scoped profile, and True assigns this taxonomy's own profile instead, so it cannot be locally weakened by an organization. Nothing reads this field yet: organization-scoped CompetencyRuleProfile rows do not exist in this phase, so the tie it resolves cannot arise until they do."), + ), + migrations.CreateModel( + name='CompetencyCriteriaGroup', + 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')), + ('name', openedx_django_lib.fields.MultiCollationCharField(blank=True, db_collations={'mysql': 'utf8mb4_unicode_ci', 'sqlite': 'NOCASE'}, default='', max_length=255)), + ('ordering', models.PositiveIntegerField(default=0, help_text='Deterministic sibling evaluation sequence. Used to short-circuit evaluation and to order child scans during event-driven recomputation.')), + ('logic_operator', models.CharField(blank=True, choices=[('AND', 'And'), ('OR', 'Or')], help_text="How this group's children combine. Null until the group has children to combine.", max_length=3, null=True)), + ('course', models.ForeignKey(blank=True, help_text='The course run that scopes this criteria tree for evaluation windowing, if any.', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='competency_criteria_groups', to='openedx_catalog.courserun')), + ('parent', models.ForeignKey(blank=True, help_text='The parent CompetencyCriteriaGroup. Null means this group is a tree root.', null=True, on_delete=django.db.models.deletion.CASCADE, related_name='child_groups', to='openedx_learning.competencycriteriagroup')), + ('tag', models.ForeignKey(db_column='oel_tagging_tag_id', help_text='The competency (tag) that this criteria tree evaluates mastery of.', on_delete=django.db.models.deletion.CASCADE, related_name='competency_criteria_groups', to='oel_tagging.tag')), + ], + ), + migrations.CreateModel( + name='CompetencyRuleProfile', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('scope_code', models.GeneratedField(db_persist=True, expression=django.db.models.functions.text.Concat(models.Value('org:'), django.db.models.functions.comparison.Coalesce(django.db.models.functions.comparison.Cast(models.F('organization_id'), output_field=models.CharField(max_length=20)), models.Value('')), models.Value(',course:'), django.db.models.functions.comparison.Coalesce(django.db.models.functions.comparison.Cast(models.F('course_id'), output_field=models.CharField(max_length=20)), models.Value('')), models.Value(',taxonomy:'), django.db.models.functions.comparison.Coalesce(django.db.models.functions.comparison.Cast(models.F('competency_taxonomy_id'), output_field=models.CharField(max_length=20)), models.Value(''))), output_field=models.CharField(max_length=255))), + ('rule_type', models.CharField(choices=[('View', 'View'), ('Grade', 'Grade'), ('MasteryLevel', 'Mastery Level')], 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="Set instead of deleting a profile that's no longer wanted. Archived profiles are hidden from authoring and new associations but remain queryable, so existing criteria stay resolvable.")), + ('uuid', models.UUIDField(default=uuid.uuid4, editable=False, unique=True, verbose_name='UUID')), + ('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.PROTECT, 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.PROTECT, 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')), + ], + ), + 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=[('View', 'View'), ('Grade', 'Grade'), ('MasteryLevel', 'Mastery Level')], 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.PROTECT, related_name='criteria', to='openedx_learning.competencyruleprofile')), + ], + options={ + 'db_table': 'openedx_learning_competencycriteria', + 'verbose_name': 'Competency Criterion', + 'verbose_name_plural': 'Competency Criteria', + }, + ), + migrations.CreateModel( + name='HistoricalCompetencyCriteriaGroup', + 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')), + ('name', openedx_django_lib.fields.MultiCollationCharField(blank=True, db_collations={'mysql': 'utf8mb4_unicode_ci', 'sqlite': 'NOCASE'}, default='', max_length=255)), + ('ordering', models.PositiveIntegerField(default=0, help_text='Deterministic sibling evaluation sequence. Used to short-circuit evaluation and to order child scans during event-driven recomputation.')), + ('logic_operator', models.CharField(blank=True, choices=[('AND', 'And'), ('OR', 'Or')], help_text="How this group's children combine. Null until the group has children to combine.", max_length=3, 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)), + ('course', models.ForeignKey(blank=True, db_constraint=False, help_text='The course run that scopes this criteria tree for evaluation windowing, 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)), + ('parent', models.ForeignKey(blank=True, db_constraint=False, help_text='The parent CompetencyCriteriaGroup. Null means this group is a tree root.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='openedx_learning.competencycriteriagroup')), + ('tag', models.ForeignKey(blank=True, db_column='oel_tagging_tag_id', db_constraint=False, help_text='The competency (tag) that this criteria tree evaluates mastery of.', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='oel_tagging.tag')), + ], + options={ + 'verbose_name': 'historical competency criteria group', + 'verbose_name_plural': 'historical competency criteria groups', + 'ordering': ('-history_date', '-history_id'), + 'get_latest_by': ('history_date', 'history_id'), + }, + bases=(simple_history.models.HistoricalChanges, models.Model), + ), + 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=[('View', 'View'), ('Grade', 'Grade'), ('MasteryLevel', 'Mastery Level')], 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='HistoricalCompetencyRuleProfile', + fields=[ + ('id', models.BigIntegerField(auto_created=True, blank=True, db_index=True, verbose_name='ID')), + ('rule_type', models.CharField(choices=[('View', 'View'), ('Grade', 'Grade'), ('MasteryLevel', 'Mastery Level')], 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="Set instead of deleting a profile that's no longer wanted. Archived profiles are hidden from authoring and new associations but remain queryable, so existing criteria stay resolvable.")), + ('uuid', models.UUIDField(db_index=True, default=uuid.uuid4, editable=False, verbose_name='UUID')), + ('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.AddIndex( + model_name='competencycriteriagroup', + index=models.Index(fields=['tag', 'course'], name='openedx_lea_oel_tag_737416_idx'), + ), + migrations.AddConstraint( + model_name='competencyruleprofile', + constraint=models.UniqueConstraint(fields=('scope_code',), name='oel_cbe_ruleprofile_scope_code_uniq'), + ), + migrations.AddConstraint( + model_name='competencyruleprofile', + constraint=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.'), + ), + migrations.AddConstraint( + model_name='competencycriterion', + constraint=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/src/openedx_learning/migrations/0003_seed_default_rule_profile.py b/src/openedx_learning/migrations/0003_seed_default_rule_profile.py new file mode 100644 index 000000000..28374c49e --- /dev/null +++ b/src/openedx_learning/migrations/0003_seed_default_rule_profile.py @@ -0,0 +1,39 @@ +""" +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 + + +def seed_default_rule_profile(apps, schema_editor): + """Create the all-null-scope CompetencyRuleProfile.""" + CompetencyRuleProfile = apps.get_model('openedx_learning', 'CompetencyRuleProfile') + CompetencyRuleProfile.objects.create( + rule_type='Grade', + rule_payload={'op': 'gte', 'value': 0.8, 'scale': 'percent'}, + archived=False, + ) + + +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', '0002_competency_criteria'), + ] + + operations = [ + migrations.RunPython(seed_default_rule_profile, remove_default_rule_profile), + ] diff --git a/src/openedx_learning/migrations/0004_learner_status_models.py b/src/openedx_learning/migrations/0004_learner_status_models.py new file mode 100644 index 000000000..31f55869d --- /dev/null +++ b/src/openedx_learning/migrations/0004_learner_status_models.py @@ -0,0 +1,79 @@ +# Generated by Django 5.2.16 on 2026-09-03 20:28 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + +import openedx_django_lib.validators + + +class Migration(migrations.Migration): + + dependencies = [ + ('oel_tagging', '0021_remove_system_defined_add_read_only'), + ('openedx_learning', '0003_seed_default_rule_profile'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='CompetencyMasteryStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('status', models.CharField(max_length=64, unique=True)), + ], + options={ + 'verbose_name': 'Competency Mastery Status', + 'verbose_name_plural': 'Competency Mastery Statuses', + 'ordering': ('id',), + }, + ), + migrations.CreateModel( + name='StudentCompetencyCriteriaGroupStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('modified', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('group', models.ForeignKey(db_column='competency_criteria_group_id', on_delete=django.db.models.deletion.PROTECT, related_name='student_statuses', to='openedx_learning.competencycriteriagroup')), + ('status', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='student_criteria_group_statuses', to='openedx_learning.competencymasterystatus')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='competency_criteria_group_statuses', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Student Competency Criteria Group Status', + 'verbose_name_plural': 'Student Competency Criteria Group Statuses', + 'constraints': [models.UniqueConstraint(fields=('user', 'group'), name='oex_learning_studentcriteriagroupstatus_user_group_uniq')], + }, + ), + migrations.CreateModel( + name='StudentCompetencyCriteriaStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('modified', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('criterion', models.ForeignKey(db_column='competency_criteria_id', on_delete=django.db.models.deletion.PROTECT, related_name='student_statuses', to='openedx_learning.competencycriterion')), + ('status', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='student_criteria_statuses', to='openedx_learning.competencymasterystatus')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='competency_criteria_statuses', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Student Competency Criteria Status', + 'verbose_name_plural': 'Student Competency Criteria Statuses', + 'constraints': [models.UniqueConstraint(fields=('user', 'criterion'), name='oex_learning_studentcriteriastatus_user_criterion_uniq')], + }, + ), + migrations.CreateModel( + name='StudentCompetencyStatus', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('modified', models.DateTimeField(validators=[openedx_django_lib.validators.validate_utc_datetime])), + ('status', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='student_competency_statuses', to='openedx_learning.competencymasterystatus')), + ('tag', models.ForeignKey(db_column='oel_tagging_tag_id', on_delete=django.db.models.deletion.PROTECT, related_name='student_competency_statuses', to='oel_tagging.tag')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='competency_mastery_statuses', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Student Competency Status', + 'verbose_name_plural': 'Student Competency Statuses', + 'constraints': [models.UniqueConstraint(fields=('user', 'tag'), name='oex_learning_studentcompetencystatus_user_tag_uniq'), models.CheckConstraint(condition=models.Q(('status__in', (2, 3))), name='oex_learning_studentcompetencystatus_status_allowed', violation_error_message='A competency-level status may only be PartiallyAttempted or Demonstrated, since it represents overall demonstration state, not an in-progress state.')], + }, + ), + ] diff --git a/src/openedx_learning/migrations/0005_seed_competency_mastery_statuses.py b/src/openedx_learning/migrations/0005_seed_competency_mastery_statuses.py new file mode 100644 index 000000000..d9f503ee4 --- /dev/null +++ b/src/openedx_learning/migrations/0005_seed_competency_mastery_statuses.py @@ -0,0 +1,36 @@ +from django.db import migrations + +# These ids and strings are literals, not references to `MasteryStatus` +# (`src/openedx_learning/applets/cbe/models.py`), and must stay that way: once +# applied, a migration has to keep meaning what it meant at the time it ran, so +# it cannot depend on a constant that a later edit to that enum could change. +# See `MasteryStatus` for the names these ids correspond to. + + +def forward(apps, schema_editor): + """ + Seed the three CompetencyMasteryStatus rows, in rank order. + """ + CompetencyMasteryStatus = apps.get_model("openedx_learning", "CompetencyMasteryStatus") + CompetencyMasteryStatus.objects.get_or_create(id=1, defaults={"status": "AttemptedNotDemonstrated"}) + CompetencyMasteryStatus.objects.get_or_create(id=2, defaults={"status": "PartiallyAttempted"}) + CompetencyMasteryStatus.objects.get_or_create(id=3, defaults={"status": "Demonstrated"}) + + +def revert(apps, schema_editor): # pragma: no cover + """ + Delete the seeded CompetencyMasteryStatus rows. + """ + CompetencyMasteryStatus = apps.get_model("openedx_learning", "CompetencyMasteryStatus") + CompetencyMasteryStatus.objects.filter(id__in=(1, 2, 3)).delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ("openedx_learning", "0004_learner_status_models"), + ] + + operations = [ + migrations.RunPython(forward, revert), + ] 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..97d6c93bc --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criteria_deletion.py @@ -0,0 +1,353 @@ +""" +Delete-behavior tests for CompetencyCriteriaGroup, CompetencyRuleProfile, and CompetencyCriterion. + +Four of these nine foreign keys are `on_delete=models.CASCADE` and five are `models.PROTECT`; see +the module docstring in `openedx_learning.applets.cbe.models.criteria` for which is which and why. +""" +import pytest +from django.apps import apps +from django.db.models import ProtectedError +from organizations.api import ensure_organization +from organizations.models import Organization + +from openedx_catalog.models import CatalogCourse, 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"} + + +@pytest.fixture(name="organization") +def _organization() -> Organization: + """An Organization for use as a scope in these tests.""" + ensure_organization("Org1") + 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.""" + catalog_course = CatalogCourse.objects.create(org=organization, course_code="Python100") + return CourseRun.objects.create(catalog_course=catalog_course, run_code="Fall2026") + + +@pytest.fixture(name="competency_taxonomy") +def _competency_taxonomy() -> CompetencyTaxonomy: + """A CompetencyTaxonomy for use as a scope, and as the home taxonomy for `tag`.""" + return CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + + +@pytest.fixture(name="tag") +def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: + """A Tag, from `competency_taxonomy`, for use as the competency a criteria tree evaluates.""" + 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, for use as 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 0003.""" + return CompetencyRuleProfile.objects.get( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + ) + + +# ============================================================================================== +# One test per foreign key. The five that stayed PROTECT assert ProtectedError and inspect +# `protected_objects` to confirm which relationship actually fired: several protected +# relationships can fire on one delete (see test_rule_profile_organization_protect below for a +# real trap of that kind, where CatalogCourse.org is also PROTECT), so a bare +# `pytest.raises(ProtectedError)` would not actually prove which foreign key did the protecting. +# The four that became CASCADE assert the delete succeeds and that the referencing row is +# actually gone from the database afterward, not merely that no exception was raised, and assert +# the referencing row existed beforehand, so the "gone" assertion can't pass because a fixture +# never created it in the first place. +# ============================================================================================== + + +def test_group_parent_cascade(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_group_tag_cascade(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_group_course_protect(tag: Tag, course_run: CourseRun) -> None: + """Deleting a CourseRun that a CompetencyCriteriaGroup references via `course` raises ProtectedError.""" + group = CompetencyCriteriaGroup.objects.create(tag=tag, course=course_run) + + with pytest.raises(ProtectedError) as exc_info: + course_run.delete() + + protected = exc_info.value.protected_objects + assert any(isinstance(obj, CompetencyCriteriaGroup) and obj.pk == group.pk for obj in protected) + + +def test_rule_profile_organization_protect(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_rule_profile_course_protect(course_run: CourseRun) -> None: + """Deleting a CourseRun that a CompetencyRuleProfile references via `course` raises ProtectedError.""" + profile = CompetencyRuleProfile.objects.create( + course=course_run, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + with pytest.raises(ProtectedError) as exc_info: + course_run.delete() + + protected = exc_info.value.protected_objects + assert any(isinstance(obj, CompetencyRuleProfile) and obj.pk == profile.pk for obj in protected) + + +def test_rule_profile_competency_taxonomy_protect(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + Deleting a CompetencyTaxonomy that a CompetencyRuleProfile references via + `competency_taxonomy` raises ProtectedError naming the profile. + + Deliberately does not use the `tag` or `group` fixtures: they are not needed to isolate this + relationship. Tag.taxonomy and CompetencyCriteriaGroup.tag are both CASCADE now, so a tag and + group under this taxonomy would just be silently left untouched by the aborted delete (the + whole operation rolls back once any PROTECT fires) rather than competing for the raised + error's `protected_objects`; keeping this test to only what it needs stays the clearer read. + """ + profile = CompetencyRuleProfile.objects.create( + competency_taxonomy=competency_taxonomy, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + + with pytest.raises(ProtectedError) as exc_info: + competency_taxonomy.delete() + + protected = exc_info.value.protected_objects + assert any(isinstance(obj, CompetencyRuleProfile) and obj.pk == profile.pk for obj in protected) + + +def test_criterion_group_cascade( + 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_criterion_object_tag_cascade( + 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_criterion_rule_profile_protect( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile +) -> None: + """ + Deleting a CompetencyRuleProfile that a CompetencyCriterion references via `rule_profile` + raises ProtectedError. + """ + criterion = CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_profile=default_rule_profile + ) + + with pytest.raises(ProtectedError) as exc_info: + default_rule_profile.delete() + + protected = exc_info.value.protected_objects + assert any(isinstance(obj, CompetencyCriterion) and obj.pk == criterion.pk for obj in protected) + + +# ============================================================================================== +# Transitive deletion tests required by #641's Deletions criteria: deleting an oel_tagging.Tag, +# a CompetencyCriteriaGroup at depth, an oel_tagging.ObjectTag, or an oel_tagging.Taxonomy, when +# no learner status exists beneath the target, must succeed and take the whole referencing +# criteria tree with it. +# +# Each of these criteria also has a "raises ProtectedError when a learner status row exists +# beneath it" half. That half is NOT covered here: it needs #642's Student*Status tables, which +# do not exist on this branch, and #642's own criterion says those tests belong in the slice that +# follows #641, once those tables exist. This file does not stub, mock, or fake a status model to +# test them; their absence here is deliberate, not an oversight. +# ============================================================================================== + + +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() diff --git a/tests/openedx_learning/applets/cbe/test_criteria_models.py b/tests/openedx_learning/applets/cbe/test_criteria_models.py new file mode 100644 index 000000000..eade547c4 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_criteria_models.py @@ -0,0 +1,402 @@ +""" +Tests for CompetencyCriteriaGroup, CompetencyRuleProfile, and CompetencyCriterion. +""" +import pytest +from django.apps import apps +from django.core.exceptions import ValidationError +from django.db import connection, models, transaction +from django.db.utils import IntegrityError +from organizations.api import ensure_organization +from organizations.models import Organization + +from openedx_catalog.models import CatalogCourse, CourseRun +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyRuleProfile, + CompetencyTaxonomy, + LogicOperator, + RuleType, +) +from openedx_tagging.models import ObjectTag, Tag + +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. +_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", "value": 1.5, "scale": "percent"}, id="value_out_of_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"), + pytest.param(RuleType.VIEW, _GRADE_PAYLOAD, id="unsupported_rule_type"), +] + + +@pytest.fixture(name="organization") +def _organization() -> Organization: + """An Organization for use as a scope in these tests.""" + ensure_organization("Org1") + 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.""" + catalog_course = CatalogCourse.objects.create(org=organization, course_code="Python100") + return CourseRun.objects.create(catalog_course=catalog_course, run_code="Fall2026") + + +@pytest.fixture(name="competency_taxonomy") +def _competency_taxonomy() -> CompetencyTaxonomy: + """A CompetencyTaxonomy for use as a scope, and as the home taxonomy for `tag`.""" + return CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + + +@pytest.fixture(name="tag") +def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: + """A Tag, from `competency_taxonomy`, for use as the competency a criteria tree evaluates.""" + 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, for use as 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 0003.""" + return CompetencyRuleProfile.objects.get( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + ) + + +def test_group_tree_and_logic_operator(tag: Tag) -> None: + """ + A CompetencyCriteriaGroup's parent is null for a root and points at its parent for a child, + and logic_operator accepts AND, OR, or null (the "no children yet" state). See ADR-0002 + Decision 2. + """ + root = CompetencyCriteriaGroup.objects.create(tag=tag, logic_operator=None) + assert root.parent is None + + child_and = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root, logic_operator=LogicOperator.AND) + assert child_and.parent == root + + child_or = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root, logic_operator=LogicOperator.OR) + assert child_or.parent == root + + +def test_rule_profile_scope_check_constraint( + 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), and rejects one scoped + to any two, or to all three. See ADR-0002 Decision 3. + """ + # Free the all-null slot the seed migration (0003) occupies, so the "all null" case below can + # be tested in isolation from the uniqueness constraint on scope_code, which is a separate + # constraint covered by its own tests. + CompetencyRuleProfile.objects.filter( + organization__isnull=True, course__isnull=True, competency_taxonomy__isnull=True + ).delete() + + accepted_scopes: list[dict] = [ + {"organization": organization}, + {"course": course_run}, + {"competency_taxonomy": competency_taxonomy}, + {}, + ] + for scope_kwargs in accepted_scopes: + with transaction.atomic(): + CompetencyRuleProfile.objects.create(rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD, **scope_kwargs) + + rejected_scopes: list[dict] = [ + {"organization": organization, "course": course_run}, + {"organization": organization, "competency_taxonomy": competency_taxonomy}, + {"course": course_run, "competency_taxonomy": competency_taxonomy}, + {"organization": organization, "course": course_run, "competency_taxonomy": competency_taxonomy}, + ] + for scope_kwargs in rejected_scopes: + with pytest.raises(IntegrityError): + with transaction.atomic(): + CompetencyRuleProfile.objects.create( + rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD, **scope_kwargs + ) + + +def test_scope_code_generated_value( + organization: Organization, course_run: CourseRun, competency_taxonomy: CompetencyTaxonomy +) -> None: + """ + scope_code is derived from the three scope columns as "org:X,course:Y,taxonomy:Z", with each + segment blank when the corresponding 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_uniqueness(organization: Organization) -> None: + """ + Two 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 + ) + + +def test_scope_code_unique_constraint_has_no_condition() -> None: + """ + No UniqueConstraint on CompetencyRuleProfile carries a `condition`. A conditional + UniqueConstraint compiles to a partial index, which this project's MySQL backend does not + support: Django would only raise a non-fatal system-check warning (models.W036) and silently + skip creating the constraint, leaving uniqueness unenforced in production, while SQLite (used + for local test runs) supports partial indexes and would mask the gap. See ADR-0002 Rejected + Alternative 6. + """ + unique_constraints = [c for c in CompetencyRuleProfile._meta.constraints if isinstance(c, models.UniqueConstraint)] + assert unique_constraints + for constraint in unique_constraints: + assert constraint.condition is None + + +def test_criterion_profile_xor_override_constraint( + 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. + """ + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + CompetencyCriterion.objects.create( + group=group, object_tag=object_tag, rule_type_override=RuleType.GRADE, rule_payload_override=_GRADE_PAYLOAD + ) + + invalid_kwargs_list: list[dict] = [ + { # both set + "rule_profile": default_rule_profile, + "rule_type_override": RuleType.GRADE, + "rule_payload_override": _GRADE_PAYLOAD, + }, + {}, # neither set + {"rule_type_override": RuleType.GRADE}, # only the type override set + {"rule_payload_override": _GRADE_PAYLOAD}, # only the payload override set + ] + for kwargs in invalid_kwargs_list: + with pytest.raises(IntegrityError): + with transaction.atomic(): + CompetencyCriterion.objects.create(group=group, object_tag=object_tag, **kwargs) + + +@pytest.mark.parametrize("rule_type, payload", _INVALID_GRADE_PAYLOADS) +def test_rule_profile_full_clean_rejects_invalid_payload(rule_type: str, payload: object) -> None: + """ + full_clean() raises ValidationError for a CompetencyRuleProfile on every documented way a + rule_payload can be invalid: a bad op, a value given on a 0-100 scale instead of 0.0-1.0, a + value outside that range, a missing or extra key, a non-dict payload, a wrong scale, a + boolean value, and a rule_type with no defined payload shape yet. See ADR-0002 Decision 3. + """ + profile = CompetencyRuleProfile(rule_type=rule_type, rule_payload=payload) + with pytest.raises(ValidationError): + profile.full_clean() + + +@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_history_recorded_for_new_models_but_not_taxonomy( + organization: Organization, + group: CompetencyCriteriaGroup, + object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, + competency_taxonomy: CompetencyTaxonomy, +) -> None: + """ + HistoricalRecords() is applied to CompetencyCriteriaGroup, CompetencyRuleProfile, and + CompetencyCriterion: each is registered in the app registry under its expected + Historical* name, and editing an instance writes a row there. CompetencyTaxonomy has no + history at all. See ADR-0003 Decisions 1 and 2. + + Historical* models are looked up via the app registry rather than the `.history` attribute + because simple_history installs `.history` as a runtime descriptor with no type stubs, which + mypy cannot type; apps.get_model() returns something mypy can call `.objects` on. + """ + historical_group = apps.get_model("openedx_learning", "HistoricalCompetencyCriteriaGroup") + historical_profile = apps.get_model("openedx_learning", "HistoricalCompetencyRuleProfile") + historical_criterion = apps.get_model("openedx_learning", "HistoricalCompetencyCriterion") + + group.name = "Poetry Mastery" + group.save() + assert historical_group.objects.filter(id=group.pk).count() == 2 + + 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 + + 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 + + assert not hasattr(competency_taxonomy, "history") + + +def test_scope_immutability(organization: Organization, course_run: CourseRun) -> None: + """ + Changing a CompetencyRuleProfile's scope (organization, course, or competency_taxonomy) after + creation raises ValidationError on save(). Criteria store the profile id they were assigned + and never re-resolve it, so letting the scope change would silently re-govern every criterion + already pointing at this profile. This guard catches instance.save() but not a bulk + QuerySet.update(). See ADR-0002 Decision 3. + """ + profile = CompetencyRuleProfile.objects.create( + organization=organization, rule_type=RuleType.GRADE, rule_payload=_GRADE_PAYLOAD + ) + profile.organization = None + profile.course = course_run + with pytest.raises(ValidationError): + profile.save() + + +def test_scope_immutability_with_deferred_load(organization: Organization, organization2: Organization) -> None: + """ + Scope immutability is enforced even when the profile was loaded with .only()/.defer() and so + never had a complete `loaded_scope` captured by from_db(). Without falling back to read the + persisted scope back from the database, this edit would go through unchecked, because + _check_scope_immutable() would find `loaded_scope` still None and skip the comparison + entirely. + + Uses a second organization rather than setting the scope to None: setting it to None would + make scope_code collide with the seeded system-default row, so the unique constraint would + raise IntegrityError instead of the scope guard, and the test would pass for the wrong + reason. Do not "simplify" this back to None. + """ + 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) + assert deferred.loaded_scope is None + + deferred.organization = organization2 + with pytest.raises(ValidationError): + deferred.save() + + +def test_adr_indexes_present() -> None: + """ + The real database tables carry the ADR-0002 Decision 5 indexes this migration is responsible + for: positions 1, 2, 4, 5 (all covering indexes), and 9 (unique). Positions 2, 4, and 5 come + from Django's automatic per-ForeignKey index rather than an explicit models.Index; this test + introspects the database, not the model, so it holds regardless of which mechanism produced + the index. Positions 3, 6, 7, 8, and 10 belong to tables this migration doesn't create. + """ + with connection.cursor() as cursor: + group_constraints = connection.introspection.get_constraints(cursor, CompetencyCriteriaGroup._meta.db_table) + criterion_constraints = connection.introspection.get_constraints(cursor, CompetencyCriterion._meta.db_table) + profile_constraints = connection.introspection.get_constraints(cursor, CompetencyRuleProfile._meta.db_table) + + def is_indexed(constraints: dict, columns: list[str]) -> bool: + # Compare the ordered column list, not a set: column order is the whole point of a + # composite index. An index on (course_id, oel_tagging_tag_id) would satisfy a set + # comparison against ADR index 1 just as well as (oel_tagging_tag_id, course_id), but + # only the tag-first ordering also serves tag-only lookups. + return any(c["columns"] == columns and c["index"] for c in constraints.values()) + + # 1: CompetencyCriteriaGroup(tag, course), the one explicit composite index. + assert is_indexed(group_constraints, ["oel_tagging_tag_id", "course_id"]) + # 2: CompetencyCriteriaGroup(parent). + assert is_indexed(group_constraints, ["parent_id"]) + # 4: CompetencyCriteria(object_tag). + assert is_indexed(criterion_constraints, ["oel_tagging_objecttag_id"]) + # 5: CompetencyCriteria(group). + assert is_indexed(criterion_constraints, ["competency_criteria_group_id"]) + # 9: CompetencyRuleProfile(scope_code), unique. + assert any( + set(c["columns"]) == {"scope_code"} and c["unique"] for c in profile_constraints.values() + ) + + +def test_seeded_default_rule_profile_exists() -> 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 diff --git a/tests/openedx_learning/applets/cbe/test_mastery.py b/tests/openedx_learning/applets/cbe/test_mastery.py new file mode 100644 index 000000000..e523c7881 --- /dev/null +++ b/tests/openedx_learning/applets/cbe/test_mastery.py @@ -0,0 +1,726 @@ +""" +Tests for the mastery status models: MasteryStatus, CompetencyMasteryStatus, +StudentCompetencyCriteriaStatus, StudentCompetencyCriteriaGroupStatus, and +StudentCompetencyStatus. + +The deletion tests below are this ticket's headline responsibility, not an afterthought. +#641's foreign keys that carry Django's collector down a criteria tree +(`CompetencyCriteriaGroup.parent`, `CompetencyCriteriaGroup.tag`, `CompetencyCriterion.group`, +`CompetencyCriterion.object_tag`) are all CASCADE, and +`tests/openedx_learning/applets/cbe/test_criteria_deletion.py` only asserts that CASCADE half of +each transitive case, because the tables that stop the walk -- this module's three PROTECT +foreign keys (`StudentCompetencyCriteriaStatus.criterion`, +`StudentCompetencyCriteriaGroupStatus.group`, `StudentCompetencyStatus.tag`) -- did not exist on +that branch. Every ProtectedError half of those transitive cases is asserted here instead, +alongside the succeeding (no status beneath the target) half, so each case is proven both ways. +""" +from datetime import datetime, timezone + +import pytest +from django.contrib.auth import get_user_model +from django.core.exceptions import ValidationError +from django.db import IntegrityError, transaction +from django.db.models import ProtectedError + +from openedx_learning.models import ( + CompetencyCriteriaGroup, + CompetencyCriterion, + CompetencyMasteryStatus, + CompetencyRuleProfile, + CompetencyTaxonomy, + MasteryStatus, + StudentCompetencyCriteriaGroupStatus, + StudentCompetencyCriteriaStatus, + StudentCompetencyStatus, +) +from openedx_tagging.models import ObjectTag, Tag + +pytestmark = pytest.mark.django_db + + +@pytest.fixture(name="competency_taxonomy") +def _competency_taxonomy() -> CompetencyTaxonomy: + """A CompetencyTaxonomy for use as a scope, and as the home taxonomy for `tag`.""" + return CompetencyTaxonomy.objects.create(name="Nursing", export_id="nursing-v1") + + +@pytest.fixture(name="tag") +def _tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: + """A Tag, from `competency_taxonomy`, for use as the competency a criteria tree evaluates.""" + 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, for use as 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 0003.""" + return CompetencyRuleProfile.objects.get( + organization__isnull=True, + course__isnull=True, + competency_taxonomy__isnull=True, + ) + + +@pytest.fixture(name="user") +def _user(): + """ + Create a single learner for use in these tests. + + Deliberately unannotated: the user model is swappable, so this library must not + name a concrete one (edx-lint enforces that as `imported-auth-user`). + """ + return get_user_model().objects.create(username="learner") + + +@pytest.fixture(name="now") +def _now() -> datetime: + """A single UTC timestamp shared by writes in a test.""" + return datetime.now(timezone.utc) + + +@pytest.fixture(name="other_tag") +def _other_tag(competency_taxonomy: CompetencyTaxonomy) -> Tag: + """A second Tag, from the same taxonomy as `tag`, for a second competency-level status.""" + return Tag.objects.create(taxonomy=competency_taxonomy, value="Decimals") + + +@pytest.fixture(name="other_object_tag") +def _other_object_tag(competency_taxonomy: CompetencyTaxonomy, tag: Tag) -> ObjectTag: + """A second ObjectTag on `tag`, distinct from `object_tag`, for a second leaf criterion.""" + return ObjectTag.objects.create( + object_id="block-v1:Org1+Python100+Fall2026+problem+p2", + taxonomy=competency_taxonomy, + tag=tag, + ) + + +@pytest.fixture(name="criterion") +def _criterion( + group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile, +) -> CompetencyCriterion: + """A leaf CompetencyCriterion under `group`, using the system-default rule profile.""" + return CompetencyCriterion.objects.create(group=group, object_tag=object_tag, rule_profile=default_rule_profile) + + +@pytest.fixture(name="child_group") +def _child_group(tag: Tag, group: CompetencyCriteriaGroup) -> CompetencyCriteriaGroup: + """A child CompetencyCriteriaGroup under `group`, for a tree the collector must walk two levels down.""" + return CompetencyCriteriaGroup.objects.create(tag=tag, parent=group) + + +@pytest.fixture(name="child_criterion") +def _child_criterion( + child_group: CompetencyCriteriaGroup, object_tag: ObjectTag, default_rule_profile: CompetencyRuleProfile, +) -> CompetencyCriterion: + """A leaf CompetencyCriterion under `child_group`, for exercising a delete at depth.""" + return CompetencyCriterion.objects.create( + group=child_group, object_tag=object_tag, rule_profile=default_rule_profile + ) + + +# ============================================================================================== +# The lookup table and its rank ordering. +# ============================================================================================== + + +def test_seed_produces_three_rows_in_rank_order() -> None: + """ + The seed_competency_mastery_statuses data migration creates exactly the three + CompetencyMasteryStatus rows, with the pinned ids from MasteryStatus, and ids ascend in + rank (lowest to highest mastery). + """ + rows = list(CompetencyMasteryStatus.objects.order_by("id")) + assert [row.id for row in rows] == [ + MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + MasteryStatus.PARTIALLY_ATTEMPTED, + MasteryStatus.DEMONSTRATED, + ] + assert [row.status for row in rows] == [ + MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED.label, + MasteryStatus.PARTIALLY_ATTEMPTED.label, + MasteryStatus.DEMONSTRATED.label, + ] + + +def test_status_is_unique_on_lookup_table() -> None: + """ + CompetencyMasteryStatus.status is unique: a second row with a status string + that already exists raises IntegrityError. + """ + with pytest.raises(IntegrityError), transaction.atomic(): + CompetencyMasteryStatus.objects.create(status=MasteryStatus.DEMONSTRATED.label) + + +# ============================================================================================== +# The monotone conditional-update comparison, on the leaf level (where ADR-0004 Decision 1 has an +# automatic raise actually land) and on the top level (the backup's original coverage). +# ============================================================================================== + + +def test_conditional_raise_is_a_single_no_op_or_effective_update_on_leaf( + user, criterion: CompetencyCriterion, now: datetime, +) -> None: + """ + The monotone comparison works in a single statement on StudentCompetencyCriteriaStatus, the + leaf level. ADR-0004 Decision 1 writes the leaf synchronously with the learner's grade, so + this is where an automatic raise actually lands; the top-level equivalent below exercises the + same UPDATE shape one level up. + """ + scs = StudentCompetencyCriteriaStatus.objects.create( + user=user, + criterion=criterion, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + + # Already at the top rank: an attempted raise to a lower/equal rank changes nothing. + changed = StudentCompetencyCriteriaStatus.objects.filter( + user=user, criterion=criterion, status_id__lt=MasteryStatus.PARTIALLY_ATTEMPTED, + ).update(status_id=MasteryStatus.PARTIALLY_ATTEMPTED, modified=now) + assert changed == 0 + scs.refresh_from_db() + assert scs.status_id == MasteryStatus.DEMONSTRATED + + # Lower the stored status, then confirm the same shape raises it exactly once. + StudentCompetencyCriteriaStatus.objects.filter(pk=scs.pk).update(status_id=MasteryStatus.PARTIALLY_ATTEMPTED) + changed = StudentCompetencyCriteriaStatus.objects.filter( + user=user, criterion=criterion, status_id__lt=MasteryStatus.DEMONSTRATED, + ).update(status_id=MasteryStatus.DEMONSTRATED, modified=now) + assert changed == 1 + scs.refresh_from_db() + assert scs.status_id == MasteryStatus.DEMONSTRATED + + +def test_conditional_raise_is_a_single_no_op_or_effective_update(user, tag: Tag, now: datetime) -> None: + """ + The monotone comparison works in a single statement on StudentCompetencyStatus, the top + level: a conditional UPDATE guarded by status_id__lt is a no-op against an already-higher + status, and is the one write that takes effect when the stored status is lower. + """ + scs = StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + + # Already at the top rank: an attempted raise to a lower/equal rank changes nothing. + changed = StudentCompetencyStatus.objects.filter( + user=user, tag=tag, status_id__lt=MasteryStatus.PARTIALLY_ATTEMPTED, + ).update(status_id=MasteryStatus.PARTIALLY_ATTEMPTED, modified=now) + assert changed == 0 + scs.refresh_from_db() + assert scs.status_id == MasteryStatus.DEMONSTRATED + + # Lower the stored status, then confirm the same shape raises it exactly once. + StudentCompetencyStatus.objects.filter(pk=scs.pk).update(status_id=MasteryStatus.PARTIALLY_ATTEMPTED) + changed = StudentCompetencyStatus.objects.filter( + user=user, tag=tag, status_id__lt=MasteryStatus.DEMONSTRATED, + ).update(status_id=MasteryStatus.DEMONSTRATED, modified=now) + assert changed == 1 + scs.refresh_from_db() + assert scs.status_id == MasteryStatus.DEMONSTRATED + + +# ============================================================================================== +# The allow-list check constraint, which applies only to StudentCompetencyStatus, and its +# boundary: the other two models accept all three MasteryStatus values. +# ============================================================================================== + + +def test_attempted_not_demonstrated_rejected_on_create(user, tag: Tag, now: datetime) -> None: + """ + The allow-list constraint rejects AttemptedNotDemonstrated on a direct create(). + """ + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + created=now, + modified=now, + ) + + +def test_attempted_not_demonstrated_rejected_on_bulk_create(user, tag: Tag, now: datetime) -> None: + """ + The allow-list constraint rejects AttemptedNotDemonstrated on bulk_create(), which + bypasses Model.save() and so would otherwise skip any Python-side validation. + """ + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyStatus.objects.bulk_create([ + StudentCompetencyStatus( + user=user, + tag=tag, + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + created=now, + modified=now, + ) + ]) + + +def test_attempted_not_demonstrated_rejected_on_queryset_update(user, tag: Tag, now: datetime) -> None: + """ + The allow-list constraint also rejects AttemptedNotDemonstrated on QuerySet.update(), + the same write path the conditional raise above uses. + """ + scs = StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=now, + modified=now, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyStatus.objects.filter(pk=scs.pk).update( + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + ) + + +def test_demonstrated_and_partially_attempted_both_accepted( + user, tag: Tag, other_tag: Tag, now: datetime, +) -> None: + """ + Both statuses the allow-list permits, Demonstrated and PartiallyAttempted, are + accepted on create(). + """ + demonstrated = StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + partially_attempted = StudentCompetencyStatus.objects.create( + user=user, + tag=other_tag, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=now, + modified=now, + ) + assert demonstrated.status_id == MasteryStatus.DEMONSTRATED + assert partially_attempted.status_id == MasteryStatus.PARTIALLY_ATTEMPTED + + +def test_leaf_and_group_status_accept_attempted_not_demonstrated( + user, criterion: CompetencyCriterion, group: CompetencyCriteriaGroup, now: datetime, +) -> None: + """ + Unlike StudentCompetencyStatus, neither StudentCompetencyCriteriaStatus nor + StudentCompetencyCriteriaGroupStatus carries a check constraint on `status`: all three + MasteryStatus values, including AttemptedNotDemonstrated, are valid at the leaf and group + levels, because an in-progress state is meaningful there. The restriction to + PartiallyAttempted/Demonstrated is specific to the top-level competency status, which + represents overall demonstration, not an in-progress state. + """ + leaf = StudentCompetencyCriteriaStatus.objects.create( + user=user, + criterion=criterion, + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + created=now, + modified=now, + ) + group_status = StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, + group=group, + status_id=MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED, + created=now, + modified=now, + ) + assert leaf.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + assert group_status.status_id == MasteryStatus.ATTEMPTED_NOT_DEMONSTRATED + + +# ============================================================================================== +# One row per learner and node (ADR-0002 Decision 5 indexes 6, 7, and 8), one test per model. +# ============================================================================================== + + +def test_one_row_per_user_and_criterion_but_multiple_criteria_per_user( + user, criterion: CompetencyCriterion, other_object_tag: ObjectTag, + default_rule_profile: CompetencyRuleProfile, now: datetime, +) -> None: + """ + The (user, criterion) unique constraint rejects a second row for the same pair, but the + same learner may hold a status for a different criterion. + """ + # Same group as `criterion`, read off it rather than taking a separate `group` fixture + # argument, which would push this function over pylint's max-args. + other_criterion = CompetencyCriterion.objects.create( + group=criterion.group, object_tag=other_object_tag, rule_profile=default_rule_profile + ) + StudentCompetencyCriteriaStatus.objects.create( + user=user, + criterion=criterion, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=now, + modified=now, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyCriteriaStatus.objects.create( + user=user, + criterion=criterion, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + + # A different criterion for the same user is a different (user, criterion) pair, so it's allowed. + other = StudentCompetencyCriteriaStatus.objects.create( + user=user, + criterion=other_criterion, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + assert other.criterion_id == other_criterion.pk + + +def test_one_row_per_user_and_group_but_multiple_groups_per_user( + user, tag: Tag, group: CompetencyCriteriaGroup, now: datetime, +) -> None: + """ + The (user, group) unique constraint rejects a second row for the same pair, but the + same learner may hold a status for a different group. + """ + other_group = CompetencyCriteriaGroup.objects.create(tag=tag) + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, + group=group, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=now, + modified=now, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, + group=group, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + + # A different group for the same user is a different (user, group) pair, so it's allowed. + other = StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, + group=other_group, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + assert other.group_id == other_group.pk + + +def test_one_row_per_user_and_tag_but_multiple_tags_per_user( + user, tag: Tag, other_tag: Tag, now: datetime, +) -> None: + """ + The (user, tag) unique constraint rejects a second row for the same pair, but + the same learner may hold a status for a different tag. + """ + StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=now, + modified=now, + ) + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + + # A different tag for the same user is a different (user, tag) pair, so it's allowed. + other = StudentCompetencyStatus.objects.create( + user=user, + tag=other_tag, + status_id=MasteryStatus.DEMONSTRATED, + created=now, + modified=now, + ) + assert other.tag_id == other_tag.pk + + +# ============================================================================================== +# created/modified: caller-supplied, required, and UTC-only. The field is defined identically +# (manual_date_time_field()) on all three models, so one model's coverage stands for all three. +# ============================================================================================== + + +def test_created_and_modified_are_required_and_must_be_utc(user, tag: Tag, now: datetime) -> None: + """ + created and modified are caller-supplied, not automatic: omitting either on create() + raises IntegrityError (NOT NULL, since there is no auto_now/auto_now_add default), and + passing a naive (non-UTC) datetime fails full_clean() with ValidationError, which is + the UTC validator manual_date_time_field() carries. + """ + with pytest.raises(IntegrityError), transaction.atomic(): + StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.DEMONSTRATED, + ) + + naive_now = datetime.now() # deliberately naive, to trigger the UTC validator + scs = StudentCompetencyStatus( + user=user, + tag=tag, + status_id=MasteryStatus.DEMONSTRATED, + created=naive_now, + modified=now, + ) + with pytest.raises(ValidationError): + scs.full_clean() + + +def test_conditional_raise_can_carry_modified_without_touching_created(user, tag: Tag, now: datetime) -> None: + """ + A conditional raise can carry `modified` in the same UPDATE while leaving `created` + untouched, showing the mandated write path can keep `modified` honest without an + auto_now, which is why the field is caller-supplied rather than automatic. + """ + created_at = now + scs = StudentCompetencyStatus.objects.create( + user=user, + tag=tag, + status_id=MasteryStatus.PARTIALLY_ATTEMPTED, + created=created_at, + modified=created_at, + ) + + later = datetime.now(timezone.utc) + changed = StudentCompetencyStatus.objects.filter( + user=user, tag=tag, status_id__lt=MasteryStatus.DEMONSTRATED, + ).update(status_id=MasteryStatus.DEMONSTRATED, modified=later) + assert changed == 1 + + scs.refresh_from_db() + assert scs.status_id == MasteryStatus.DEMONSTRATED + assert scs.modified == later + assert scs.created == created_at + + +# ============================================================================================== +# No history package on any of the three learner status models (ADR-0003 Decision 5), unlike +# #641's criteria definition models. +# ============================================================================================== + + +def test_no_history_package_applied() -> None: + """ + None of the three learner status models has a `history` attribute. + + ADR-0003 Decision 1 gives django-simple-history to the three criteria definition models + (CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile) only. ADR-0003 Decision + 5 leaves how learner status history is retained undecided, so no history package is applied + to any of the three models here. + """ + assert not hasattr(StudentCompetencyCriteriaStatus, "history") + assert not hasattr(StudentCompetencyCriteriaGroupStatus, "history") + assert not hasattr(StudentCompetencyStatus, "history") + + +# ============================================================================================== +# Deletion. Each transitive case is proven both ways: ProtectedError when a status row exists +# somewhere beneath the target, and a full cascade of the criteria tree when none does. Across +# the tag/group/taxonomy cases, the blocking status row is deliberately placed at a different +# level each time (a competency status directly on the tag, a group status under a group, and a +# leaf status two levels below a taxonomy) so the suite as a whole exercises all three PROTECT +# foreign keys, not just one of them repeatedly. +# ============================================================================================== + + +def test_tag_delete_protected_by_competency_status_on_tag( + tag: Tag, group: CompetencyCriteriaGroup, criterion: CompetencyCriterion, user, now: datetime, +) -> None: + """ + Deleting a Tag raises ProtectedError when a StudentCompetencyStatus row references it + directly via `tag` (`on_delete=models.PROTECT`). This is the shallowest of the transitive + cases: the collector finds the blocking row on the tag itself, with no CASCADE hop needed + first. + """ + StudentCompetencyStatus.objects.create( + user=user, tag=tag, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + tag.delete() + + assert Tag.objects.filter(pk=tag.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_tag_delete_with_no_status_cascades_whole_criteria_tree( + tag: Tag, group: CompetencyCriteriaGroup, criterion: CompetencyCriterion, +) -> None: + """ + Deleting a Tag with no learner status beneath it succeeds and cascades away the whole + criteria tree hanging off it: Tag -> CompetencyCriteriaGroup.tag (CASCADE) -> + CompetencyCriterion.group (CASCADE). + """ + 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_protected_by_leaf_status( + group: CompetencyCriteriaGroup, child_group: CompetencyCriteriaGroup, child_criterion: CompetencyCriterion, + user, now: datetime, +) -> None: + """ + Deleting a CompetencyCriteriaGroup raises ProtectedError when a + StudentCompetencyCriteriaStatus row exists on a criterion two levels below it: + `group` -> `child_group` via CompetencyCriteriaGroup.parent (CASCADE), then `child_group` -> + `child_criterion` via CompetencyCriterion.group (CASCADE), then `child_criterion` -> the + status row via StudentCompetencyCriteriaStatus.criterion (PROTECT). Deleting `group` makes + Django's collector walk both CASCADE hops before it reaches the PROTECT that stops it. + """ + StudentCompetencyCriteriaStatus.objects.create( + user=user, criterion=child_criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + group.delete() + + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=child_group.pk).exists() + assert CompetencyCriterion.objects.filter(pk=child_criterion.pk).exists() + + +def test_group_delete_at_depth_with_no_status_cascades_descendants( + group: CompetencyCriteriaGroup, child_group: CompetencyCriteriaGroup, child_criterion: CompetencyCriterion, +) -> None: + """ + Deleting a CompetencyCriteriaGroup with no status rows beneath it succeeds and cascades away + its child group and that child's criterion, via the same two CASCADE hops + (CompetencyCriteriaGroup.parent, then CompetencyCriterion.group) that the protected case + above walks before hitting a PROTECT. + """ + group.delete() + + assert not CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert not CompetencyCriteriaGroup.objects.filter(pk=child_group.pk).exists() + assert not CompetencyCriterion.objects.filter(pk=child_criterion.pk).exists() + + +def test_object_tag_delete_protected_by_leaf_status( + object_tag: ObjectTag, criterion: CompetencyCriterion, user, now: datetime, +) -> None: + """ + Deleting an ObjectTag raises ProtectedError when a StudentCompetencyCriteriaStatus row + references its criterion, reached transitively: ObjectTag -> CompetencyCriterion.object_tag + (CASCADE) -> the status row via StudentCompetencyCriteriaStatus.criterion (PROTECT). + """ + StudentCompetencyCriteriaStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + object_tag.delete() + + assert ObjectTag.objects.filter(pk=object_tag.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_object_tag_delete_with_no_status_cascades_criterion( + object_tag: ObjectTag, criterion: CompetencyCriterion, +) -> None: + """ + Deleting an ObjectTag with no learner status on its criterion succeeds and cascades that + criterion away via CompetencyCriterion.object_tag (CASCADE). + """ + object_tag.delete() + + assert not CompetencyCriterion.objects.filter(pk=criterion.pk).exists() + + +def test_taxonomy_delete_protected_by_group_status( + competency_taxonomy: CompetencyTaxonomy, tag: Tag, group: CompetencyCriteriaGroup, user, now: datetime, +) -> None: + """ + Deleting a CompetencyTaxonomy raises ProtectedError when a + StudentCompetencyCriteriaGroupStatus row exists on a group under one of its tags, reached + transitively: CompetencyTaxonomy -> Tag via Tag.taxonomy (CASCADE, in openedx_tagging) -> + `group` via CompetencyCriteriaGroup.tag (CASCADE) -> the status row via + StudentCompetencyCriteriaGroupStatus.group (PROTECT). + """ + StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + with pytest.raises(ProtectedError), transaction.atomic(): + competency_taxonomy.delete() + + assert CompetencyTaxonomy.objects.filter(pk=competency_taxonomy.pk).exists() + assert Tag.objects.filter(pk=tag.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + + +def test_taxonomy_delete_with_no_status_cascades_every_tag_and_its_criteria( + competency_taxonomy: CompetencyTaxonomy, tag: Tag, group: CompetencyCriteriaGroup, + criterion: CompetencyCriterion, +) -> None: + """ + Deleting a CompetencyTaxonomy with no learner status beneath any of its tags succeeds and + cascades away every tag and its criteria tree: CompetencyTaxonomy -> Tag (CASCADE) -> + CompetencyCriteriaGroup.tag (CASCADE) -> CompetencyCriterion.group (CASCADE). This is the + same chain case 13 (`test_tag_delete_with_no_status_cascades_whole_criteria_tree`) proves, + reached transitively from one level higher. + """ + 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() + + +def test_user_delete_removes_status_across_all_three_models( + user, tag: Tag, group: CompetencyCriteriaGroup, criterion: CompetencyCriterion, now: datetime, +) -> None: + """ + Deleting a User cascades to that learner's status rows in all three models + (`on_delete=models.CASCADE` on each model's `user` foreign key), while leaving the + competency definitions themselves untouched: the status is a derived fact about the + learner, so it goes when they do, but the criteria tree it was measuring does not. + """ + leaf = StudentCompetencyCriteriaStatus.objects.create( + user=user, criterion=criterion, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + group_status = StudentCompetencyCriteriaGroupStatus.objects.create( + user=user, group=group, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + competency_status = StudentCompetencyStatus.objects.create( + user=user, tag=tag, status_id=MasteryStatus.DEMONSTRATED, created=now, modified=now, + ) + + user.delete() + + assert not StudentCompetencyCriteriaStatus.objects.filter(pk=leaf.pk).exists() + assert not StudentCompetencyCriteriaGroupStatus.objects.filter(pk=group_status.pk).exists() + assert not StudentCompetencyStatus.objects.filter(pk=competency_status.pk).exists() + assert Tag.objects.filter(pk=tag.pk).exists() + assert CompetencyCriteriaGroup.objects.filter(pk=group.pk).exists() + assert CompetencyCriterion.objects.filter(pk=criterion.pk).exists() diff --git a/tests/openedx_learning/applets/cbe/test_models.py b/tests/openedx_learning/applets/cbe/test_models.py index b38c25928..7b113cb8a 100644 --- a/tests/openedx_learning/applets/cbe/test_models.py +++ b/tests/openedx_learning/applets/cbe/test_models.py @@ -44,6 +44,14 @@ def test_plain_taxonomy_has_no_competencytaxonomy() -> None: _ = plain.competencytaxonomy +def test_taxonomy_overrides_org_defaults_false(competency_taxonomy: CompetencyTaxonomy) -> None: + """ + taxonomy_overrides_org defaults to False, so an organization-scoped profile wins the + contested case by default until an author opts a taxonomy out. See ADR-0002 Decision 1. + """ + assert competency_taxonomy.taxonomy_overrides_org is False + + def test_delete_cascades_both_directions() -> None: """ Deleting the parent Taxonomy removes the CompetencyTaxonomy row, and deleting