refactor: TaggedBlockMixin (#642)

* refactor: TaggedBlockMixin overrides add_xml_to_node

so we can use inheritance to include tags in exported blocks

* refactor: store tags_v1 in an internal field

instead of an XBlock field, because we don't want this information
persisted to the modulestore.

* refactor: adds studio_post_paste handler

for use after copy/pasting content from the clipboard.

* fix: make serialize_tag_data a classmethod

so we can serialize tags for blocks that aren't officially TaggedXBlocks

cms/djangoapps/contentstore/helpers.py

@@ -17,7 +17,6 @@
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
 from xmodule.exceptions import NotFoundError
-from xmodule.library_content_block import LibraryContentBlock
 from xmodule.modulestore.django import modulestore
 from xmodule.xml_block import XmlMixin
 
@@ -337,20 +336,18 @@ def _import_xml_node_to_parent(
     new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True)
     parent_xblock.children.append(new_xblock.location)
     store.update_item(parent_xblock, user_id)
-    if isinstance(new_xblock, LibraryContentBlock):
-        # Special case handling for library content. If we need this for other blocks in the future, it can be made into
-        # an API, and we'd call new_block.studio_post_paste() instead of this code.
-        # In this case, we want to pull the children from the library and let library_tools assign their IDs.
-        new_xblock.sync_from_library(upgrade_to_latest=False)
-    else:
+
+    children_handled = False
+    if hasattr(new_xblock, 'studio_post_paste'):
+        # Allow an XBlock to do anything fancy it may need to when pasted from the clipboard.
+        # These blocks may handle their own children or parenting if needed. Let them return booleans to
+        # let us know if we need to handle these or not.
+        children_handed = new_xblock.studio_post_paste(store, node)
+
+    if not children_handled:
         for child_node in child_nodes:
             _import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id)
 
-    from cms.lib.xblock.tagging.tagged_block_mixin import TaggedBlockMixin
-    if isinstance(new_xblock, TaggedBlockMixin):
-        new_xblock.read_tags_from_node(node)
-        new_xblock.add_tags_from_field()
-
     return new_xblock
 
 

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

@@ -1138,11 +1138,11 @@ def test_duplicate_tags(self):
         # Refresh.
         source_chapter = self.store.get_item(source_chapter.location)
         expected_chapter_tags = 'A:one,two;B:four,three'
-        assert source_chapter.serialize_tag_data() == expected_chapter_tags
+        assert source_chapter.serialize_tag_data(source_chapter.location) == expected_chapter_tags
 
         source_block = self.store.get_item(source_block.location)
         expected_block_tags = 'A:two'
