Skip to content

Commit

Permalink
refactor: store copied block tags in StagedContent.tags
Browse files Browse the repository at this point in the history
Adds a new JSON field to StagedContent to store a dict of serialized
object tags from the copied block and its children, if any.

These tags are deserialized and applied to the new block(s) on paste.

To support this, we had to make 2 changes to XBlockSerializer:

* XBlockSerializer now serializes the block's (and it's child block's)
  tags into a dict, keyed off the block usage key. These serialized tags
  are stored in StagedContent.tags
* XBlockSerializer now inserts a url_name attribute when serializing
  blocks that don't provide one.
  When pasting serialized blocks, the url_name is used to reconstruct
  the original usage keys of any serialized children, so we can identify
  their tags.

Other related changes:

* Updates import_staged_content_from_user_clipboard's helper method to
  reconstruct child usage keys, so we can determine which tags apply to
  the pasted block(s).
* Updates ClipboardPasteTestCase to test copying child tags
* Removes TaggedBlockMixin, and moves its object tag serialization logic
  into content_tagging.api
  • Loading branch information
pomegranited committed Mar 22, 2024
1 parent 16192fc commit 7702971
Show file tree
Hide file tree
Showing 15 changed files with 260 additions and 178 deletions.
53 changes: 52 additions & 1 deletion cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from attrs import frozen, Factory
from django.conf import settings
from django.utils.translation import gettext as _
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
from opaque_keys.edx.locator import DefinitionLocator, LocalId
from xblock.core import XBlock
Expand All @@ -24,6 +25,7 @@
from cms.djangoapps.models.settings.course_grading import CourseGradingModel
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
import openedx.core.djangoapps.content_staging.api as content_staging_api
import openedx.core.djangoapps.content_tagging.api as content_tagging_api

from .utils import reverse_course_url, reverse_library_url, reverse_usage_url

