Don't use OLX when copying/duplicating blocks [FC-0049]

cms/djangoapps/contentstore/helpers.py

@@ -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
@@ -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
 
@@ -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(
@@ -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
@@ -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
 
@@ -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

cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py

@@ -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")
@@ -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

cms/envs/common.py

@@ -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,
@@ -986,7 +985,6 @@
     XModuleMixin,
     EditInfoMixin,
     AuthoringMixin,
-    TaggedBlockMixin,
 )
 XBLOCK_EXTRA_MIXINS = ()
 

openedx/core/djangoapps/content_staging/admin.py

@@ -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, )
 
 

openedx/core/djangoapps/content_staging/api.py

@@ -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,
     )

openedx/core/djangoapps/content_staging/data.py

@@ -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

openedx/core/djangoapps/content_staging/migrations/0004_stagedcontent_tags.py

@@ -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),
+        ),
+    ]

openedx/core/djangoapps/content_staging/models.py

@@ -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
@@ -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 """