diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index 525e0edc26fc..4d6a3e661507 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/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,10 +1563,12 @@ def test_duplicate_tags(self): expected_chapter_tags = [ f' {str(dupe_chapter.location)}: A=one', f' {str(dupe_chapter.location)}: A=two', - f' {str(dupe_chapter.location)}: B=four', f' {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' {str(dupe_block.location)}: A=two', @@ -1566,6 +1576,9 @@ def test_duplicate_tags(self): 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): diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 4a604dae7c2d..8e0a870cbaf9 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/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' {dest_unit_key}: test_taxonomy=tag_1' - assert str(tags[1]) == f' {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. diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index d2c333fce120..7f64cba09554 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/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) diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index ecfd04a91d86..727970edaa48 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/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( diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py index 91494a696b7e..fda058ee1c17 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/tests/test_views.py +++ b/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: diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py index b49109fc1d3c..fcf54c14dfa4 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/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. diff --git a/openedx/core/djangoapps/content_tagging/tests/test_api.py b/openedx/core/djangoapps/content_tagging/tests/test_api.py index ea97706a9d1d..069c0d5f57ee 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_api.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_api.py @@ -1,15 +1,20 @@ """Tests for the Tagging models""" +import copy import io import os import tempfile +from unittest import mock import ddt import pytest +from django.db.models import Q from django.test.testcases import TestCase from fs.osfs import OSFS from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator, LibraryLocatorV2 -from openedx_tagging.models import ObjectTag +from openedx_tagging.import_export.api import import_tags +from openedx_tagging.import_export.parsers import ParserFormat +from openedx_tagging.models import ObjectTag, Tag, Taxonomy from organizations.models import Organization from .. import api @@ -245,6 +250,7 @@ def test_get_tags(self): assert result[0]["depth"] == 0 +@ddt.ddt class TestAPIObjectTags(TestGetAllObjectTagsMixin, TestCase): """ Tests object tag API functions. @@ -306,6 +312,14 @@ def test_get_course_object_tags_with_add_tags(self): self.taxonomy_2.id: [tag.value for tag in vertical2_tags], } + # The order of values within a taxonomy is not guaranteed, and this test is about no tags being lost. + for tags_by_taxonomy in ( + object_tags["block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1"], + self.expected_course_objecttags["block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical1"], + ): + for values in tags_by_taxonomy.values(): + values.sort() + assert object_tags == self.expected_course_objecttags assert taxonomies == { self.taxonomy_1.id: self.taxonomy_1, @@ -327,6 +341,67 @@ def test_get_library_object_tags(self): self.taxonomy_2.id: self.taxonomy_2, } + def test_get_library_object_tags_excludes_archived_association(self): + problem_id = f"lb:orgA:lib_{self.block_suffix}:problem:problem1_{self.block_suffix}" + ObjectTag.objects.filter(object_id=problem_id, _value="Tag 1.1").update(archived=True) + + object_tags, taxonomies = api.get_all_object_tags( + LibraryLocatorV2.from_string(f"lib:orgA:lib_{self.block_suffix}") + ) + + expected = copy.deepcopy(self.expected_library_objecttags) + # The problem has no other tags, so it is dropped from the result. + del expected[problem_id] + assert object_tags == expected + assert taxonomies == { + self.taxonomy_1.id: self.taxonomy_1, + self.taxonomy_2.id: self.taxonomy_2, + } + + @ddt.data( + ("association", ["Tag 1.2"]), + ("tag", ["Tag 1.1"]), + ) + @ddt.unpack + def test_get_course_object_tags_excludes_archived_association_or_tag( + self, archived: str, expected_sequential_values: list[str] + ): + sequential_id = "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential" + if archived == "association": + ObjectTag.objects.filter(object_id=sequential_id, _value="Tag 1.1").update(archived=True) + else: + Tag.objects.filter(pk=self.tag_1_2.pk).update(archived=True) + + object_tags, taxonomies = api.get_all_object_tags( + CourseKey.from_string("course-v1:orgA+test_course+test_run") + ) + + expected = copy.deepcopy(self.expected_course_objecttags) + expected[sequential_id][self.taxonomy_1.id] = expected_sequential_values + assert object_tags == expected + assert taxonomies == { + self.taxonomy_1.id: self.taxonomy_1, + self.taxonomy_2.id: self.taxonomy_2, + } + + def test_get_course_object_tags_excludes_archived_taxonomy(self): + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + + object_tags, taxonomies = api.get_all_object_tags( + CourseKey.from_string("course-v1:orgA+test_course+test_run") + ) + + # The vertical and html blocks only had taxonomy_2 tags, so they have no entry at all. + assert object_tags == { + "course-v1:orgA+test_course+test_run": self.expected_course_objecttags[ + "course-v1:orgA+test_course+test_run" + ], + "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential": { + self.taxonomy_1.id: ["Tag 1.1", "Tag 1.2"], + }, + } + assert taxonomies == {self.taxonomy_1.id: self.taxonomy_1} + def _test_copy_object_tags(self, src_key, dst_key, expected_tags): """ Test copying object tags to a new object. @@ -431,6 +506,7 @@ def test_tag_container_invalid_org(self) -> None: api.tag_object(object_id=str(unit_key_org_b), taxonomy=self.taxonomy_3, tags=[tag]) +@ddt.ddt class TestExportImportTags(TaggedCourseMixin): """ Tests for export/import functions @@ -453,6 +529,59 @@ def test_generate_csv_rows(self) -> None: assert csv_content == self.expected_csv + def test_generate_csv_rows_excludes_archived(self) -> None: + sequential_id = "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential" + ObjectTag.objects.filter(object_id=sequential_id, _value="Tag 1.1").update(archived=True) + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + + buffer = io.StringIO() + list(api.generate_csv_rows(str(self.course.id), buffer)) + buffer.seek(0) + csv_content = buffer.getvalue() + + assert csv_content == ( + '"Name","Type","ID","1-taxonomy-1"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1"\r\n' + '" test sequential","sequential","test_sequential","Tag 1.2"\r\n' + '" test vertical1","vertical","test_vertical1",""\r\n' + '" test vertical2","vertical","test_vertical2",""\r\n' + '" test html","html","test_html",""\r\n' + '" untagged sequential","sequential","untagged_sequential",""\r\n' + '" untagged vertical","vertical","untagged_vertical",""\r\n' + ) + + def test_generate_csv_rows_all_archived(self) -> None: + ObjectTag.objects.update(archived=True) + + buffer = io.StringIO() + list(api.generate_csv_rows(str(self.course.id), buffer)) + buffer.seek(0) + csv_content = buffer.getvalue() + + # Nothing is left to export, so no taxonomy columns remain, but every block still has a row. + assert csv_content == ( + '"Name","Type","ID"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run"\r\n' + '" test sequential","sequential","test_sequential"\r\n' + '" test vertical1","vertical","test_vertical1"\r\n' + '" test vertical2","vertical","test_vertical2"\r\n' + '" test html","html","test_html"\r\n' + '" untagged sequential","sequential","untagged_sequential"\r\n' + '" untagged vertical","vertical","untagged_vertical"\r\n' + ) + + def test_generate_csv_rows_keeps_disabled_taxonomy(self) -> None: + # Disabled taxonomies are not archived, so their tags are still exported. + self.taxonomy_1.enabled = False + self.taxonomy_1.save() + + buffer = io.StringIO() + list(api.generate_csv_rows(str(self.course.id), buffer)) + buffer.seek(0) + csv_content = buffer.getvalue() + + assert csv_content == self.expected_csv + def test_export_tags_in_csv_file(self) -> None: file_dir_name = tempfile.mkdtemp() file_dir = OSFS(file_dir_name) @@ -530,3 +659,173 @@ def test_import_tags_invalid_taxonomy(self) -> None: assert object_tag.value == 'Tag 1.11' assert object_tag.taxonomy is None assert object_tag.export_id == '1-taxonomy-1-1' + + def _course_object_tags(self, course_key: CourseKey | str): + """ + Returns every association, hidden or not, of the course and its blocks. + """ + block_id_prefix = str(course_key).replace("course-v1:", "block-v1:", 1) + return ObjectTag.objects.filter(Q(object_id=str(course_key)) | Q(object_id__startswith=block_id_prefix)) + + def test_export_then_import_into_new_course_carries_only_visible_tags(self) -> None: + sequential_id = "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential" + ObjectTag.objects.filter(object_id=sequential_id, _value="Tag 1.1").update(archived=True) + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + buffer = io.StringIO() + list(api.generate_csv_rows(str(self.course.id), buffer)) + csv_path = self._create_csv_file(buffer.getvalue()) + + # The import only builds usage keys from the CSV, so the second course needs no blocks of its own. + second_course_key = CourseKey.from_string("course-v1:orgA+test_course+second_run") + api.import_course_tags_from_csv(csv_path, second_course_key) + + second_course_tags = self._course_object_tags(second_course_key) + assert sorted(second_course_tags.values_list("object_id", "taxonomy_id", "_value")) == sorted([ + ("course-v1:orgA+test_course+second_run", self.taxonomy_1.id, "Tag 1.1"), + ( + "block-v1:orgA+test_course+second_run+type@sequential+block@test_sequential", + self.taxonomy_1.id, + "Tag 1.2", + ), + ]) + assert not second_course_tags.filter(Q(taxonomy=self.taxonomy_2) | Q(_export_id="2-taxonomy-2")).exists() + + def _hide_association(self, hidden_by: str, hidden_pk: int) -> None: + """ + Hides the given association by archiving the association, its tag, or its taxonomy. + """ + if hidden_by == "association": + ObjectTag.objects.filter(pk=hidden_pk).update(archived=True) + elif hidden_by == "tag": + Tag.objects.filter(objecttag__pk=hidden_pk).update(archived=True) + else: + Taxonomy.objects.filter(objecttag__pk=hidden_pk).update(archived=True) + + @ddt.data( + ("association", False), + ("tag", False), + ("taxonomy", False), + # An archived taxonomy is skipped before tag_object() runs, so it has no same-taxonomy case. + ("association", True), + ("tag", True), + ) + @ddt.unpack + def test_import_keeps_hidden_association(self, hidden_by: str, visible_in_same_taxonomy: bool) -> None: + course_id = str(self.course.id) + api.tag_object(object_id=course_id, taxonomy=self.taxonomy_2, tags=["Tag 2.1"]) + hidden_pk = ObjectTag.objects.get(object_id=course_id, _value="Tag 2.1").pk + self._hide_association(hidden_by, hidden_pk) + + if visible_in_same_taxonomy: + # tag_object() runs on the taxonomy that holds the hidden row, and must leave that row alone. + row = '"Test Course","course","course-v1:orgA+test_course+test_run","","Tag 2.2"\r\n' + expected_visible = ["Tag 2.2"] + else: + row = '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.2",""\r\n' + expected_visible = ["Tag 1.2"] + csv_path = self._create_csv_file('"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + row) + api.import_course_tags_from_csv(csv_path, self.course.id) + + visible_tags = list(api.get_object_tags(self.course.id)) + assert [t.value for t in visible_tags] == expected_visible + assert ObjectTag.objects.filter(object_id=course_id, _value="Tag 2.1").count() == 1 + assert ObjectTag.objects.filter(pk=hidden_pk).exists() # The same row survives. + assert hidden_pk not in [t.pk for t in visible_tags] # It is still hidden. + + def test_import_clears_placeholder_and_deleted_reference_rows(self) -> None: + course_id = str(self.course.id) + # A placeholder for a taxonomy that does not exist, and a row whose tag was deleted. + ObjectTag.objects.create(object_id=course_id, taxonomy=None, tag=None, _value="X", _export_id="gone-taxonomy") + ObjectTag.objects.create(object_id=course_id, taxonomy=self.taxonomy_1, tag=None, _value="Tag 1.11") + + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1"\r\n' + ) + api.import_course_tags_from_csv(csv_path, self.course.id) + + assert not ObjectTag.objects.filter(object_id=course_id, _export_id="gone-taxonomy").exists() + assert not ObjectTag.objects.filter(object_id=course_id, _value="Tag 1.11").exists() + assert [t.value for t in api.get_object_tags(self.course.id)] == ["Tag 1.1"] + assert ObjectTag.objects.filter(object_id=course_id).count() == 1 + + def test_import_archives_locked_visible_association(self) -> None: + course_id = str(self.course.id) + locked_pk = ObjectTag.objects.get(object_id=course_id, _value="Tag 1.1").pk + ObjectTag.objects.filter(pk=locked_pk).update(deletion_locked=True) + + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.2"\r\n' + ) + api.import_course_tags_from_csv(csv_path, self.course.id) + + assert ObjectTag.objects.get(pk=locked_pk).archived + assert [t.value for t in api.get_object_tags(self.course.id)] == ["Tag 1.2"] + + def test_import_skips_archived_taxonomy(self) -> None: + course_id = str(self.course.id) + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1","Tag 2.1"\r\n' + ) + + api.import_course_tags_from_csv(csv_path, self.course.id) + + assert [t.value for t in api.get_object_tags(course_id)] == ["Tag 1.1"] + # No placeholder either: resyncing it would link it to the archived taxonomy. + assert not ObjectTag.objects.filter(object_id=course_id, taxonomy=self.taxonomy_2).exists() + assert not ObjectTag.objects.filter(object_id=course_id, _export_id="2-taxonomy-2").exists() + + def test_import_skipping_archived_taxonomy_sends_no_event_for_it(self) -> None: + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","Tag 1.1","Tag 2.1"\r\n' + ) + + with mock.patch( + "openedx.core.djangoapps.content_tagging.api.CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event" + ) as send_event: + api.import_course_tags_from_csv(csv_path, self.course.id) + + send_event.assert_called_once() + assert send_event.call_args.kwargs["content_object"].object_id == str(self.course.id) + + def test_import_logs_skipped_archived_taxonomy_once(self) -> None: + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+test_run","","Tag 2.1;Tag 2.2"\r\n' + '" test sequential","sequential","test_sequential","","Tag 2.1"\r\n' + ) + + with self.assertLogs("openedx.core.djangoapps.content_tagging.api", level="INFO") as logs: + api.import_course_tags_from_csv(csv_path, self.course.id) + + messages = [record.getMessage() for record in logs.records if "2-taxonomy-2" in record.getMessage()] + assert len(messages) == 1 + # Two values on the course row and one on the block row. + assert "skipped 3 tags" in messages[0] + + def test_taxonomy_tag_import_does_not_restore_skipped_archived_taxonomy_tags(self) -> None: + new_course_key = CourseKey.from_string("course-v1:orgA+test_course+second_run") + Taxonomy.objects.filter(pk=self.taxonomy_2.pk).update(archived=True) + csv_path = self._create_csv_file( + '"Name","Type","ID","1-taxonomy-1","2-taxonomy-2"\r\n' + '"Test Course","course","course-v1:orgA+test_course+second_run","Tag 1.1","Tag 2.1"\r\n' + '" test sequential","sequential","test_sequential","Tag 1.2","Tag 2.2"\r\n' + ) + api.import_course_tags_from_csv(csv_path, new_course_key) + + # The same calls the taxonomy tag-file import endpoint makes. + import_success, _task, _plan = import_tags( + self.taxonomy_1, io.BytesIO(b"id,value\r\ntag_1_3,Tag 1.3\r\n"), ParserFormat.CSV + ) + assert import_success + api.resync_object_tags() + + new_course_tags = self._course_object_tags(new_course_key) + assert new_course_tags.count() == 2 + assert not new_course_tags.filter(Q(taxonomy=self.taxonomy_2) | Q(_export_id="2-taxonomy-2")).exists()