feat: add CompetencyCriteriaGroup, the criteria tree's AND/OR node

[jesperhodge]

Part 4 of 7 for issue openedx#641. Based on part 3.

One deviation from openedx#641's AC 26. AC 26 lists course as PROTECT. AC 26 needs updating so the
ticket and the code agree.

What this does

Adds CompetencyCriteriaGroup, the internal AND/OR node of a criteria tree. One CompetencyAchievementCriteria is one root group plus all of its descendant groups and, once part 7 lands, the leaf criteria under any of them. See ADR-0002 Decision 2.

Two constraints that are deliberately absent

• Nothing ties logic_operator to child count, and an empty group is not rejected. A group's AND/OR setting is never actually checked against what's inside it, because a group is always empty the moment it's created, and nothing re-checks it later when children get added one by one.
• No UniqueConstraint on (parent, ordering). There's no rule stopping two children from having the same order position, because a group's children are two different kinds of things: sub-groups, and leaves; and only one kind even has an order value. So the rule would only half-work while looking complete.

Both belong to the authoring API, which no ticket owns yet.

Reviewing this PR

The test module is ordered so that reading it top to bottom gives the model's contract: schema, then tree shape, then the two absent constraints, then indexes and history. Each test name states the expected behavior.

One test would pass for the wrong reason if written naively. test_the_database_carries_adr_0002_decision_5_indexes_1_and_2 compares the ordered column list, not a set. An index on (course_id, tag_id) would satisfy a set comparison just as well, but only the tag-first ordering also serves tag-only lookups.

Verifying

pytest tests/openedx_learning --no-cov -q                            # 18 passed
python manage.py makemigrations openedx_learning --check --dry-run   # no changes detected
lint-imports                                                         # the openedx_catalog import from part 2 is now real

By hand, confirm the tree links work and that a parent delete cascades to its children:

from openedx_learning.models import CompetencyCriteriaGroup, CompetencyTaxonomy
from openedx_tagging.models import Tag

taxonomy = CompetencyTaxonomy.objects.create(name="T", export_id="t")
tag = Tag.objects.create(taxonomy=taxonomy, value="Writing")
root = CompetencyCriteriaGroup.objects.create(tag=tag)
child = CompetencyCriteriaGroup.objects.create(tag=tag, parent=root)
child.parent_id == root.pk        # True
root.delete()                     # CASCADE: child is deleted with it, not re-rooted or orphaned
CompetencyCriteriaGroup.objects.filter(pk=child.pk).exists()   # False

Refs openedx#641

[jesperhodge]

nit: Since the term CompetencyAchievementCriteria has been confusing, we should workshop this. Will it be its own non-database class? If so, is there a better name? If not, can we explain this more clearly?

[jesperhodge]

nit / discussion for later: Do leaf criteria maybe need an ordering field as well?

[mgwozdz-unicon]

Claude and I both reviewed. All change requests were identified by me; comment drafted by Claude:

The data model and migration are correct. Everything below is naming, docstrings, and test quality, not logic. Requesting changes for the items marked as such; the rest are direct answers to open questions in this thread.

src/openedx_learning/applets/cbe/models/criteria.py

1. Module docstring (lines 2-6): drop CompetencyAchievementCriteria, summarize the ADR decisions instead of only pointing at them, and lead with what this tree is actually for.

Current:

"""
The CompetencyAchievementCriteria tree: CompetencyCriteriaGroup, the internal AND/OR node.

See :ref:`openedx-learning-adr-0002` Decision 2 for the design and Decision 7 for why every
foreign key here cascades, and :ref:`openedx-learning-adr-0003` Decisions 1 and 2 for why this
model carries ``django-simple-history`` tracking and CompetencyTaxonomy does not.
"""

Requested:

"""
The Competency Criteria tree: the AND/OR hierarchy of criteria that defines the rule for
demonstrating mastery of one competency. CompetencyCriteriaGroup is this tree's internal
AND/OR node: each group combines its child nodes, which may themselves be groups or, at the
bottom of the tree, leaf :class:`CompetencyCriterion` rows. Each leaf names the learning
object and the threshold used to measure it.

Every foreign key here cascades, so that deleting a tag, a course run, or a parent group
removes the whole subtree beneath it (:ref:`openedx-learning-adr-0002` Decision 7). This
model carries ``django-simple-history`` tracking so that changes to a criteria tree are
auditable (:ref:`openedx-learning-adr-0003` Decisions 1 and 2).
"""

Three changes bundled here, since they all land on the same docstring:

