From 7a1797af86a4f85adaccc2716202466b1284b6ab Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 17 Oct 2024 20:10:49 -0500 Subject: [PATCH] feat: Sync tags when sync upstream --- cms/djangoapps/contentstore/helpers.py | 2 +- .../views/tests/test_clipboard_paste.py | 4 +- cms/lib/xblock/test/test_upstream_sync.py | 64 +++++++++++++++++++ cms/lib/xblock/upstream_sync.py | 18 +++++- 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index c7de661eba5b..05c79961c13d 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -438,7 +438,7 @@ def copy_tags(): ) # Copy content tags to the new xblock - if (new_xblock.upstream): + if new_xblock.upstream: # Verify if the upstream is a library component # Copy the tags from library component upstream as ready only try: diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 3827a48940a5..0f2ec95f7fcb 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -454,7 +454,7 @@ def setUp(self): 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, ['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): """ @@ -495,9 +495,7 @@ def test_paste_from_library_read_only_tags(self): object_tags = tagging_api.get_object_tags(new_block_key) assert len(object_tags) == len(self.lib_block_tags) - print(object_tags) for object_tag in object_tags: - print(object_tag) assert object_tag.value in self.lib_block_tags assert object_tag.is_copied diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py index 5db020393eab..5596f3eb86a9 100644 --- a/cms/lib/xblock/test/test_upstream_sync.py +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -13,6 +13,7 @@ ) 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 @@ -48,6 +49,18 @@ def setUp(self): upstream.data = "Upstream content V2" upstream.save() + 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 @@ -127,11 +140,19 @@ def test_sync_updates_happy_path(self): 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) # Follow-up sync. Assert that updates are pulled into downstream. sync_from_upstream(downstream, self.user) @@ -140,6 +161,12 @@ def test_sync_updates_happy_path(self): 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? @@ -335,3 +362,40 @@ def test_sever_upstream_link(self): # 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() + + 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 785cc7dc7e36..fb0199788f12 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -28,7 +28,6 @@ if t.TYPE_CHECKING: from django.contrib.auth.models import User # pylint: disable=imported-auth-user - logger = logging.getLogger(__name__) @@ -187,6 +186,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 @@ -284,6 +284,22 @@ 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_object_tags, copy_tags_as_read_only + if isinstance(upstream.location, LibraryUsageLocatorV2): + # If is a library component, then update the tags as read_only + # This keeps tags added locally. + copy_tags_as_read_only( + str(upstream.location), + str(downstream.location), + ) + else: + copy_object_tags(upstream.location, 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.