Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Copy tags when sync library [FC-0062] #35596

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:/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
Loading