Skip to content

Commit

Permalink
feat: Copy tags when sync library [FC-0062] (#35596)
Browse files Browse the repository at this point in the history
* feat: Copy tags when sync library

* feat: Avoid delete object tag if is copied

* chore: Bump version of openedx-learning to 0.16.0

* test: Tests for copy paste library blocks

* feat: Sync tags when sync upstream
  • Loading branch information
ChrisChV authored Oct 21, 2024
1 parent 123ad8d commit 9e14566
Show file tree
Hide file tree
Showing 13 changed files with 185 additions and 9 deletions.
12 changes: 10 additions & 2 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
36 changes: 35 additions & 1 deletion cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _setup_tagged_content(self, course_key) -> dict:

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,
Expand Down Expand Up @@ -444,6 +444,18 @@ def setUp(self):

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.
Expand All @@ -464,6 +476,28 @@ def test_paste_from_library_creates_link(self):
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):
"""
Expand Down
66 changes: 66 additions & 0 deletions cms/lib/xblock/test/test_upstream_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,6 +51,18 @@ def setUp(self):

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
Expand Down Expand Up @@ -129,11 +142,19 @@ def test_sync_updates_happy_path(self):
assert downstream.display_name == "Upstream Title V2"
assert downstream.data == "<html><body>Upstream content V2</body></html>"

# 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 = "<html><body>Upstream content V3</body></html>"
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)
Expand All @@ -152,6 +173,12 @@ def test_sync_updates_happy_path(self):
assert downstream.display_name == "Upstream Title V3"
assert downstream.data == "<html><body>Upstream content V3</body></html>"

# 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?
Expand Down Expand Up @@ -357,3 +384,42 @@ 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 = "<html>another lib block</html>"
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 = "<html><body>Upstream content V3</body></html>"
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
14 changes: 14 additions & 0 deletions cms/lib/xblock/upstream_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_tagging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,3 +440,4 @@ def tag_object(
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
22 changes: 22 additions & 0 deletions openedx/core/djangoapps/content_tagging/rest_api/v1/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from rest_framework import serializers, fields

from openedx_tagging.core.tagging.rest_api.v1.serializers import (
ObjectTagMinimalSerializer,
TaxonomyListQueryParamsSerializer,
TaxonomySerializer,
)
Expand Down Expand Up @@ -94,3 +95,24 @@ class Meta:
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)
Original file line number Diff line number Diff line change
Expand Up @@ -1850,6 +1850,31 @@ def test_get_tags(self):
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),
Expand Down
8 changes: 7 additions & 1 deletion openedx/core/djangoapps/content_tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@
)
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):
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9e14566

Please sign in to comment.