feat: hide archived tagging records in Studio's content_tagging app

cms/djangoapps/contentstore/views/tests/test_block.py

@@ -28,6 +28,7 @@
 from openedx_events.content_authoring.data import DuplicatedXBlockData
 from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
 from openedx_events.testing import OpenEdxEventsTestMixin
+from openedx_tagging.models import ObjectTag
 from pytz import UTC
 from web_fragments.fragment import Fragment
 from webob import Response
@@ -1541,6 +1542,13 @@ def test_duplicate_tags(self):
         # Tag the child block
         tagging_api.tag_object(str(source_block.location), taxonomyA, ["two"],)
 
+        # Disable a taxonomy; disabled is not archived, so its tags are still duplicated.
+        taxonomyB.enabled = False
+        taxonomyB.save()
+
+        # Archive one of the chapter's associations; it must not be duplicated.
+        ObjectTag.objects.filter(object_id=str(source_chapter.location), _value="four").update(archived=True)
+
         # Duplicate the chapter (and its children)
         dupe_location = duplicate_block(
             parent_usage_key=source_course.location,
@@ -1555,17 +1563,22 @@ def test_duplicate_tags(self):
         expected_chapter_tags = [
             f'<ObjectTag> {str(dupe_chapter.location)}: A=one',
             f'<ObjectTag> {str(dupe_chapter.location)}: A=two',
-            f'<ObjectTag> {str(dupe_chapter.location)}: B=four',
             f'<ObjectTag> {str(dupe_chapter.location)}: B=three',
         ]
-        dupe_chapter_tags = [str(object_tag) for object_tag in tagging_api.get_object_tags(str(dupe_chapter.location))]
+        # get_object_tags() hides disabled taxonomies, so read the rows directly.
+        dupe_chapter_tags = sorted(str(object_tag) for object_tag in ObjectTag.objects.filter(
+            object_id=str(dupe_chapter.location)
+        ))
         assert dupe_chapter_tags == expected_chapter_tags
         expected_block_tags = [
             f'<ObjectTag> {str(dupe_block.location)}: A=two',
         ]
         dupe_block_tags = [str(object_tag) for object_tag in tagging_api.get_object_tags(str(dupe_block.location))]
         assert dupe_block_tags == expected_block_tags
 
+        # The archived association is left in place on the source.
+        assert ObjectTag.objects.get(object_id=str(source_chapter.location), _value="four").archived is True
+
 
 @ddt.ddt
 class TestMoveItem(ItemTest):

cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py

@@ -5,6 +5,7 @@
 """
 import ddt
 from opaque_keys.edx.keys import UsageKey
+from openedx_content import models_api as content_models
 from openedx_content.api import signals as content_signals
 from openedx_events.content_authoring.signals import (
     LIBRARY_BLOCK_DELETED,
@@ -13,7 +14,7 @@
     XBLOCK_UPDATED,
 )
 from openedx_events.testing import OpenEdxEventsTestMixin
-from openedx_tagging.models import Tag
+from openedx_tagging.models import ObjectTag, Tag
 from organizations.models import Organization
 from rest_framework.test import APIClient
 
@@ -284,8 +285,28 @@ def test_copy_and_paste_unit_with_tags(self):
             dest_chapter = BlockFactory.create(parent=dest_course, category='chapter', display_name='Section')
             dest_sequential = BlockFactory.create(parent=dest_chapter, category='sequential', display_name='Subsection')
 
-        # Copy the unit
         unit_key = source_block_keys['vertical']
+
+        # Disable a taxonomy; disabled is not archived, so its tags are still pasted.
+        taxonomy_disabled = tagging_api.create_taxonomy(
+            "test_taxonomy_disabled",
+            "Test Taxonomy Disabled",
+            export_id="DISABLED",
+        )
+        tagging_api.set_taxonomy_orgs(taxonomy_disabled, all_orgs=True)
+        tagging_api.add_tag_to_taxonomy(taxonomy_disabled, "tag_1")
+        tagging_api.tag_object(object_id=str(unit_key), taxonomy=taxonomy_disabled, tags=["tag_1"])
+        taxonomy_disabled.enabled = False
+        taxonomy_disabled.save()
+
+        # Archive one of the unit's associations, to test that it isn't pasted.
+        ObjectTag.objects.filter(
+            object_id=str(unit_key),
+            taxonomy__export_id='ALL_ORGS',
+            _value='tag_2',
+        ).update(archived=True)
+
+        # Copy the unit
         copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(unit_key)}, format="json")
         assert copy_response.status_code == 200
 
@@ -302,9 +323,15 @@ def test_copy_and_paste_unit_with_tags(self):
 
         # Only tags from the taxonomy that is associated with the dest org should be copied
         tags = list(tagging_api.get_object_tags(str(dest_unit_key)))
-        assert len(tags) == 2
+        assert len(tags) == 1
         assert str(tags[0]) == f'<ObjectTag> {dest_unit_key}: test_taxonomy=tag_1'
-        assert str(tags[1]) == f'<ObjectTag> {dest_unit_key}: test_taxonomy=tag_2'
+
+        # get_object_tags() hides the disabled taxonomy, so read the rows directly.
+        disabled_values = ObjectTag.objects.filter(
+            object_id=str(dest_unit_key),
+            taxonomy__export_id="DISABLED",
+        ).values_list("_value", flat=True)
+        assert list(disabled_values) == ["tag_1"]
 
         # Ensure that the pasted child blocks were tagged too.
         dest_tags, _ = tagging_api.get_all_object_tags(dest_course.id)
@@ -506,6 +533,48 @@ def test_paste_from_library_read_only_tags(self):
             assert object_tag.value in self.lib_block_tags
             assert not object_tag.is_copied
 
+    def test_paste_from_library_unit_leaves_archived_tags_out(self):
+        # Neither the pasted unit nor its child carries an archived tag.
+        # If we're working with Containers in test cases, we need this line:
+        self.addCleanup(content_models.Container.reset_cache)
+        taxonomy = tagging_api.get_taxonomy_by_export_id("ALL_ORGS")
+        unit = library_api.create_container(
+            library_key=self.library.key,
+            container_cls=content_models.Unit,
+            slug="unit-1",
+            title="Unit 1",
+            user_id=None,
+        )
+        library_api.update_container_children(unit.container_key, [self.lib_block_key], None)
+        library_api.publish_changes(self.library.key)
+        unit_id = str(unit.container_key)
+        child_id = str(self.lib_block_key)
+        tagging_api.tag_object(unit_id, taxonomy, ["tag_2", "tag_3"])
+        # The child already has the tags from setUp.
+        ObjectTag.objects.filter(object_id=unit_id, _value="tag_3").update(archived=True)
+        ObjectTag.objects.filter(object_id=child_id, _value="tag_5").update(archived=True)
+
+        library_api.copy_container(unit.container_key, self.user.id)
+        with self.store.bulk_operations(self.course.id):
+            chapter = BlockFactory.create(parent=self.course, category='chapter', display_name='Section')
+            sequential = BlockFactory.create(parent=chapter, category='sequential', display_name='Subsection')
+        paste_response = self.client.post(XBLOCK_ENDPOINT, {
+            "parent_locator": str(sequential.location),
+            "staged_content": "clipboard",
+        }, format="json")
+        assert paste_response.status_code == 200
+
+        pasted_unit_key = UsageKey.from_string(paste_response.json()["locator"])
+        pasted_children = self.store.get_item(pasted_unit_key).children
+        assert len(pasted_children) == 1
+
+        def stored_values(key):
+            # Unfiltered, so that a hidden copy would be found too.
+            return sorted(ObjectTag.objects.filter(object_id=str(key)).values_list("_value", flat=True))
+
+        assert stored_values(pasted_unit_key) == ["tag_2"]
+        assert stored_values(pasted_children[0]) == ["tag_1"]
+
     def test_paste_from_library_copies_asset(self):
         """
         Assets from a library component copied into a subdir of Files & Uploads.

openedx/core/djangoapps/content_libraries/tests/test_api.py

@@ -30,11 +30,14 @@
 )
 from openedx_events.testing import EventsIsolationMixin
 from openedx_events.tooling import OpenEdxPublicSignal