-        assert source_block.serialize_tag_data() == expected_block_tags
+        assert source_block.serialize_tag_data(source_block.location) == expected_block_tags
 
         # Duplicate the chapter (and its children)
         dupe_location = duplicate_block(
@@ -1155,8 +1155,8 @@ def test_duplicate_tags(self):
         dupe_block = dupe_chapter.get_children()[0]
 
         # Check that the duplicated blocks also duplicated tags
-        assert dupe_chapter.serialize_tag_data() == expected_chapter_tags
-        assert dupe_block.serialize_tag_data() == expected_block_tags
+        assert dupe_chapter.serialize_tag_data(dupe_chapter.location) == expected_chapter_tags
+        assert dupe_block.serialize_tag_data(dupe_block.location) == expected_block_tags
 
 
 @ddt.ddt

cms/lib/xblock/tagging/tagged_block_mixin.py

@@ -3,44 +3,38 @@
 """
 from urllib.parse import quote, unquote
 
-from xblock.fields import Scope, String
-
-
-def _(text):
-    """
-    A noop underscore function that marks strings for extraction.
-    """
-    return text
-
 
 class TaggedBlockMixin:
     """
     Mixin containing XML serializing and parsing functionality for tagged blocks
     """
 
-    tags_v1 = String(
-        display_name=_("Tags v1"),
-        name="tags-v1",
-        help=_("Serialized content tags"),
-        default="",
-        scope=Scope.settings
-    )
+    def __init__(self, *args, **kwargs):
+        """
+        Initialize the tagged xblock.
+        """
+        # We store tags internally, without an XBlock field, because we don't want tags to be stored to the modulestore.
+        # But we do want them persisted on duplicate, copy, or export/import.
+        self.tags_v1 = ""
 
-    def studio_post_duplicate(self, store, source_item):
+    @property
+    def tags_v1(self) -> str:
         """
-        Duplicates content tags from the source_item.
+        Returns the serialized tags.
         """
-        if hasattr(super(), 'studio_post_duplicate'):
-            super().studio_post_duplicate()
+        return self._tags_v1
 
-        if hasattr(source_item, 'serialize_tag_data'):
-            tags = source_item.serialize_tag_data()
-            self.tags_v1 = tags
-        self.add_tags_from_field()
+    @tags_v1.setter
+    def tags_v1(self, tags: str) -> None:
+        """
+        Returns the serialized tags.
+        """
+        self._tags_v1 = tags
 
-    def serialize_tag_data(self):
+    @classmethod
+    def serialize_tag_data(cls, usage_id):
         """
-        Serialize block's tag data to include in the xml, escaping special characters
+        Serialize a block's tag data to include in the xml, escaping special characters
 
         Example tags:
             LightCast Skills Taxonomy: ["Typing", "Microsoft Office"]
@@ -52,7 +46,7 @@ def serialize_tag_data(self):
         # This import is done here since we import and use TaggedBlockMixin in the cms settings, but the
         # content_tagging app wouldn't have loaded yet, so importing it outside causes an error
         from openedx.core.djangoapps.content_tagging.api import get_object_tags
-        content_tags = get_object_tags(self.scope_ids.usage_id)
+        content_tags = get_object_tags(usage_id)
 
         serialized_tags = []
         taxonomies_and_tags = {}
@@ -76,20 +70,13 @@ def add_tags_to_node(self, node):
         """
         Serialize and add tag data (if any) to node
         """
-        tag_data = self.serialize_tag_data()
+        tag_data = self.serialize_tag_data(self.scope_ids.usage_id)
         if tag_data:
             node.set('tags-v1', tag_data)
 
-    def read_tags_from_node(self, node):
-        """
-        Deserialize and read tag data from node
-        """
-        if 'tags-v1' in node.attrib:
-            self.tags_v1 = str(node.attrib['tags-v1'])
-
     def add_tags_from_field(self):
         """
-        Parse and add tag data from tags_v1 field
+        Parse tags_v1 data and create tags for this block.
         """
         # This import is done here since we import and use TaggedBlockMixin in the cms settings, but the
         # content_tagging app wouldn't have loaded yet, so importing it outside causes an error
@@ -108,3 +95,38 @@ def add_tags_from_field(self):
             taxonomy_and_tags_dict[taxonomy_export_id] = tag_values
 
         set_object_tags(self.usage_key, taxonomy_and_tags_dict)
+
+    def add_xml_to_node(self, node):
+        """
+        Include the serialized tag data in XML when adding to node
+        """
+        super().add_xml_to_node(node)
+        self.add_tags_to_node(node)
+
+    def studio_post_duplicate(self, store, source_item) -> bool:
+        """
+        Duplicates content tags from the source_item.
+
+        Returns False to indicate the children have not been handled.
+        """
+        if hasattr(super(), 'studio_post_duplicate'):
+            super().studio_post_duplicate()
+
+        self.tags_v1 = self.serialize_tag_data(source_item.scope_ids.usage_id)
+        self.add_tags_from_field()
+        return False
+
+    def studio_post_paste(self, store, source_node) -> bool:
+        """
+        Copies content tags from the source_node.
+
+        Returns False to indicate the children have not been handled.
+        """
+        if hasattr(super(), 'studio_post_paste'):
+            super().studio_post_paste()
+
+        if 'tags-v1' in source_node.attrib:
+            self.tags_v1 = str(source_node.attrib['tags-v1'])
+
+        self.add_tags_from_field()
+        return False

xmodule/library_content_block.py

@@ -593,10 +593,23 @@ def studio_post_duplicate(self, store, source_block):
 
         Otherwise we'll end up losing data on the next refresh.
         """
+        if hasattr(super(), 'studio_post_duplicate'):
+            super().studio_post_duplicate(store, source_block)
+
         self._validate_sync_permissions()
         self.get_tools(to_read_library_content=True).trigger_duplication(source_block=source_block, dest_block=self)
         return True  # Children have been handled.
 
+    def studio_post_paste(self, store, source_node) -> bool:
+        """
+        Pull the children from the library and let library_tools assign their IDs.
+        """
+        if hasattr(super(), 'studio_post_paste'):
+            super().studio_post_paste(store, source_node)
+
+        self.sync_from_library(upgrade_to_latest=False)
+        return True  # Children have been handled
+
     def _validate_library_version(self, validation, lib_tools, version, library_key):
         """
         Validates library version

xmodule/studio_editable.py

@@ -49,7 +49,7 @@ def get_preview_view_name(block):
         """
         return AUTHOR_VIEW if has_author_view(block) else STUDENT_VIEW
 
-    # Some parts of the code use getattr to dynamically check for the following three methods on subclasses.
+    # Some parts of the code use getattr to dynamically check for the following methods on subclasses.
     # We'd like to refactor so that we can actually declare them here as overridable methods.
     # For now, we leave them here as documentation.
     # See https://github.com/openedx/edx-platform/issues/33715.
@@ -68,7 +68,7 @@ def get_preview_view_name(block):
     #     By default, is a no-op. Can be overriden in subclasses.
     #     """
     #
-    # def studio_post_duplicate(self, dest_block) -> bool:  # pylint: disable=unused-argument
+    # def studio_post_duplicate(self, store, source_block) -> bool:  # pylint: disable=unused-argument
     #     """
     #     Called when a the block is duplicated. Can be used, e.g., for special handling of child duplication.
     #
@@ -78,6 +78,17 @@ def get_preview_view_name(block):
     #     By default, is a no-op. Can be overriden in subclasses.
     #     """
     #     return False
+    #
+    # def studio_post_paste(self, store, source_node) -> bool:  # pylint: disable=unused-argument
+    #     """
+    #     Called after a block is copy-pasted. Can be used, e.g., for special handling of child duplication.
+    #
+    #     Returns 'True' if children have been handled and thus shouldn't be handled by the standard
+    #     duplication logic.
+    #
+    #     By default, is a no-op. Can be overriden in subclasses.
+    #     """
+    #     return False
 
 
 def has_author_view(block):

xmodule/xml_block.py

@@ -428,8 +428,6 @@ def add_xml_to_node(self, node):
         """
         For exporting, set data on `node` from ourselves.
         """
-        # Importing here to avoid circular import
-        from cms.lib.xblock.tagging.tagged_block_mixin import TaggedBlockMixin
         # Get the definition
         xml_object = self.definition_to_xml(self.runtime.export_fs)
 
@@ -500,10 +498,6 @@ def add_xml_to_node(self, node):
             node.set('org', self.location.org)
             node.set('course', self.location.course)
 
-        # Serialize and add tag data if any
-        if isinstance(self, TaggedBlockMixin):
-            self.add_tags_to_node(node)
-
     def definition_to_xml(self, resource_fs):
         """
         Return a new etree Element object created from this modules definition.