Expand Down Expand Up @@ -284,6 +286,7 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->
user_id=request.user.id,
slug_hint=user_clipboard.source_usage_key.block_id,
copied_from_block=str(user_clipboard.source_usage_key),
tags=user_clipboard.content.tags,
)
# Now handle static files that need to go into Files & Uploads:
notices = _import_files_into_course(
Expand All @@ -306,6 +309,8 @@ def _import_xml_node_to_parent(
slug_hint: str | None = None,
# UsageKey of the XBlock that this one is a copy of
copied_from_block: str | None = None,
# Content tags applied to the source XBlock(s)
tags: dict[str, str] | None = None,
) -> XBlock:
"""
Given an XML node representing a serialized XBlock (OLX), import it into modulestore 'store' as a child of the
Expand Down Expand Up @@ -376,7 +381,25 @@ def _import_xml_node_to_parent(

if not children_handled:
for child_node in child_nodes:
_import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id)
child_copied_from = _get_usage_key_from_node(child_node, copied_from_block) if copied_from_block else None
_import_xml_node_to_parent(
child_node,
new_xblock,
store,
user_id=user_id,
copied_from_block=str(child_copied_from),
tags=tags,
)

# Copy content tags to the new xblock
if copied_from_block and tags:
serialized_object_tags = tags.get(str(copied_from_block))
if serialized_object_tags:
object_tags = content_tagging_api.deserialize_object_tags(serialized_object_tags)
content_tagging_api.set_object_tags(
content_key=new_xblock.location,
object_tags=object_tags,
)

return new_xblock

Expand Down Expand Up @@ -471,3 +494,31 @@ def is_item_in_course_tree(item):
ancestor = ancestor.get_parent()

return ancestor is not None


def _get_usage_key_from_node(node, parent_id: str) -> UsageKey | None:
"""
Returns the UsageKey for the given node and parent ID.
If the parent_id is not a valid UsageKey, or there's no "url_name" attribute in the node, then will return None.
"""
try:
parent_key = UsageKey.from_string(parent_id)
# We're constructing a CourseKey here because they support make_usage_key,
# but this will work even if the parent context_key is a library,
# cf opaque_keys test_lib_key_make_usage_key
parent_context = CourseKey.from_string(str(parent_key.context_key))
except InvalidKeyError:
log.warning("Invalid usage key provided: %s", parent_id)

usage_key = None
block_id = node.attrib.get("url_name")
block_type = node.tag

if parent_context and block_id and block_type:
usage_key = parent_context.make_usage_key(
block_type=block_type,
block_id=block_id,
)

return usage_key
65 changes: 51 additions & 14 deletions cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,33 +158,50 @@ def test_copy_and_paste_unit_with_tags(self):
taxonomy_all_org = tagging_api.create_taxonomy("test_taxonomy", "Test Taxonomy")

tagging_api.set_taxonomy_orgs(taxonomy_all_org, all_orgs=True)
Tag.objects.create(taxonomy=taxonomy_all_org, value="tag_1")
Tag.objects.create(taxonomy=taxonomy_all_org, value="tag_2")
for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4'):
Tag.objects.create(taxonomy=taxonomy_all_org, value=tag_value)
tagging_api.tag_object(
object_id=str(unit_key),
taxonomy=taxonomy_all_org,
tags=["tag_1", "tag_2"],
)

# Tag some sub-blocks with different tags
video_block_key = course_key.make_usage_key("video", "sample_video")
tagging_api.tag_object(
object_id=str(video_block_key),
taxonomy=taxonomy_all_org,
tags=["tag_3"],
)
poll_block_key = course_key.make_usage_key("poll_question", "T1_changemind_poll_foo_2")
tagging_api.tag_object(
object_id=str(poll_block_key),
taxonomy=taxonomy_all_org,
tags=["tag_4"],
)

# Tag our blocks using a taxonomy we'll remove before pasting -- so these tags won't be pasted
taxonomy_all_org_removed = tagging_api.create_taxonomy("test_taxonomy_removed", "Test Taxonomy Removed")
tagging_api.set_taxonomy_orgs(taxonomy_all_org_removed, all_orgs=True)
Tag.objects.create(taxonomy=taxonomy_all_org_removed, value="tag_1")
Tag.objects.create(taxonomy=taxonomy_all_org_removed, value="tag_2")
tagging_api.tag_object(
object_id=str(unit_key),
taxonomy=taxonomy_all_org_removed,
tags=["tag_1", "tag_2"],
)
tagging_api.get_object_tags(str(unit_key))

for object_key in (unit_key, video_block_key, poll_block_key):
tagging_api.tag_object(
object_id=str(object_key),
taxonomy=taxonomy_all_org_removed,
tags=["tag_1", "tag_2"],
)

# Tag our blocks using a taxonomy that isn't enabled for any orgs -- these tags won't be pasted
taxonomy_no_org = tagging_api.create_taxonomy("test_taxonomy_no_org", "Test Taxonomy No Org")
Tag.objects.create(taxonomy=taxonomy_no_org, value="tag_1")
Tag.objects.create(taxonomy=taxonomy_no_org, value="tag_2")
tagging_api.tag_object(
object_id=str(unit_key),
taxonomy=taxonomy_no_org,
tags=["tag_1", "tag_2"],
)
for object_key in (unit_key, video_block_key, poll_block_key):
tagging_api.tag_object(
object_id=str(object_key),
taxonomy=taxonomy_no_org,
tags=["tag_1", "tag_2"],
)

# Copy the unit
copy_response = client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(unit_key)}, format="json")
Expand All @@ -206,6 +223,26 @@ def test_copy_and_paste_unit_with_tags(self):
assert str(tags[0]) == f'<ObjectTag> {dest_unit_key}: test_taxonomy=tag_1'
assert str(tags[1]) == f'<ObjectTag> {dest_unit_key}: test_taxonomy=tag_2'

# Ensure that the pasted child blocks were tagged too.
all_tags, _ = tagging_api.get_all_object_tags(dest_course.id)
dest_video_id = None
dest_poll_id = None
for object_id in all_tags:
if 'video' in object_id:
dest_video_id = object_id
elif 'poll' in object_id:
dest_poll_id = object_id

assert dest_video_id, f"No tags pasted from {video_block_key}"
video_tags = all_tags[dest_video_id]
assert len(video_tags) == 1
assert str(video_tags[taxonomy_all_org.id]) == f'[<ObjectTag> {dest_video_id}: test_taxonomy=tag_3]'

assert dest_poll_id, f"No tags pasted from {poll_block_key}"
poll_tags = all_tags[dest_poll_id]
assert len(poll_tags) == 1
assert str(poll_tags[taxonomy_all_org.id]) == f'[<ObjectTag> {dest_poll_id}: test_taxonomy=tag_4]'

def test_paste_with_assets(self):
"""
When pasting into a different course, any required static assets should
Expand Down
2 changes: 0 additions & 2 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@

from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin
from cms.lib.xblock.authoring_mixin import AuthoringMixin
from cms.lib.xblock.tagging.tagged_block_mixin import TaggedBlockMixin
from xmodule.modulestore.edit_info import EditInfoMixin
from openedx.core.djangoapps.theming.helpers_dirs import (
get_themes_unchecked,
Expand Down Expand Up @@ -986,7 +985,6 @@
XModuleMixin,
EditInfoMixin,
AuthoringMixin,
TaggedBlockMixin,
)
XBLOCK_EXTRA_MIXINS = ()

Expand Down
119 changes: 0 additions & 119 deletions cms/lib/xblock/tagging/tagged_block_mixin.py

This file was deleted.

2 changes: 1 addition & 1 deletion openedx/core/djangoapps/content_staging/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class StagedContentAdmin(admin.ModelAdmin):
list_display = ('id', 'user', 'created', 'purpose', 'status', 'block_type', 'display_name', 'suggested_url_name')
list_filter = ('purpose', 'status', 'block_type')
search_fields = ('user__username', 'display_name', 'suggested_url_name')
readonly_fields = ('id', 'user', 'created', 'purpose', 'status', 'block_type', 'olx')
readonly_fields = ('id', 'user', 'created', 'purpose', 'status', 'block_type', 'olx', 'tags')
inlines = (StagedContentFileInline, )


Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_staging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardDa
status=content.status,
block_type=content.block_type,
display_name=content.display_name,
tags=content.tags,
),
source_usage_key=clipboard.source_usage_key,
)
Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_staging/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class StagedContentData:
status: StagedContentStatus = field(validator=validators.in_(StagedContentStatus), converter=StagedContentStatus)
block_type: str = field(validator=validators.instance_of(str))
display_name: str = field(validator=validators.instance_of(str))
tags: dict = field(validator=validators.optional(validators.instance_of(dict)))