+from openedx_tagging.models import ObjectTag
 from user_tasks.models import UserTaskStatus
 
 from common.djangoapps.student.tests.factories import UserFactory
+from openedx.core.djangoapps.content_tagging import api as tagging_api
 
 from .. import api
+from ..api.serializers import ContainerSerializer
 from ..models import ContentLibrary
 from .base import ContentLibrariesRestApiTest
 
@@ -780,6 +783,44 @@ def test_copy_and_paste_container_same_library(self) -> None:
         child_history = content_api.get_entity_draft_history(subsections[0].container_id)
         assert len(child_history) == 0
 
+    def test_copy_container_leaves_archived_tags_out(self) -> None:
+        """
+        Copying a container leaves out archived tag associations, on the container and on its children.
+
+        Pasting into a library does not apply tags, so this checks what the serializer and the clipboard carry.
+        """
+        taxonomy = tagging_api.create_taxonomy('Archived Tags Taxonomy')
+        tagging_api.set_taxonomy_orgs(taxonomy, all_orgs=True)
+        for value in ("one", "two", "three", "four"):
+            tagging_api.add_tag_to_taxonomy(taxonomy, value)
+
+        section_id = str(self.section1.container_key)
+        child_id = str(self.subsection1.container_key)
+        tagging_api.tag_object(section_id, taxonomy, ["one", "two"])
+        tagging_api.tag_object(child_id, taxonomy, ["three", "four"])
+        # Archive one association on the copied container and one on its child
+        ObjectTag.objects.filter(object_id=section_id, _value="two").update(archived=True)
+        ObjectTag.objects.filter(object_id=child_id, _value="four").update(archived=True)
+
+        serializer = ContainerSerializer(api.get_container(self.section1.container_key))
+
+        # The serializer also holds an empty entry for each untagged child block, so check the tagged ids by name.
+        assert serializer.tags[section_id] == {taxonomy.id: ["one"]}
+        assert serializer.tags[child_id] == {taxonomy.id: ["three"]}
+        assert all(not tags for object_id, tags in serializer.tags.items() if object_id not in (section_id, child_id))
+        # The clipboard receives the same tags
+        clipboard = api.copy_container(self.section1.container_key, self.user.id)
+        clipboard_values = {
+            object_id: sorted(value for values in by_taxonomy.values() for value in values)
+            for object_id, by_taxonomy in clipboard.content.tags.items()
+        }
+        assert clipboard_values[section_id] == ["one"]
+        assert clipboard_values[child_id] == ["three"]
+        assert all(not v for k, v in clipboard_values.items() if k not in (section_id, child_id))
+        # Copying does not delete the archived associations
+        assert ObjectTag.objects.filter(object_id=section_id, _value="two", archived=True).exists()
+        assert ObjectTag.objects.filter(object_id=child_id, _value="four", archived=True).exists()
+
     def test_copy_and_paste_container_another_library(self) -> None:
         """
         Test copying and then pasting a container (into a different library)

openedx/core/djangoapps/content_tagging/api.py

@@ -5,6 +5,8 @@
 
 import csv
 import io
+import logging
+from collections import Counter
 from itertools import groupby
 from typing import Iterator  # noqa: UP035
 
@@ -25,6 +27,8 @@
 from .types import ContentKey, TagValuesByObjectIdDict, TagValuesByTaxonomyIdDict, TaxonomyDict
 from .utils import check_taxonomy_context_key_org, get_content_key_from_string, get_context_key_from_key
 
+log = logging.getLogger(__name__)
+
 
 class InvalidOrgException(ValidationError):
     """
