diff --git a/src/openedx_learning/applets/cbe/api.py b/src/openedx_learning/applets/cbe/api.py index 87552fee7..abe29ebf8 100644 --- a/src/openedx_learning/applets/cbe/api.py +++ b/src/openedx_learning/applets/cbe/api.py @@ -7,7 +7,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from decimal import Decimal -from typing import Any, Sequence +from typing import Any, Iterable, Sequence from django.core.exceptions import NON_FIELD_ERRORS, PermissionDenied, ValidationError from django.db import connection, transaction @@ -19,12 +19,13 @@ from openedx_catalog.api import get_course_run from openedx_catalog.models_api import CourseRun -from openedx_tagging.api import create_taxonomy, get_object_tags, tag_object +from openedx_tagging.api import create_taxonomy, get_ancestor_tags, get_descendant_tags, get_object_tags, tag_object from openedx_tagging.models import ObjectTag, Tag, Taxonomy from openedx_tagging.rules import ObjectTagPermissionItem, UserType from .data import GradedObjectScore from .models import ( + CompetencyCourseRun, CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, @@ -45,6 +46,7 @@ "create_competency_taxonomy", "get_competency_criteria_tree", "get_competency_rule_profiles", + "get_courses_with_relative_criteria", "is_competency_taxonomy", "record_graded_object_statuses", "resolve_competency_tag", @@ -260,6 +262,58 @@ def resolve_supplied_leaf_group(group_id: int, tag: Tag, course_run: CourseRun) return group +def get_courses_with_relative_criteria( + tag_ids: Iterable[int], course: CourseRun | None = None, +) -> dict[CourseRun, Tag]: + """ + Return every course already blocked for one of ``tag_ids`` by an existing CompetencyCriterion. + + Maps each conflicting CourseRun to the lowest-id tag (of ``tag_ids``) with a criterion there, + narrowed to ``course`` when given. Archived criteria and criteria under an archived group are ignored. + """ + groups: QuerySet[CompetencyCriteriaGroup, CompetencyCriteriaGroup] = CompetencyCriteriaGroup.objects.filter( + tag_id__in=tag_ids, + archived=False, + parent__archived=False, + parent__parent__archived=False, + # Only a leaf's parent carries a course, so this skips criteria misplaced on other groups. + parent__course__isnull=False, + criteria__archived=False, + ) + if course is not None: + groups = groups.filter(parent__course=course) + result: dict[CourseRun, Tag] = {} + for group in groups.select_related("tag", "parent__course").distinct().order_by("tag_id"): + assert group.parent is not None, f"CompetencyCriteriaGroup {group.id} has no parent" + assert group.parent.course is not None, f"CompetencyCriteriaGroup {group.id}'s parent has no course" + result.setdefault(group.parent.course, group.tag) + return result + + +def _validate_containment(group: CompetencyCriteriaGroup, course: CourseRun) -> None: + """ + Raise ValidationError (keyed "tag_id") if ``group.tag`` conflicts with an existing criterion. + + A conflict is a taxonomy ancestor or descendant of ``group.tag`` that already has a + CompetencyCriterion in ``course``. + """ + tag = group.tag + relative_ids = ( + set(get_ancestor_tags(tag).values_list("id", flat=True)) + | set(get_descendant_tags(tag).values_list("id", flat=True)) + ) + if not relative_ids: + return + conflicting_tag = get_courses_with_relative_criteria(relative_ids, course=course).get(course) + if conflicting_tag is not None: + raise ValidationError({ + "tag_id": _( + "Tag '{tag_value}' already has a criterion in course {course_key}, and is a taxonomy " + "ancestor or descendant of this criterion's tag." + ).format(tag_value=conflicting_tag.value, course_key=course.course_key), + }) + + def _get_applicable_rule_profile(taxonomy_id: int) -> CompetencyRuleProfile | None: """ Return the live profile ADR-0002 Decision 4 assigns a criterion in ``taxonomy_id``, or None if there is none. @@ -281,6 +335,8 @@ def _get_applicable_rule_profile(taxonomy_id: int) -> CompetencyRuleProfile | No def create_competency_criterion( group: CompetencyCriteriaGroup, object_id: str, + course_run: CourseRun, + *, rule_profile_id: int | None = None, rule_type_override: str | None = None, rule_payload_override: dict | None = None, @@ -300,10 +356,16 @@ def create_competency_criterion( else the live system default, rather than persisting null: ADR-0002 Decision 4 always resolves to a concrete profile, and the model's own xor-override constraint forbids leaving all three fields null. + + ``course_run`` is ``group``'s course. Must run inside a transaction holding the tag and + course run locks that associate_competency_criterion() takes, so the dominance check below + can't race another create. """ tag = group.tag # tag.taxonomy_id is nullable at the model level; already guaranteed set by the caller. assert tag.taxonomy_id is not None + _validate_containment(group, course_run) + existing_values = [existing_tag.value for existing_tag in get_object_tags(object_id, taxonomy_id=tag.taxonomy_id)] if tag.value not in existing_values: existing_values.append(tag.value) @@ -319,8 +381,6 @@ def create_competency_criterion( raise CompetencyRuleProfile.DoesNotExist("No live rule profile applies to this criterion.") rule_profile_id = profile.id - # Parent-competency dominance/containment validation belongs here, before creation. - return CompetencyCriterion.objects.create( group=group, object_tag=object_tag, @@ -433,8 +493,14 @@ def associate_competency_criterion( _validate_rule_fields(rule_profile_id, rule_type_override, rule_payload_override) with transaction.atomic(): + # Lock order is always the tag, then the course run, so two creates can't deadlock. # Serializes the duplicate check and root creation; MySQL has no partial unique index for one root per tag. Tag.objects.select_for_update().get(pk=tag.pk) + # Serializes the per-course dominance check on CBE's own row, not the catalog's CourseRun. + # Created first, since a locking read of a row that doesn't exist yet locks nothing; get_or_create() + # resolves two first creates racing on the primary key. + CompetencyCourseRun.objects.get_or_create(course_run=course_run) + CompetencyCourseRun.objects.select_for_update().get(pk=course_run.pk) _reject_duplicate_criterion(tag, object_id, group_id) if group_id is not None: group = resolve_supplied_leaf_group(group_id, tag, course_run) @@ -443,6 +509,7 @@ def associate_competency_criterion( return create_competency_criterion( group=group, object_id=object_id, + course_run=course_run, rule_profile_id=rule_profile_id, rule_type_override=rule_type_override, rule_payload_override=rule_payload_override, diff --git a/src/openedx_learning/applets/cbe/models/__init__.py b/src/openedx_learning/applets/cbe/models/__init__.py index 8b525773c..58d76ddf6 100644 --- a/src/openedx_learning/applets/cbe/models/__init__.py +++ b/src/openedx_learning/applets/cbe/models/__init__.py @@ -4,6 +4,7 @@ from ..rule_payloads import RuleType from .competency_taxonomy import CompetencyTaxonomy +from .course_run import CompetencyCourseRun from .criteria import CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, LogicOperator from .learner_status import ( CompetencyMasteryStatus, @@ -14,6 +15,7 @@ ) __all__ = [ + "CompetencyCourseRun", "CompetencyCriteriaGroup", "CompetencyCriterion", "CompetencyMasteryStatus", diff --git a/src/openedx_learning/applets/cbe/models/course_run.py b/src/openedx_learning/applets/cbe/models/course_run.py new file mode 100644 index 000000000..0f85f533f --- /dev/null +++ b/src/openedx_learning/applets/cbe/models/course_run.py @@ -0,0 +1,32 @@ +""" +CompetencyCourseRun: CBE's own per-course row for a catalog CourseRun. +""" +from __future__ import annotations + +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from openedx_catalog.models_api import CourseRun + +__all__ = [ + "CompetencyCourseRun", +] + + +class CompetencyCourseRun(models.Model): + """ + A course run that has competency criteria, created with its first criterion. + + Criterion creation locks this row, rather than the catalog's CourseRun, to serialize the + per-course dominance check without blocking other apps' writes that reference the course run. + + .. no_pii: + """ + + course_run = models.OneToOneField( + CourseRun, + primary_key=True, + on_delete=models.CASCADE, + related_name="competency_course_run", + help_text=_("The catalog course run this row extends."), + ) diff --git a/src/openedx_learning/migrations/0016_add_competency_course_run.py b/src/openedx_learning/migrations/0016_add_competency_course_run.py new file mode 100644 index 000000000..86630d0cf --- /dev/null +++ b/src/openedx_learning/migrations/0016_add_competency_course_run.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.17 on 2026-10-20 12:42 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_catalog', '0001_initial'), + ('openedx_learning', '0015_criteria_group_unique_constraints'), + ] + + operations = [ + migrations.CreateModel( + name='CompetencyCourseRun', + fields=[ + ('course_run', models.OneToOneField(help_text='The catalog course run this row extends.', on_delete=django.db.models.deletion.CASCADE, primary_key=True, related_name='competency_course_run', serialize=False, to='openedx_catalog.courserun')), + ], + ), + ] diff --git a/src/openedx_tagging/api.py b/src/openedx_tagging/api.py index 005600d63..8d2f90353 100644 --- a/src/openedx_tagging/api.py +++ b/src/openedx_tagging/api.py @@ -169,6 +169,30 @@ def get_children_tags( return taxonomy.get_filtered_tags(parent_tag_value=parent_tag_value, depth=1) +def get_ancestor_tags(tag: Tag) -> QuerySet[Tag]: + """ + Return every ancestor of ``tag`` (its parent, grandparent, ... up to the root). + + Returns an empty queryset if ``tag``'s taxonomy is unset or allows free text, since + free-text tags have no taxonomy-defined hierarchy. + """ + if tag.taxonomy is None or tag.taxonomy.allow_free_text: + return Tag.objects.none() + return Tag.objects.filter(taxonomy=tag.taxonomy, value__in=tag.get_lineage()[:-1]) + + +def get_descendant_tags(tag: Tag) -> QuerySet[Tag]: + """ + Return every descendant of ``tag`` at any depth, excluding ``tag`` itself. + + Returns an empty queryset if ``tag``'s taxonomy is unset or allows free text, since + free-text tags have no taxonomy-defined hierarchy. + """ + if tag.taxonomy is None or tag.taxonomy.allow_free_text: + return Tag.objects.none() + return Tag.objects.filter(taxonomy=tag.taxonomy, depth__gt=tag.depth, lineage__startswith=tag.lineage) + + def resync_object_tags(object_tags: QuerySet | None = None) -> int: """ Reconciles ObjectTag entries with any changes made to their associated taxonomies and tags. diff --git a/tests/openedx_learning/applets/cbe/test_api.py b/tests/openedx_learning/applets/cbe/test_api.py index d1ef776b9..f4dbcbd68 100644 --- a/tests/openedx_learning/applets/cbe/test_api.py +++ b/tests/openedx_learning/applets/cbe/test_api.py @@ -23,6 +23,7 @@ create_competency_taxonomy, get_competency_criteria_tree, get_competency_rule_profiles, + get_courses_with_relative_criteria, is_competency_taxonomy, resolve_supplied_leaf_group, select_competency_taxonomies, @@ -30,6 +31,7 @@ from openedx_learning.applets.cbe import api as cbe_api from openedx_learning.applets.cbe.api import _create_leaf_group from openedx_learning.models import ( + CompetencyCourseRun, CompetencyCriteriaGroup, CompetencyCriterion, CompetencyRuleProfile, @@ -105,6 +107,22 @@ def usage_key(course_run: CourseRun, block_id: str) -> str: return f"block-v1:{key.org}+{key.course}+{key.run}+type@sequential+block@{block_id}" +def make_tag(taxonomy: CompetencyTaxonomy, value: str, parent: Tag | None = None) -> Tag: + """Create a Tag on `taxonomy`, optionally under `parent`, for taxonomy-hierarchy tests.""" + return Tag.objects.create(taxonomy=taxonomy, value=value, parent=parent) + + +def make_leaf_criterion( + tag: Tag, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, block_id: str, +) -> CompetencyCriterion: + """ + Create a leaf group and one live CompetencyCriterion for `tag` in `course_run`. + """ + leaf = _create_leaf_group(tag, course_run) + object_tag = ObjectTag.objects.create(object_id=usage_key(course_run, block_id), taxonomy=tag.taxonomy, tag=tag) + return CompetencyCriterion.objects.create(group=leaf, object_tag=object_tag, rule_profile=default_rule_profile) + + def test_is_competency_taxonomy() -> None: """ is_competency_taxonomy() is True for a competency taxonomy, False for a plain one. @@ -698,7 +716,7 @@ def test_bulk_update_values_return_a_criterion_to_the_taxonomy_profile_not_the_d rule_type=RuleType.GRADE, rule_payload=NEW_PAYLOAD, competency_taxonomy=competency_taxonomy, ) assert leaf.parent is not None and leaf.parent.course is not None - criterion = create_competency_criterion(leaf, usage_key(leaf.parent.course, "p1")) + criterion = create_competency_criterion(leaf, usage_key(leaf.parent.course, "p1"), course_run=leaf.parent.course) assert rule_state(criterion) == (taxonomy_profile.id, None, None) bulk_update_competency_criteria([criterion.id], leaf.id, user=user, **SEEDED_RULE) @@ -1295,3 +1313,247 @@ def test_get_competency_criteria_tree_costs_a_bounded_number_of_queries( _ = group.course # accessing the select_related'd relation must not add a query for criterion in tree.criteria: _ = criterion.object_tag + + +def test_get_courses_with_relative_criteria_single_match( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A single tag with a live criterion in a course is returned, mapped to that tag.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + + assert get_courses_with_relative_criteria([relative.id]) == {course_run: relative} + + +def test_get_courses_with_relative_criteria_ties_break_on_the_lowest_tag_id( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """When two of the given tags both have a live criterion in the same course, the lower id tag wins.""" + first = make_tag(competency_taxonomy, "First Relative") + second = make_tag(competency_taxonomy, "Second Relative") + assert first.id < second.id + make_leaf_criterion(first, course_run, default_rule_profile, "p1") + make_leaf_criterion(second, course_run, default_rule_profile, "p2") + + result = get_courses_with_relative_criteria([second.id, first.id]) + + assert result == {course_run: first} + + +def test_get_courses_with_relative_criteria_empty_input_yields_an_empty_result() -> None: + """An empty tag_ids collection yields an empty result, not an error.""" + assert not get_courses_with_relative_criteria([]) + + +def test_get_courses_with_relative_criteria_excludes_a_group_with_no_live_criterion( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """A CompetencyCriteriaGroup for a relative tag with no attached criterion is not a conflict.""" + relative = make_tag(competency_taxonomy, "Empty Relative") + _create_leaf_group(relative, course_run) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_get_courses_with_relative_criteria_ignores_an_archived_criterion( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """An archived criterion is gone for authoring, so it blocks nothing.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + criterion = make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + CompetencyCriterion.objects.filter(pk=criterion.pk).update(archived=True) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_get_courses_with_relative_criteria_ignores_a_criterion_under_an_archived_leaf_group( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion under an archived leaf group blocks nothing.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + criterion = make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + CompetencyCriteriaGroup.objects.filter(pk=criterion.group_id).update(archived=True) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_get_courses_with_relative_criteria_ignores_a_criterion_under_an_archived_course_level_group( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion under an archived course-level group blocks nothing.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + criterion = make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + course_level_id = criterion.group.parent_id + assert course_level_id is not None + CompetencyCriteriaGroup.objects.filter(pk=course_level_id).update(archived=True) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_get_courses_with_relative_criteria_ignores_a_criterion_under_an_archived_root_group( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion under an archived root group blocks nothing.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + criterion = make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + course_level = criterion.group.parent + assert course_level is not None + root_id = course_level.parent_id + assert root_id is not None + CompetencyCriteriaGroup.objects.filter(pk=root_id).update(archived=True) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_get_courses_with_relative_criteria_counts_a_course_with_one_live_and_one_archived_criterion( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Archiving one of a tag's two criteria in a course leaves the course blocked by the other.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + archived = make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + make_leaf_criterion(relative, course_run, default_rule_profile, "p2") + CompetencyCriterion.objects.filter(pk=archived.pk).update(archived=True) + + assert get_courses_with_relative_criteria([relative.id]) == {course_run: relative} + + +def test_validate_containment_rejects_an_ancestor_conflict( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion on an ancestor of group.tag, in the same course, is rejected.""" + parent = make_tag(competency_taxonomy, "Ancestor Skill") + child = make_tag(competency_taxonomy, "Descendant Skill", parent=parent) + make_leaf_criterion(parent, course_run, default_rule_profile, "p1") + group = _create_leaf_group(child, course_run) + + with pytest.raises(ValidationError, match="tag_id"): + cbe_api._validate_containment(group, course_run) # pylint: disable=protected-access + + +def test_validate_containment_rejects_a_descendant_conflict( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion on a descendant of group.tag, in the same course, is rejected.""" + parent = make_tag(competency_taxonomy, "Ancestor Skill 2") + child = make_tag(competency_taxonomy, "Descendant Skill 2", parent=parent) + make_leaf_criterion(child, course_run, default_rule_profile, "p1") + group = _create_leaf_group(parent, course_run) + + with pytest.raises(ValidationError, match="tag_id"): + cbe_api._validate_containment(group, course_run) # pylint: disable=protected-access + + +def test_validate_containment_allows_sibling_tags( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """Two tags sharing a parent, neither one the other's ancestor, do not conflict.""" + parent = make_tag(competency_taxonomy, "Shared Parent") + sibling_a = make_tag(competency_taxonomy, "Sibling A", parent=parent) + sibling_b = make_tag(competency_taxonomy, "Sibling B", parent=parent) + make_leaf_criterion(sibling_a, course_run, default_rule_profile, "p1") + group = _create_leaf_group(sibling_b, course_run) + + cbe_api._validate_containment(group, course_run) # pylint: disable=protected-access # Must not raise. + + +def test_validate_containment_allows_the_same_tag_in_a_different_course( + tag: Tag, course_run: CourseRun, organization: Organization, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A live criterion for the same tag, scoped to a different course, does not block this course.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + make_leaf_criterion(tag, other_course_run, default_rule_profile, "p1") + group = _create_leaf_group(tag, course_run) + + cbe_api._validate_containment(group, course_run) # pylint: disable=protected-access # Must not raise. + + +def test_validate_containment_allows_a_tag_with_no_relatives(tag: Tag, course_run: CourseRun) -> None: + """A tag with neither ancestors nor descendants has nothing to conflict with.""" + group = _create_leaf_group(tag, course_run) + + cbe_api._validate_containment(group, course_run) # pylint: disable=protected-access # Must not raise. + + +def test_get_courses_with_relative_criteria_narrows_to_the_given_course( + competency_taxonomy: CompetencyTaxonomy, + course_run: CourseRun, + organization: Organization, + default_rule_profile: CompetencyRuleProfile, +) -> None: + """With ``course`` given, only that course's conflict comes back, though another course has one too.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + other_course_run = make_course_run(organization, "Python200", "Fall2026") + make_leaf_criterion(relative, course_run, default_rule_profile, "p1") + make_leaf_criterion(relative, other_course_run, default_rule_profile, "p1") + + assert get_courses_with_relative_criteria([relative.id], course=course_run) == {course_run: relative} + + +def test_get_courses_with_relative_criteria_ignores_a_criterion_outside_a_leaf( + competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, default_rule_profile: CompetencyRuleProfile, +) -> None: + """A criterion on a group nested below a leaf, e.g. made through the admin, is skipped rather than crashing.""" + relative = make_tag(competency_taxonomy, "Relative Skill") + leaf = _create_leaf_group(relative, course_run) + nested = CompetencyCriteriaGroup.objects.create(tag=relative, parent=leaf) + object_tag = ObjectTag.objects.create( + object_id=usage_key(course_run, "p1"), taxonomy=relative.taxonomy, tag=relative, + ) + CompetencyCriterion.objects.create(group=nested, object_tag=object_tag, rule_profile=default_rule_profile) + + assert not get_courses_with_relative_criteria([relative.id]) + + +def test_create_competency_criterion_uses_the_callers_course_run( + tag: Tag, course_run: CourseRun, monkeypatch: pytest.MonkeyPatch, +) -> None: + """The caller already resolved the course run, so creating the criterion doesn't look it up again.""" + def _fail(*args, **kwargs): + raise AssertionError("get_course_run() called again") + monkeypatch.setattr(cbe_api, "get_course_run", _fail) + group = _create_leaf_group(tag, course_run) + + criterion = cbe_api.create_competency_criterion(group, usage_key(course_run, "p1"), course_run=course_run) + + assert criterion.group_id == group.id + + +def test_associate_competency_criterion_creates_the_course_runs_cbe_row( + tag: Tag, course_run: CourseRun, +) -> None: + """A course's first criterion creates its CompetencyCourseRun row.""" + associate_competency_criterion(tag.id, usage_key(course_run, "p1")) + + assert CompetencyCourseRun.objects.filter(course_run=course_run).exists() + + +def test_associate_competency_criterion_reuses_the_course_runs_cbe_row( + tag: Tag, course_run: CourseRun, +) -> None: + """Later criteria in the same course reuse the one CompetencyCourseRun row.""" + associate_competency_criterion(tag.id, usage_key(course_run, "p1")) + associate_competency_criterion(tag.id, usage_key(course_run, "p2")) + + assert CompetencyCourseRun.objects.filter(course_run=course_run).count() == 1 + + +def test_associate_competency_criterion_does_not_lock_the_catalog_course_run( + tag: Tag, course_run: CourseRun, monkeypatch: pytest.MonkeyPatch, +) -> None: + """The per-course lock is on CBE's own row, never on the catalog's CourseRun.""" + def _fail(*args, **kwargs): + raise AssertionError("CourseRun row locked") + monkeypatch.setattr(CourseRun.objects, "select_for_update", _fail) + + associate_competency_criterion(tag.id, usage_key(course_run, "p1")) + + +def test_competency_course_run_is_deleted_with_its_course_run( + tag: Tag, course_run: CourseRun, +) -> None: + """Deleting a course run deletes its CompetencyCourseRun row.""" + associate_competency_criterion(tag.id, usage_key(course_run, "p1")) + + course_run.delete() + + assert not CompetencyCourseRun.objects.exists() diff --git a/tests/openedx_learning/applets/cbe/test_views.py b/tests/openedx_learning/applets/cbe/test_views.py index 0320b1fea..983d252ae 100644 --- a/tests/openedx_learning/applets/cbe/test_views.py +++ b/tests/openedx_learning/applets/cbe/test_views.py @@ -85,6 +85,11 @@ def make_course_run(organization: Organization, course_code: str, run_code: str) return CourseRun.objects.create(catalog_course=catalog_course, run_code=run_code) +def make_tag(taxonomy: CompetencyTaxonomy, value: str, parent: Tag | None = None) -> Tag: + """Create a Tag on `taxonomy`, optionally under `parent`, for taxonomy-hierarchy tests.""" + return Tag.objects.create(taxonomy=taxonomy, value=value, parent=parent) + + # What migration 0005 seeds the system default with. Asserted verbatim rather than imported, so # that a change to the seed surfaces here as a failing contract instead of passing silently. SEEDED_GRADE_PAYLOAD = {"op": "gte", "value": 0.8, "scale": "percent"} @@ -1344,3 +1349,134 @@ def test_view_checks_the_view_taxonomy_rule_not_view_tag(user_client: APIClient, rules.set_perm(view_taxonomy, original_view_taxonomy) assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_descendant_is_rejected_when_its_ancestor_already_has_a_criterion_in_the_same_course( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """A tag is rejected when its taxonomy ancestor already has a criterion in the same course.""" + parent = make_tag(competency_taxonomy, "S1 Parent") + child = make_tag(competency_taxonomy, "S1 Child", parent=parent) + first = user_client.post( + criterion_create_url(parent.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post( + criterion_create_url(child.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert parent.value in str(response.data["tag_id"]) + assert str(course_run.course_key) in str(response.data["tag_id"]) + + +def test_ancestor_is_rejected_when_a_descendant_already_has_a_criterion_in_the_same_course( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """A tag is rejected when one of its taxonomy descendants already has a criterion in the same course.""" + parent = make_tag(competency_taxonomy, "S2 Parent") + child = make_tag(competency_taxonomy, "S2 Child", parent=parent) + first = user_client.post( + criterion_create_url(child.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post( + criterion_create_url(parent.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert child.value in str(response.data["tag_id"]) + assert str(course_run.course_key) in str(response.data["tag_id"]) + + +def test_multi_level_ancestor_conflict_names_the_actual_conflicting_tag( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """ + A conflict several levels up the taxonomy is rejected, and the response names the real + conflicting tag (the grandparent), not just any ancestor. + """ + grandparent = make_tag(competency_taxonomy, "S3 Grandparent") + parent = make_tag(competency_taxonomy, "S3 Parent", parent=grandparent) + child = make_tag(competency_taxonomy, "S3 Child", parent=parent) + first = user_client.post( + criterion_create_url(grandparent.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post( + criterion_create_url(child.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert grandparent.value in str(response.data["tag_id"]) + assert str(course_run.course_key) in str(response.data["tag_id"]) + + +def test_sibling_tags_are_allowed( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """Sibling tags, sharing a parent but neither one the other's ancestor, are both allowed.""" + parent = make_tag(competency_taxonomy, "S4 Parent") + sibling_a = make_tag(competency_taxonomy, "S4 Sibling A", parent=parent) + sibling_b = make_tag(competency_taxonomy, "S4 Sibling B", parent=parent) + first = user_client.post( + criterion_create_url(sibling_a.id), {"object_id": usage_key(course_run, "p1")}, format="json", + ) + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post( + criterion_create_url(sibling_b.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + + +def test_an_archived_ancestor_criterion_does_not_block_its_descendant( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """Once the ancestor's criterion is archived, the descendant may be associated in the same course.""" + parent = make_tag(competency_taxonomy, "S7 Parent") + child = make_tag(competency_taxonomy, "S7 Child", parent=parent) + parent_object_id = usage_key(course_run, "p1") + first = user_client.post(criterion_create_url(parent.id), {"object_id": parent_object_id}, format="json") + assert first.status_code == status.HTTP_201_CREATED + CompetencyCriterion.objects.filter(object_tag__object_id=parent_object_id).update(archived=True) + + response = user_client.post( + criterion_create_url(child.id), {"object_id": usage_key(course_run, "p2")}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + + +def test_hierarchically_related_tags_scoped_to_different_courses_are_allowed( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, organization: Organization, +) -> None: + """An ancestor/descendant pair is allowed when each criterion is scoped to a different course.""" + other_course_run = make_course_run(organization, "Python200", "Fall2026") + parent = make_tag(competency_taxonomy, "S5 Parent") + child = make_tag(competency_taxonomy, "S5 Child", parent=parent) + first = user_client.post(criterion_create_url(parent.id), {"object_id": usage_key(course_run, "p1")}, format="json") + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post( + criterion_create_url(child.id), {"object_id": usage_key(other_course_run, "p1")}, format="json", + ) + + assert response.status_code == status.HTTP_201_CREATED + + +def test_the_same_tag_with_two_different_criteria_in_the_same_course_is_allowed( + user_client: APIClient, competency_taxonomy: CompetencyTaxonomy, course_run: CourseRun, +) -> None: + """A repeat criterion for the same tag in the same course stays allowed.""" + tag = make_tag(competency_taxonomy, "S6 Tag") + first = user_client.post(criterion_create_url(tag.id), {"object_id": usage_key(course_run, "p1")}, format="json") + assert first.status_code == status.HTTP_201_CREATED + + response = user_client.post(criterion_create_url(tag.id), {"object_id": usage_key(course_run, "p2")}, format="json") + + assert response.status_code == status.HTTP_201_CREATED diff --git a/tests/openedx_tagging/test_api.py b/tests/openedx_tagging/test_api.py index 7c9931925..e047c6a07 100644 --- a/tests/openedx_tagging/test_api.py +++ b/tests/openedx_tagging/test_api.py @@ -209,6 +209,56 @@ def test_get_children_tags_no_children(self) -> None: """ assert not list(tagging_api.get_children_tags(self.read_only_taxonomy, self.read_only_taxonomy_tag.value)) + def test_get_ancestor_tags(self) -> None: + """ + get_ancestor_tags() returns every ancestor of a multi-level tag, up to the root. + """ + eukaryota = get_tag("Eukaryota") + result = {tag.value for tag in tagging_api.get_ancestor_tags(self.mammalia)} + assert result == {eukaryota.value, self.animalia.value, self.chordata.value} + + def test_get_ancestor_tags_root_has_no_ancestors(self) -> None: + """ + A root tag (no parent) has no ancestors. + """ + assert not list(tagging_api.get_ancestor_tags(self.archaea)) + + def test_get_ancestor_tags_free_text_taxonomy(self) -> None: + """ + A tag on a free-text-enabled taxonomy has no taxonomy-defined hierarchy to walk. + """ + tag = Tag.objects.create(taxonomy=self.free_text_taxonomy, value="some_tag") + assert not list(tagging_api.get_ancestor_tags(tag)) + + def test_get_descendant_tags(self) -> None: + """ + get_descendant_tags() returns every descendant at any depth, including grandchildren. + """ + result = {tag.value for tag in tagging_api.get_descendant_tags(self.animalia)} + assert result == { + "Arthropoda", + self.chordata.value, + self.mammalia.value, # A grandchild of Animalia (via Chordata), not just a direct child. + "Cnidaria", + "Ctenophora", + "Gastrotrich", + "Placozoa", + "Porifera", + } + + def test_get_descendant_tags_leaf_has_no_descendants(self) -> None: + """ + A leaf tag (no children) has no descendants. + """ + assert not list(tagging_api.get_descendant_tags(self.mammalia)) + + def test_get_descendant_tags_free_text_taxonomy(self) -> None: + """ + A tag on a free-text-enabled taxonomy has no taxonomy-defined hierarchy to walk. + """ + tag = Tag.objects.create(taxonomy=self.free_text_taxonomy, value="some_tag") + assert not list(tagging_api.get_descendant_tags(tag)) + def test_resync_object_tags(self) -> None: self.taxonomy.allow_multiple = True self.taxonomy.save()