@frozen
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Generated by Django 4.2.10 on 2024-03-21 23:42

from django.db import migrations
import jsonfield.fields


class Migration(migrations.Migration):

dependencies = [
('content_staging', '0003_olx_unicode'),
]

operations = [
migrations.AddField(
model_name='stagedcontent',
name='tags',
field=jsonfield.fields.JSONField(help_text='Content tags applied to these blocks', null=True),
),
]
4 changes: 4 additions & 0 deletions openedx/core/djangoapps/content_staging/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.core.exceptions import ValidationError
from django.db import models
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from opaque_keys.edx.django.models import UsageKeyField
from opaque_keys.edx.keys import LearningContextKey
from openedx_learning.lib.fields import case_insensitive_char_field, MultiCollationTextField
Expand Down Expand Up @@ -64,6 +65,9 @@ class Meta:
# a new url_name instead.
suggested_url_name = models.CharField(max_length=1024)

# Tags applied to the original source block(s) will be copied to the new block(s) on paste.
tags = JSONField(null=True, help_text=_("""Content tags applied to these blocks"""))

@property
def olx_filename(self) -> str:
""" Get a filename that can be used for the OLX content of this staged content """
Expand Down
1 change: 1 addition & 0 deletions openedx/core/djangoapps/content_staging/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ def post(self, request):
olx=block_data.olx_str,
display_name=block_metadata_utils.display_name_with_default(block),
suggested_url_name=usage_key.block_id,
tags=block_data.tags,
)
(clipboard, _created) = UserClipboard.objects.update_or_create(user=request.user, defaults={
"content": staged_content,
Expand Down
Loading

0 comments on commit 7702971

Please sign in to comment.