@@ -155,6 +159,7 @@ def get_all_object_tags(
 
     Includes any tags applied to the course/library as a whole.
     Returns a tuple with a dictionary of grouped object tag values for all blocks and a dictionary of taxonomies.
+    Archived associations, and those whose tag or taxonomy is archived, are left out.
 
     If `prefetch_orgs` is set, then the returned ObjectTag taxonomies will have their TaxonomyOrgs prefetched,
     which makes checking permissions faster.
@@ -176,10 +181,10 @@ def get_all_object_tags(
     if block_id_prefix:
         object_id_clause |= Q(object_id__startswith=block_id_prefix)
 
-    all_object_tags = ObjectTag.objects.filter(
+    all_object_tags = ObjectTag.objects.unarchived().filter(
         Q(tag__isnull=False, tag__taxonomy__isnull=False),
         object_id_clause,
-    ).select_related("tag__taxonomy").order_by("object_id")
+    ).select_related("tag__taxonomy").order_by("object_id", "id")
 
     if prefetch_orgs:
         all_object_tags = all_object_tags.prefetch_related("tag__taxonomy__taxonomyorg_set")
@@ -289,26 +294,40 @@ def export_tags_in_csv_file(object_id, file_dir, file_name) -> None:
 def set_exported_object_tags(
     content_key: ContentKey,
     exported_tags: TagValuesByTaxonomyIdDict,
-) -> None:
+) -> dict[str, int]:
     """
-    Sets the tags for the given exported content object.
+    Replace the object's tags with the tags from an export, keeping its hidden associations.
+
+    A hidden association is one that is archived, or whose tag or taxonomy is archived. ``exported_tags`` maps a
+    taxonomy export id to that taxonomy's tag values. Every other association of the object is cleared first, and a
+    deletion-locked one is archived instead of deleted. Tags from an archived taxonomy are skipped. Tags from a
+    taxonomy that does not exist on this instance become placeholder associations. ``delete_object_tags`` also
+    removes the hidden associations, and it too archives a deletion-locked one.
+
+    Returns the number of skipped tag values for each archived taxonomy, keyed by export id.
     """
     content_key_str = str(content_key)
+    skipped: dict[str, int] = {}
 
-    # Clear all tags related with the content.
-    oel_tagging.delete_object_tags(content_key_str)
+    oel_tagging.delete_object_tags(content_key_str, keep_hidden=True)
 
     for taxonomy_export_id, tags_values in exported_tags.items():
         if not tags_values:
             continue
 
-        taxonomy = oel_tagging.get_taxonomy_by_export_id(str(taxonomy_export_id))
+        export_id = str(taxonomy_export_id)
+        # Unfiltered on purpose: get_taxonomy_by_export_id() hides archived taxonomies, which are handled below.
+        taxonomy = Taxonomy.objects.filter(export_id=export_id).first()
+        if taxonomy is not None and taxonomy.archived:
+            # No placeholder either: ObjectTag.resync() would re-link it to the archived taxonomy.
+            skipped[export_id] = len(tags_values)
+            continue
         oel_tagging.tag_object(
             object_id=content_key_str,
             taxonomy=taxonomy,
             tags=tags_values,
             create_invalid=True,
-            taxonomy_export_id=str(taxonomy_export_id),
+            taxonomy_export_id=export_id,
         )
 
         # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED
@@ -329,6 +348,8 @@ def set_exported_object_tags(
             content_object=ContentObjectData(object_id=content_key_str)
         )
 
+    return skipped
+
 
 def import_course_tags_from_csv(csv_path, course_id) -> None:
     """
@@ -351,6 +372,7 @@ def get_exported_tags(block) -> TagValuesByTaxonomyIdDict:
         return result
 
     course_key = CourseKey.from_string(str(course_id))
+    skipped_by_taxonomy: Counter[str] = Counter()
 
     for block in tags_in_blocks:
         exported_tags = get_exported_tags(block)
@@ -361,10 +383,13 @@ def get_exported_tags(block) -> TagValuesByTaxonomyIdDict:
             raise ValueError(f"Invalid format of csv in: '{block}'.")
 
         if block_type == 'course':
-            set_exported_object_tags(course_key, exported_tags)
+            skipped_by_taxonomy.update(set_exported_object_tags(course_key, exported_tags))
         else:
             block_key = course_key.make_usage_key(block_type, block_id)
-            set_exported_object_tags(block_key, exported_tags)
+            skipped_by_taxonomy.update(set_exported_object_tags(block_key, exported_tags))
+
+    for export_id, count in skipped_by_taxonomy.items():
+        log.info("Course tag import for %s skipped %d tags from archived taxonomy %s", course_key, count, export_id)
 
 
 def copy_object_tags(

openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py

@@ -615,6 +615,35 @@ def test_list_taxonomy_query_count(self, user_attr: str, expected_queries: int):
             assert taxonomy["can_delete_taxonomy"] == user.is_staff  # not the metadata about the taxonomy.
             assert taxonomy["can_tag_object"]
 
+    def _get_tags_count(self, taxonomy: Taxonomy, query_params: dict) -> int:
+        """
+        Returns the "tags_count" that the list endpoint reports for the given taxonomy.
+        """
+        self.client.force_authenticate(user=self.staff)
+        response = self.client.get(TAXONOMY_ORG_LIST_URL, {**query_params, "page_size": 20}, format="json")
+        assert response.status_code == status.HTTP_200_OK
+        return next(t for t in response.data["results"] if t["id"] == taxonomy.id)["tags_count"]
+
+    @ddt.data(
+        ("tA1", {}, ("three",), 2),
+        ("tA1", {"org": "orgA"}, ("three",), 2),
+        ("ot1", {"unassigned": "true"}, ("three",), 2),
+        ("tA1", {}, ("one", "two", "three"), 0),
+    )
+    @ddt.unpack
+    def test_list_taxonomy_tags_count_excludes_archived(
+        self, taxonomy_attr: str, query_params: dict, archived_values: tuple[str, ...], expected_count: int
+    ) -> None:
+        """
+        Archived tags are hidden from the taxonomy's tag list, so they must not be counted either.
+        """
+        taxonomy = getattr(self, taxonomy_attr)
+        for value in ("one", "two", "three"):
+            Tag.objects.create(taxonomy=taxonomy, value=value)
+        Tag.objects.filter(taxonomy=taxonomy, value__in=archived_values).update(archived=True)
+
+        assert self._get_tags_count(taxonomy, query_params) == expected_count
+
     def test_list_taxonomy_type_mixed(self) -> None:
         """
         The list endpoint reports "taxonomy_type" per row for a mixed set of taxonomies:

openedx/core/djangoapps/content_tagging/rest_api/v1/views.py

@@ -7,7 +7,7 @@
 from typing import TYPE_CHECKING
 
 from django.core import exceptions
-from django.db.models import Count
+from django.db.models import Count, Q
 from django.http import StreamingHttpResponse
 from openedx_authz import api as authz_api
 from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
@@ -103,8 +103,8 @@ def get_queryset(self):
         # Prefetch taxonomyorgs so we can check permissions
         queryset = queryset.prefetch_related("taxonomyorg_set__org")
 
-        # Annotate with tags_count to avoid selecting all the tags
-        queryset = queryset.annotate(tags_count=Count("tag", distinct=True))
+        # Annotate with tags_count to avoid selecting all the tags. The count leaves archived tags out.
+        queryset = queryset.annotate(tags_count=Count("tag", filter=Q(tag__archived=False), distinct=True))
 
         # Select the competency taxonomy relation (if any) so is_competency_taxonomy()
         # costs no extra query per row when serializing taxonomy_type.