fix/814: treat an archived tag as no longer part of its taxonomy on create, import, and export

docs/openedx_tagging/decisions/0014-archived-tags-on-create-import-export.rst

@@ -0,0 +1,167 @@
+.. _openedx-tagging-adr-0014:
+
+14. Archived tags on create, import, and export
+================================================
+
+Status
+------
+
+Proposed
+
+Several decisions below are marked **(awaiting confirmation)**. They need an answer from the
+owner named in each one before this ADR can be accepted.
+
+Context
+-------
+
+Competency-based education (CBE) records learner mastery against tags. Deleting a tag would
+destroy the history that points at it, so the approved design on `#655`_ archives a tag that
+learner mastery depends on instead of deleting it (see also :ref:`openedx_learning ADR 0003
+<openedx-learning-adr-0003>`, decision 3).
+`#776`_ adds the ``Tag.archived`` flag, `#778`_ hides archived tags from the read paths and the
+export through ``TagQuerySet.unarchived()``, and `#780`_ turns a tag delete into an archive when
+something depends on the tag.
+
+An archived tag stays in the database and keeps its ``value`` and ``external_id``. The
+per-taxonomy ``unique_together`` constraints on ``(taxonomy, value)`` and
+``(taxonomy, external_id)`` have no exception for archived rows, so both values stay taken.
+No API function or REST endpoint can restore an archived tag. An operator can still clear ``archived``
+in the Django admin, as "Known limitations" describes.
+
+This ADR records how the paths that write tags into a taxonomy treat an archived tag: creating a
+tag (``Taxonomy.add_tag``, ``add_tag_to_taxonomy`` and the REST create endpoint), importing a file
+(``TagImportPlan`` through ``import_tags``), and exporting a taxonomy. It covers ``Tag`` only.
+Archived taxonomies and archived tag associations (``ObjectTag``) follow `#778`_ and `#779`_.
+
+Decision
+--------
+
+An archived tag is no longer part of its taxonomy
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+No path in this ADR creates, updates, renames, re-parents or revives an archived tag. The tag
+still occupies its value and its ``external_id``, and those values stay reserved for as long as
+the tag exists.
+
+The archived subtree
+~~~~~~~~~~~~~~~~~~~~
+
+The **archived subtree** of a taxonomy is every archived tag together with every tag under one.
+``TagQuerySet.in_archived_subtree()`` returns it. Because tree depth is capped by
+``TAXONOMY_MAX_DEPTH``, the query checks the tag and a fixed number of ancestors, so its size does
+not grow with the number of archived tags.
+
+Create, import, and the replace sweep treat every tag in the archived subtree as outside the
+taxonomy. The export from `#778`_ already leaves the whole subtree out.
+
+**(Awaiting confirmation from the #778 owner.)** Whether an unarchived tag under an archived tag
+can exist at all is open. If archiving always covers the whole subtree, as `#780`_'s rules produce,
+the subtree rule above only guards against direct database edits. If such a tag can exist, the
+listings and ``update_tag`` from `#778`_ still treat it as live, while the paths in this ADR do
+not.
+
+Create
+~~~~~~
+
+Adding a tag whose value or ``external_id`` belongs to an archived tag raises ``ValueError`` with a
+message that names the archived tag as the cause and asks for a different value. A value held by an
+unarchived tag keeps the existing "already exists" message. Adding a tag under a tag in the
+archived subtree is refused. Values are stripped before they are compared, as ``Tag.clean`` strips
+them before saving.
+
+Import
+~~~~~~
+
+A row whose ``value``, ``id``, ``previous_id`` or ``parent_id`` matches a tag in the archived
+subtree is an import plan error. When any row has this error, the plan builds no actions, so the
+import changes nothing. The error is reported once per row.
+
+The match runs in the database, under the column collation, because the import actions find the
+tags they change with database lookups. A comparison in Python would disagree with the database:
+on MySQL the collation also ignores accents and trailing spaces.
+
+A replace import never deletes a tag in the archived subtree, because the export leaves that
+subtree out and its absence from the file says nothing about it. These checks hold for the tags
+that are archived when the plan is built.
+
+Export
+~~~~~~
+
+An export leaves out the archived subtree (`#778`_). Importing that export into an empty taxonomy,
+or re-importing it into its own taxonomy with ``replace=True``, succeeds and leaves the archived
+subtree as it was.
+
+Known limitations
+~~~~~~~~~~~~~~~~~
+
+- **(Awaiting confirmation from the #780 owner.)** A replace import that deletes the parent of an
+  archived tag moves the archived tag to the root. This rewrites the parent of an archived record.
+  The recommended rule is to archive such a parent instead, the way `#780`_ archives the ancestors
+  of a depended-upon tag. Moving the tag to the root is the interim behavior until the import's
+  deletes follow `#780`_.
+- **(Awaiting confirmation from the #780 owner.)** A replace import deletes through its own
+  ``DeleteTag`` action, which `#780`_ does not cover. Until the import's deletes go through the
+  archive-or-delete decision of `#780`_, a replace import can hard-delete a tag that learner mastery
+  depends on. The recommended shape is one decision for the whole delete set, made inside the
+  import's ``execute`` transaction.
+- Course import reuses ``TagImportPlan`` (:ref:`ADR 0011 <openedx-tagging-adr-0011>`).
+  A course whose tags match an archived tag on the target instance therefore fails its criteria
+  import with this ADR's plan error, although the course author cannot change the taxonomy. ADR
+  0011 should be amended for this case separately.
+- The error quotes the administrator's input, not the archived tag that holds the value. An
+  administrator cannot see that tag, for example the archived "Cafe" that refuses "Café" on MySQL.
+- ``Taxonomy.update_tag`` renaming a tag to an archived value raises ``IntegrityError`` rather than
+  a message that names the archived tag. The REST rename path reports it through the serializer.
+- The Django admin edit page for ``Tag`` does not restrict its fields, so an operator can change
+  ``archived`` and an archived tag's value there. Such changes are outside the rules in this ADR.
+
+Future considerations
+~~~~~~~~~~~~~~~~~~~~~
+
+**(Awaiting confirmation from product.)** The approved thread on `#655`_ returns an error "for now"
+and expects that adding a tag with an archived tag's value may later un-archive that tag instead.
+The reserved values above describe the current state, not a permanent rule. A restore path would
+also let the messages suggest restoring the tag instead of choosing a different value.
+
+Rejected Alternatives
+---------------------
+
+Un-archive the tag when its value is created or imported again
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Rejected for now because no restore path has been designed: the approved design on `#655`_ chose an
+error, and silently reviving a tag would reattach learner history to a tag an administrator chose to
+remove.
+
+Release the archived tag's value and ``external_id``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Making the unique constraints conditional on ``archived`` would let a new tag take the value.
+Rejected because two tags in one taxonomy would then share a value and an ``external_id``, which
+breaks import and export, both keyed on them, and any later restore.
+
+Find the archived subtree by lineage prefix
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+One ``lineage__startswith`` condition per archived tag selects the same rows, but the query grows
+with every archived tag. SQLite refuses an expression tree deeper than 1,000, so a taxonomy with
+about 1,000 archived tags could no longer be imported at all.
+
+Delete the archived subtree on a replace import
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Rejected because the export omits the subtree, so every round trip of a taxonomy through export and
+replace import would destroy the history that archiving exists to keep.
+
+Changelog
+---------
+
+2026-11-06:
+
+* Proposed.
+
+.. _#655: https://github.com/openedx/openedx-core/issues/655
+.. _#776: https://github.com/openedx/openedx-core/issues/776
+.. _#778: https://github.com/openedx/openedx-core/issues/778
+.. _#779: https://github.com/openedx/openedx-core/issues/779
+.. _#780: https://github.com/openedx/openedx-core/issues/780

src/openedx_tagging/api.py

@@ -82,36 +82,36 @@ def create_taxonomy(  # pylint: disable=too-many-positional-arguments
 
 def get_taxonomy(taxonomy_id: int) -> Taxonomy | None:
     """
-    Returns the Taxonomy which has the given ID, or None if not found.
+    Returns the Taxonomy which has the given ID, or None if not found or archived.
     """
-    return Taxonomy.objects.filter(pk=taxonomy_id).first()
+    return Taxonomy.objects.unarchived().filter(pk=taxonomy_id).first()
 
 
 def get_taxonomy_by_export_id(taxonomy_export_id: str) -> Taxonomy | None:
     """
-    Returns the Taxonomy which has the given export ID, or None if not found.
+    Returns the Taxonomy which has the given export ID, or None if not found or archived.
     """
-    return Taxonomy.objects.filter(export_id=taxonomy_export_id).first()
+    return Taxonomy.objects.unarchived().filter(export_id=taxonomy_export_id).first()
 
 
 def get_taxonomies(enabled=True) -> QuerySet[Taxonomy]:
     """
-    Returns a queryset containing the enabled taxonomies, sorted by name.
+    Returns a queryset containing the enabled, unarchived taxonomies, sorted by name.
 
     We return a QuerySet here for ease of use with Django Rest Framework and other query-based use cases.
 
     If you want the disabled taxonomies, pass enabled=False.
     If you want all taxonomies (both enabled and disabled), pass enabled=None.
     """
-    queryset = Taxonomy.objects.order_by("name", "id")
+    queryset = Taxonomy.objects.unarchived().order_by("name", "id")
     if enabled is None:
         return queryset.all()
     return queryset.filter(enabled=enabled)
 
 
 def get_tags(taxonomy: Taxonomy) -> TagDataQuerySet:
     """
-    Returns a QuerySet of all the tags in the given taxonomy.
+    Returns a QuerySet of all the unarchived tags in the given taxonomy.
 
     Note that if the taxonomy is dynamic or free-text, only tags that have
     already been applied to some object will be returned.
@@ -146,7 +146,7 @@ def search_tags(
         # Fetch tags that the object already has to exclude them from the result.
         # Note: this adds a fair bit of complexity. In the future, maybe we can just do this filtering on the frontend?
         excluded_values = list(
-            taxonomy.objecttag_set.filter(object_id=exclude_object_id).values_list(
+            taxonomy.objecttag_set.unarchived().filter(object_id=exclude_object_id).values_list(
                 "_value", flat=True
             )
         )
@@ -162,7 +162,9 @@ def get_children_tags(
     parent_tag_value: str,
 ) -> TagDataQuerySet:
     """
-    Returns a QuerySet of children tags for the given parent tag.
+    Returns a QuerySet of unarchived children tags for the given parent tag.
+
+    Raises Tag.DoesNotExist if the parent tag does not exist, or Tag.IsArchived if it is archived.
 
     Note that if the taxonomy allows free-text tags, then the returned list will be empty.
     """
@@ -196,11 +198,14 @@ def get_object_tags(
     """
     Returns a Queryset of object tags for a given object.
 
+    Archived object tags are excluded, as are those whose tag or taxonomy is archived.
+
     Pass taxonomy_id to limit the returned object_tags to a specific taxonomy.
     """
     filters = {"taxonomy_id": taxonomy_id} if taxonomy_id else {}
     base_qs = (
         ObjectTag.objects
+        .unarchived()
         .filter(object_id=object_id, **filters)
         .exclude(taxonomy__enabled=False)  # Exclude if the whole taxonomy is disabled
     )
@@ -234,14 +239,15 @@ def get_object_tag_counts(object_id_pattern: str, count_implicit=False) -> dict[
     "course-v1:foo+bar+baz@*", or a list of "comma,separated,IDs", return a
     dict of matching object IDs and how many tags each object has.
 
-    Deleted tags and disabled taxonomies are excluded from the counts, even if
+    Deleted tags, disabled taxonomies, and archived object tags (including those
+    whose tag or taxonomy is archived) are excluded from the counts, even if
     ObjectTag data about them is present.
 
     count_implicit: if True, this means to count all ancestor tags (implicit
     tags) of the explict tags that are associated with the object.
     """
     # Note: in the future we may add an option to exclude system taxonomies from the count.
-    qs: Any = ObjectTag.objects
+    qs: Any = ObjectTag.objects.unarchived()
     if object_id_pattern.endswith("*"):
         qs = qs.filter(object_id__startswith=object_id_pattern[0:len(object_id_pattern) - 1])
     elif "*" in object_id_pattern:
@@ -449,6 +455,9 @@ def add_tag_to_taxonomy(
     Tag is returned.
 
     If `external_id` is omitted, one is generated from the tag's name.
+
+    Raises ``ValueError`` if the value or ``external_id`` is held by another tag,
+    archived or not, or if the parent is archived or under an archived tag.
     """
     new_tag = taxonomy.add_tag(tag, parent_tag_value, external_id)
 
@@ -549,13 +558,15 @@ def add_usage_counts(taxonomy: Taxonomy, tag_data: TagDataQuerySet) -> TagDataQu
     parent tag, "Natural Science" is only applied to it once,
     because no tag can be applied to the same object twice.
 
+    Archived object tags, and those whose tag or taxonomy is archived, are not counted.
+
     For performance reasons, we call this function with the list result of the
     QuerySet so we can then add the counts in-memory rather than annotate to a
     QuerySet which would require a very expensive annotation to join the
     in-memory data to the original QuerySet.
     """
 
-    object_tags = taxonomy.objecttag_set.values_list("object_id", "tag__lineage")
+    object_tags = taxonomy.objecttag_set.unarchived().values_list("object_id", "tag__lineage")
     tag_counts: Counter[str] = Counter()
     object_tag_lineage_seen: defaultdict[str, set] = defaultdict(set)
 

src/openedx_tagging/import_export/api.py

@@ -71,10 +71,14 @@ def import_tags(
     of each import step and the current status.
     There can only be one task in progress at a time per taxonomy
 
-    Set `replace` to True to delete all not readed Tag of the given taxonomy.
+    Set `replace` to True to delete every tag of the given taxonomy that the file does not mention.
     Ex. Given a taxonomy with `tag_1`, `tag_2` and `tag_3`. If there is only `tag_1`
     in the file (regardless of action), then `tag_2` and `tag_3` will be deleted
-    if `replace=True`
+    if `replace=True`. A tag in an archived subtree when the plan is built is not deleted this way.
+    An archived tag whose parent is deleted is moved to the root.
+
+    A file row whose value, id, previous_id or parent_id belongs to a tag in an archived
+    subtree is reported as an error, and nothing is imported.
 
     Set `plan_only` to True to only generate the actions and not execute them.
     """

src/openedx_tagging/import_export/exceptions.py

@@ -80,6 +80,24 @@ def __init__(self, tag_id: str, plain_row: int, rename_row: int, **kargs):
         ).format(tag_id=tag_id, plain_row=plain_row, rename_row=rename_row)
 
 
+class ArchivedTagCollisionError(TagImportError):
+    """
+    Exception raised when a row's value, id, previous_id or parent_id matches an archived tag or a tag under one.
+
+    ``row`` is the row's 1-based position among the file's data rows.
+    """
+
+    def __init__(self, row: int, field: str, value: str, under_archived: bool = False, **kargs):
+        super().__init__(**kargs)
+        if under_archived:
+            message = _("Row {row}'s {field} ({value}) belongs to a tag under an archived tag in this taxonomy.")
+        else:
+            message = _("Row {row}'s {field} ({value}) belongs to an archived tag in this taxonomy.")
+        # previous_id names the one tag the row means to rename, so no other value would do.
+        advice = _("An import cannot change it.") if field == "previous_id" else _("Use a different {field}.")
+        self.message = f"{message} {advice}".format(row=row, field=field, value=value)
+
+
 class ParentCycleError(TagImportError):
     """
     Exception raised when a row's parent_id would make its tag its own ancestor.