• Lead with what the tree is for before describing its shape: one AND/OR hierarchy of criteria per competency, bottoming out at the leaf criteria that actually measure something.
• Answering the open thread on this docstring: CompetencyAchievementCriteria is not a class and will not become one. The model breakdown across openedx#613/openedx#641 is CompetencyCriteriaGroup (internal nodes) plus CompetencyCriterion (leaf nodes); there is no third class. CompetencyAchievementCriteria appears four times in this PR, all as prose, never as an identifier: this docstring's first line (being replaced here), the class docstring two paragraphs below (criteria.py:33 and criteria.py:35, item 2 below), and the test module docstring (test_criteria_group.py:2, item 3 below). Please replace all four with plain prose "Competency Criteria tree" (no code formatting). Formatting it in double backticks anywhere makes it look like a code symbol, which is what's generating the "is this a class?" questions in this thread. ADR-0002 has the same code-formatted usage in several places; that's a separate, non-blocking cleanup for whoever next touches that ADR, not something to fix in this PR.
• Drop "and CompetencyTaxonomy does not" from the history sentence. That's a fact about a different model and belongs in CompetencyTaxonomy's own docstring or in ADR-0003, not repeated here.

2. Class docstring (lines 33, 35): same rename.

Current:

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. ...

Requested:

class CompetencyCriteriaGroup(models.Model):
    """
    An internal AND/OR node in a Competency Criteria tree.

    A single Competency Criteria tree is one root CompetencyCriteriaGroup plus all of its
    descendant groups and leaf :class:`CompetencyCriterion` rows. ...

tests/openedx_learning/applets/cbe/test_criteria_group.py

3. Line 2: same rename. "...the internal AND/OR node of a CompetencyAchievementCriteria tree" becomes "...the internal AND/OR node of a Competency Criteria tree."

4. Line 65, test_group_logic_operator_accepts_and_or_and_null_regardless_of_child_count: the assertion for the 2-children case doesn't test what the name claims. The final line, assert CompetencyCriteriaGroup.objects.filter(parent=parent).count() == 2, only proves two children can be created under a parent. It never checks that the parent's own logic_operator value is still what it was set to once children exist, which is the actual claim in the test's name and docstring. Please add a check on the parent itself before the existing assertion:

    parent = CompetencyCriteriaGroup.objects.create(tag=tag, logic_operator=logic_operator)
    CompetencyCriteriaGroup.objects.create(tag=tag, parent=parent)
    CompetencyCriteriaGroup.objects.create(tag=tag, parent=parent)
    parent.refresh_from_db()
    assert parent.logic_operator == logic_operator
    assert CompetencyCriteriaGroup.objects.filter(parent=parent).count() == 2

5. Line 121: rename test_the_database_carries_adr_0002_decision_5_indexes_1_and_2 to describe the behavior, not the ADR decision it comes from. Requested name: test_composite_tag_course_index_and_automatic_parent_index_exist.

6. Lines 122-129: rewrite the docstring to describe the indexes in its own words instead of citing "ADR-0002 Decision 5." State these as facts about this table directly, rather than by reference to the ADR: the index-level detail lives in ADR-0002 at a level of specificity an ADR shouldn't really carry, so a docstring that leans on "ADR-0002 Decision 5" to explain itself is pointing at something that may not stay worded that way.

Current:

    """
    The real table carries ADR-0002 Decision 5's index 1, the composite (tag, course), and index
    2 on parent. Index 2 comes from Django's automatic per-ForeignKey index rather than an
    explicit models.Index, so this introspects the database rather than the model.

    Compares 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 just as
    well, but only the tag-first ordering also serves tag-only lookups.
    """

Requested:

    """
    The table carries a composite index on (tag, course), declared explicitly in Meta.indexes,
    and a second index on parent, which Django creates automatically for every ForeignKey and
    which this model does not declare explicitly. This test introspects the real database
    rather than the model definition, since the parent index exists only as a side effect of
    Django's default behavior.

    Compares 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 just as
    well, but only the tag-first ordering also serves tag-only lookups.
    """

7. Lines 162-179, test_history_not_recorded_for_tag_taxonomy_or_competencytaxonomy: remove this test from this file. It asserts that Tag and Taxonomy (owned by openedx_tagging, not this app) have no history tracking, alongside CompetencyTaxonomy. None of that is about CompetencyCriteriaGroup, which is what this file covers. If a change in openedx_tagging ever adds history tracking to Tag or Taxonomy for reasons unrelated to CBE, this unrelated test file breaks for a reason nobody touching this file would expect. If this coverage is worth keeping, it belongs in a test module owned by whichever app owns that decision (openedx_tagging's own tests, or a dedicated ADR-0003 versioning test module), not here.

tests/openedx_learning/applets/cbe/test_criteria_group_deletion.py

8. Lines 83-95, test_a_cascaded_group_removal_is_recorded_in_history: remove, it duplicates lines 49-64. test_deleting_a_tag_also_deletes_its_competency_criteria_groups (lines 49-64) already creates a group, deletes its tag, and asserts a HistoricalCompetencyCriteriaGroup row with history_type='-' exists for that group, in addition to asserting the group itself is gone. The test at line 83 does the identical setup, delete, and history assertion, and nothing else. If you want to keep the framing that this specifically covers a cascade-triggered removal rather than a direct delete, fold that sentence into the docstring of the line-49 test instead of keeping a second test whose assertions are a strict subset of the first.