diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index e9f599772d..bc8f03f186 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -267,7 +267,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> empty, and (2) a summary of changes made to static files in the destination course. """ - from cms.djangoapps.contentstore.views.preview import _load_preview_block if not content_staging_api: @@ -324,6 +323,8 @@ def _import_xml_node_to_parent( Given an XML node representing a serialized XBlock (OLX), import it into modulestore 'store' as a child of the specified parent block. Recursively copy children as needed. """ + # pylint: disable=too-many-statements + runtime = parent_xblock.runtime parent_key = parent_xblock.scope_ids.usage_id block_type = node.tag @@ -429,7 +430,14 @@ def _import_xml_node_to_parent( ) # Copy content tags to the new xblock - if copied_from_block and tags: + if new_xblock.upstream: + # If this block is synced from an upstream (e.g. library content), + # copy the tags from the upstream as ready-only + content_tagging_api.copy_tags_as_read_only( + new_xblock.upstream, + new_xblock.location, + ) + elif copied_from_block and tags: object_tags = tags.get(str(copied_from_block)) if object_tags: content_tagging_api.set_all_object_tags( diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 5706b44e2c..a864b29025 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -184,7 +184,7 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): tagging_api.set_taxonomy_orgs(taxonomy_all_org, all_orgs=True) for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4', 'tag_5', 'tag_6', 'tag_7'): - Tag.objects.create(taxonomy=taxonomy_all_org, value=tag_value) + tagging_api.add_tag_to_taxonomy(taxonomy_all_org, tag_value) tagging_api.tag_object( object_id=str(unit_key), taxonomy=taxonomy_all_org, @@ -444,6 +444,18 @@ class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): self.course = CourseFactory.create(display_name='Course') + taxonomy_all_org = tagging_api.create_taxonomy( + "test_taxonomy", + "Test Taxonomy", + export_id="ALL_ORGS", + ) + tagging_api.set_taxonomy_orgs(taxonomy_all_org, all_orgs=True) + for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4', 'tag_5', 'tag_6', 'tag_7'): + tagging_api.add_tag_to_taxonomy(taxonomy_all_org, tag_value) + + self.lib_block_tags = ['tag_1', 'tag_5'] + tagging_api.tag_object(str(self.lib_block_key), taxonomy_all_org, self.lib_block_tags) + def test_paste_from_library_creates_link(self): """ When we copy a v2 lib block into a course, the dest block should be linked up to the lib block. @@ -464,6 +476,28 @@ class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): assert new_block.upstream_display_name == "MCQ-draft" assert new_block.upstream_max_attempts == 5 + def test_paste_from_library_read_only_tags(self): + """ + When we copy a v2 lib block into a course, the dest block should have read-only copied tags. + """ + + copy_response = self.client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(self.lib_block_key)}, format="json") + assert copy_response.status_code == 200 + + paste_response = self.client.post(XBLOCK_ENDPOINT, { + "parent_locator": str(self.course.usage_key), + "staged_content": "clipboard", + }, format="json") + assert paste_response.status_code == 200 + + new_block_key = paste_response.json()["locator"] + + object_tags = tagging_api.get_object_tags(new_block_key) + assert len(object_tags) == len(self.lib_block_tags) + for object_tag in object_tags: + assert object_tag.value in self.lib_block_tags + assert object_tag.is_copied + class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): """ diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py index 6a3af7455b..cc3d661ca6 100644 --- a/cms/lib/xblock/test/test_upstream_sync.py +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -13,6 +13,7 @@ from cms.lib.xblock.upstream_sync import ( ) from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as libs +from openedx.core.djangoapps.content_tagging import api as tagging_api from openedx.core.djangoapps.xblock import api as xblock from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory @@ -50,6 +51,18 @@ class UpstreamTestCase(ModuleStoreTestCase): libs.publish_changes(self.library.key, self.user.id) + self.taxonomy_all_org = tagging_api.create_taxonomy( + "test_taxonomy", + "Test Taxonomy", + export_id="ALL_ORGS", + ) + tagging_api.set_taxonomy_orgs(self.taxonomy_all_org, all_orgs=True) + for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4', 'tag_5', 'tag_6', 'tag_7'): + tagging_api.add_tag_to_taxonomy(self.taxonomy_all_org, tag_value) + + self.upstream_tags = ['tag_1', 'tag_5'] + tagging_api.tag_object(str(self.upstream_key), self.taxonomy_all_org, self.upstream_tags) + def test_sync_bad_downstream(self): """ Syncing into an unsupported downstream (such as a another Content Library block) raises BadDownstream, but @@ -129,11 +142,19 @@ class UpstreamTestCase(ModuleStoreTestCase): assert downstream.display_name == "Upstream Title V2" assert downstream.data == "Upstream content V2" + # Verify tags + object_tags = tagging_api.get_object_tags(str(downstream.location)) + assert len(object_tags) == len(self.upstream_tags) + for object_tag in object_tags: + assert object_tag.value in self.upstream_tags + # Upstream updates upstream = xblock.load_block(self.upstream_key, self.user) upstream.display_name = "Upstream Title V3" upstream.data = "Upstream content V3" upstream.save() + new_upstream_tags = self.upstream_tags + ['tag_2', 'tag_3'] + tagging_api.tag_object(str(self.upstream_key), self.taxonomy_all_org, new_upstream_tags) # Assert that un-published updates are not yet pulled into downstream sync_from_upstream(downstream, self.user) @@ -152,6 +173,12 @@ class UpstreamTestCase(ModuleStoreTestCase): assert downstream.display_name == "Upstream Title V3" assert downstream.data == "Upstream content V3" + # Verify tags + object_tags = tagging_api.get_object_tags(str(downstream.location)) + assert len(object_tags) == len(new_upstream_tags) + for object_tag in object_tags: + assert object_tag.value in new_upstream_tags + def test_sync_updates_to_modified_content(self): """ If we sync to modified content, will it preserve customizable fields, but overwrite the rest? @@ -357,3 +384,42 @@ class UpstreamTestCase(ModuleStoreTestCase): # AND, we have recorded the old upstream as our copied_from_block. assert downstream.copied_from_block == str(self.upstream_key) + + def test_sync_library_block_tags(self): + upstream_lib_block_key = libs.create_library_block(self.library.key, "html", "upstream").usage_key + upstream_lib_block = xblock.load_block(upstream_lib_block_key, self.user) + upstream_lib_block.display_name = "Another lib block" + upstream_lib_block.data = "another lib block" + upstream_lib_block.save() + + libs.publish_changes(self.library.key, self.user.id) + + expected_tags = self.upstream_tags + tagging_api.tag_object(str(upstream_lib_block_key), self.taxonomy_all_org, expected_tags) + + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(upstream_lib_block_key)) + + # Initial sync + sync_from_upstream(downstream, self.user) + + # Verify tags + object_tags = tagging_api.get_object_tags(str(downstream.location)) + assert len(object_tags) == len(expected_tags) + for object_tag in object_tags: + assert object_tag.value in expected_tags + + # Upstream updates + upstream_lib_block.display_name = "Upstream Title V3" + upstream_lib_block.data = "Upstream content V3" + upstream_lib_block.save() + new_upstream_tags = self.upstream_tags + ['tag_2', 'tag_3'] + tagging_api.tag_object(str(upstream_lib_block_key), self.taxonomy_all_org, new_upstream_tags) + + # Follow-up sync. + sync_from_upstream(downstream, self.user) + + #Verify tags + object_tags = tagging_api.get_object_tags(str(downstream.location)) + assert len(object_tags) == len(new_upstream_tags) + for object_tag in object_tags: + assert object_tag.value in new_upstream_tags diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 2ee22319a2..2b1082fa7a 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -183,6 +183,7 @@ def sync_from_upstream(downstream: XBlock, user: User) -> None: link, upstream = _load_upstream_link_and_block(downstream, user) _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=False) _update_non_customizable_fields(upstream=upstream, downstream=downstream) + _update_tags(upstream=upstream, downstream=downstream) downstream.upstream_version = link.version_available @@ -285,6 +286,19 @@ def _update_non_customizable_fields(*, upstream: XBlock, downstream: XBlock) -> setattr(downstream, field_name, new_upstream_value) +def _update_tags(*, upstream: XBlock, downstream: XBlock) -> None: + """ + Update tags from `upstream` to `downstream` + """ + from openedx.core.djangoapps.content_tagging.api import copy_tags_as_read_only + # For any block synced with an upstream, copy the tags as read_only + # This keeps tags added locally. + copy_tags_as_read_only( + str(upstream.location), + str(downstream.location), + ) + + def _get_synchronizable_fields(upstream: XBlock, downstream: XBlock) -> set[str]: """ The syncable fields are the ones which are content- or settings-scoped AND are defined on both (up,down)stream. diff --git a/openedx/core/djangoapps/content_tagging/api.py b/openedx/core/djangoapps/content_tagging/api.py index f015770e5d..8a06e483ab 100644 --- a/openedx/core/djangoapps/content_tagging/api.py +++ b/openedx/core/djangoapps/content_tagging/api.py @@ -440,3 +440,4 @@ delete_object_tags = oel_tagging.delete_object_tags resync_object_tags = oel_tagging.resync_object_tags get_object_tags = oel_tagging.get_object_tags add_tag_to_taxonomy = oel_tagging.add_tag_to_taxonomy +copy_tags_as_read_only = oel_tagging.copy_tags diff --git a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py index 8bd2623085..849d3891f2 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py @@ -7,6 +7,7 @@ from __future__ import annotations from rest_framework import serializers, fields from openedx_tagging.core.tagging.rest_api.v1.serializers import ( + ObjectTagMinimalSerializer, TaxonomyListQueryParamsSerializer, TaxonomySerializer, ) @@ -94,3 +95,24 @@ class TaxonomyOrgSerializer(TaxonomySerializer): model = TaxonomySerializer.Meta.model fields = TaxonomySerializer.Meta.fields + ["orgs", "all_orgs"] read_only_fields = ["orgs", "all_orgs"] + + +class ObjectTagCopiedMinimalSerializer(ObjectTagMinimalSerializer): + """ + Serializer for Object Tags. + + This override `get_can_delete_objecttag` to avoid delete + object tags if is copied. + """ + + def get_can_delete_objecttag(self, instance): + """ + Verify if the user can delete the object tag. + + Override to return `False` if the object tag is copied. + """ + if instance.is_copied: + # The user can't delete copied tags. + return False + + return super().get_can_delete_objecttag(instance) 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 e386ee2342..463ea42d08 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 @@ -1850,6 +1850,31 @@ class TestObjectTagViewSet(TestObjectTagMixin, APITestCase): assert status.is_success(response3.status_code) assert response3.data[str(self.courseA)]["taxonomies"] == expected_tags + def test_get_copied_tags(self): + self.client.force_authenticate(user=self.staffB) + + object_id_1 = str(self.courseA) + object_id_2 = str(self.courseB) + tagging_api.tag_object(object_id=object_id_1, taxonomy=self.t1, tags=["android"]) + tagging_api.tag_object(object_id=object_id_2, taxonomy=self.t1, tags=["anvil"]) + tagging_api.copy_tags_as_read_only(object_id_1, object_id_2) + + expected_tags = [{ + 'name': self.t1.name, + 'taxonomy_id': self.t1.pk, + 'can_tag_object': True, + 'export_id': self.t1.export_id, + 'tags': [ + {'value': 'android', 'lineage': ['ALPHABET', 'android'], 'can_delete_objecttag': False}, + {'value': 'anvil', 'lineage': ['ALPHABET', 'anvil'], 'can_delete_objecttag': True} + ] + }] + + get_url = OBJECT_TAGS_URL.format(object_id=self.courseB) + response = self.client.get(get_url, format="json") + assert status.is_success(response.status_code) + assert response.data[str(object_id_2)]["taxonomies"] == expected_tags + @ddt.data( ('staff', 'courseA', 8), ('staff', 'libraryA', 8), 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 3fc99736ba..c2f79ef677 100644 --- a/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py +++ b/openedx/core/djangoapps/content_tagging/rest_api/v1/views.py @@ -31,7 +31,12 @@ from ...api import ( ) from ...rules import get_admin_orgs from .filters import ObjectTagTaxonomyOrgFilterBackend, UserOrgFilterBackend -from .serializers import TaxonomyOrgListQueryParamsSerializer, TaxonomyOrgSerializer, TaxonomyUpdateOrgBodySerializer +from .serializers import ( + ObjectTagCopiedMinimalSerializer, + TaxonomyOrgListQueryParamsSerializer, + TaxonomyOrgSerializer, + TaxonomyUpdateOrgBodySerializer, +) class TaxonomyOrgView(TaxonomyView): @@ -148,6 +153,7 @@ class ObjectTagOrgView(ObjectTagView): Refer to ObjectTagView docstring for usage details. """ + minimal_serializer_class = ObjectTagCopiedMinimalSerializer filter_backends = [ObjectTagTaxonomyOrgFilterBackend] def update(self, request, *args, **kwargs) -> Response: diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 43efc6043e..79d4f45377 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -146,7 +146,7 @@ optimizely-sdk<5.0 # Date: 2023-09-18 # pinning this version to avoid updates while the library is being developed # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-learning==0.15.0 +openedx-learning==0.16.0 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f783caabe3..587e7310f6 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -825,7 +825,7 @@ openedx-filters==1.11.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-learning==0.15.0 +openedx-learning==0.16.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index f8f8128322..40fd29ba83 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1375,7 +1375,7 @@ openedx-filters==1.11.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-learning==0.15.0 +openedx-learning==0.16.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 7e41b67c73..6686a11b78 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -984,7 +984,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.15.0 +openedx-learning==0.16.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 890d978125..debf11d1a7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1035,7 +1035,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-learning==0.15.0 +openedx-learning==0.16.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt