refactor: inheritable authoring mixin callbacks for editing & duplication

cms/djangoapps/contentstore/utils.py

@@ -6,8 +6,7 @@
 import logging
 from collections import defaultdict
 from contextlib import contextmanager
-from datetime import datetime, timezone
-from uuid import uuid4
+from datetime import datetime
 
 from django.conf import settings
 from django.core.exceptions import ValidationError
@@ -18,8 +17,6 @@
 from lti_consumer.models import CourseAllowPIISharingInLTIFlag
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import LibraryLocator
-from openedx_events.content_authoring.data import DuplicatedXBlockData
-from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
 from milestones import api as milestones_api
 from pytz import UTC
 from xblock.fields import Scope
@@ -1027,77 +1024,22 @@ def duplicate_block(
     Duplicate an existing xblock as a child of the supplied parent_usage_key. You can
     optionally specify what usage key the new duplicate block will use via dest_usage_key.
 
-    If shallow is True, does not copy children. Otherwise, this function calls itself
-    recursively, and will set the is_child flag to True when dealing with recursed child
-    blocks.
+    If shallow is True, does not copy children.
     """
     store = modulestore()
     with store.bulk_operations(duplicate_source_usage_key.course_key):
         source_item = store.get_item(duplicate_source_usage_key)
-        if not dest_usage_key:
-            # Change the blockID to be unique.
-            dest_usage_key = source_item.location.replace(name=uuid4().hex)
-
-        category = dest_usage_key.block_type
-
-        duplicate_metadata, asides_to_create = gather_block_attributes(
-            source_item, display_name=display_name, is_child=is_child,
+        return source_item.studio_duplicate(
+            parent_usage_key=parent_usage_key,
+            duplicate_source_usage_key=duplicate_source_usage_key,
+            user=user,
+            store=store,
+            dest_usage_key=dest_usage_key,
+            display_name=display_name,
+            shallow=shallow,
+            is_child=is_child,
         )
 
-        dest_block = store.create_item(
-            user.id,
-            dest_usage_key.course_key,
-            dest_usage_key.block_type,
-            block_id=dest_usage_key.block_id,
-            definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content),
-            metadata=duplicate_metadata,
-            runtime=source_item.runtime,
-            asides=asides_to_create
-        )
-
-        children_handled = False
-
-        if hasattr(dest_block, 'studio_post_duplicate'):
-            # Allow an XBlock to do anything fancy it may need to when duplicated from another block.
-            # 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.
-            load_services_for_studio(dest_block.runtime, user)
-            children_handled = dest_block.studio_post_duplicate(store, source_item)
-
-        # Children are not automatically copied over (and not all xblocks have a 'children' attribute).
-        # Because DAGs are not fully supported, we need to actually duplicate each child as well.
-        if source_item.has_children and not shallow and not children_handled:
-            dest_block.children = dest_block.children or []
-            for child in source_item.children:
-                dupe = duplicate_block(dest_block.location, child, user=user, is_child=True)
-                if dupe not in dest_block.children:  # _duplicate_block may add the child for us.
-                    dest_block.children.append(dupe)
-            store.update_item(dest_block, user.id)
-
-        # pylint: disable=protected-access
-        if 'detached' not in source_item.runtime.load_block_type(category)._class_tags:
-            parent = store.get_item(parent_usage_key)
-            # If source was already a child of the parent, add duplicate immediately afterward.
-            # Otherwise, add child to end.
-            if source_item.location in parent.children:
-                source_index = parent.children.index(source_item.location)
-                parent.children.insert(source_index + 1, dest_block.location)
-            else:
-                parent.children.append(dest_block.location)
-            store.update_item(parent, user.id)
-
-        # .. event_implemented_name: XBLOCK_DUPLICATED
-        XBLOCK_DUPLICATED.send_event(
-            time=datetime.now(timezone.utc),
-            xblock_info=DuplicatedXBlockData(
-                usage_key=dest_block.location,
-                block_type=dest_block.location.block_type,
-                source_usage_key=duplicate_source_usage_key,
-            )
-        )
-
-        return dest_block.location
-
 
 def update_from_source(*, source_block, destination_block, user_id):
     """

cms/djangoapps/contentstore/views/block.py

@@ -12,6 +12,7 @@
 from opaque_keys.edx.keys import CourseKey
 from web_fragments.fragment import Fragment
 
+from cms.djangoapps.contentstore.utils import load_services_for_studio
 from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW
 from common.djangoapps.edxmako.shortcuts import render_to_string
 from common.djangoapps.student.auth import (
@@ -27,7 +28,7 @@
 )
 from xmodule.modulestore.django import (
     modulestore,
-)  # lint-amnesty, pylint: disable=wrong-import-order
+)
 
 
 from xmodule.x_module import (
@@ -46,7 +47,6 @@
 from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
     handle_xblock,
     create_xblock_info,
-    load_services_for_studio,
     get_block_info,
     get_xblock,
     delete_orphans,

cms/djangoapps/contentstore/views/component.py

@@ -34,14 +34,13 @@
 from openedx.core.djangoapps.content_staging import api as content_staging_api
 from openedx.core.djangoapps.content_tagging.api import get_content_tags
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
-from xmodule.modulestore.exceptions import ItemNotFoundError  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.exceptions import ItemNotFoundError
 from ..toggles import use_new_unit_page
-from ..utils import get_lms_link_for_item, get_sibling_urls, reverse_course_url, get_unit_url
+from ..utils import get_lms_link_for_item, get_sibling_urls, load_services_for_studio, reverse_course_url, get_unit_url
 from ..helpers import get_parent_xblock, is_unit, xblock_type_display_name
 from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import (
     add_container_page_publishing_info,
     create_xblock_info,
-    load_services_for_studio,
 )
 
 __all__ = [

cms/djangoapps/contentstore/views/preview.py

@@ -45,7 +45,7 @@
     wrap_xblock_aside
 )
 
-from ..utils import get_visibility_partition_info, StudioPermissionsService
+from ..utils import StudioPermissionsService, get_visibility_partition_info
 from .access import get_user_role
 from .session_kv_store import SessionKeyValueStore
 

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -302,13 +302,10 @@ def _update_with_callback(xblock, user, old_metadata=None, old_content=None):
         old_metadata = own_metadata(xblock)
     if old_content is None:
         old_content = xblock.get_explicitly_set_fields_by_scope(Scope.content)
-    if hasattr(xblock, "editor_saved"):
-        load_services_for_studio(xblock.runtime, user)
-        xblock.editor_saved(user, old_metadata, old_content)
+    load_services_for_studio(xblock.runtime, user)
+    xblock.editor_saved(user, old_metadata, old_content)
     xblock_updated = modulestore().update_item(xblock, user.id)
-    if hasattr(xblock_updated, "post_editor_saved"):
-        load_services_for_studio(xblock_updated.runtime, user)
-        xblock_updated.post_editor_saved(user, old_metadata, old_content)
+    xblock_updated.post_editor_saved(user, old_metadata, old_content)
     return xblock_updated
 
 

cms/lib/xblock/authoring_mixin.py

@@ -4,11 +4,16 @@
 
 
 import logging
+from datetime import datetime, timezone
+from uuid import uuid4
 
 from django.conf import settings
 from web_fragments.fragment import Fragment
 from xblock.core import XBlock, XBlockMixin
 from xblock.fields import String, Scope
+from openedx_events.content_authoring.data import DuplicatedXBlockData
+from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
+
 
 log = logging.getLogger(__name__)
 
@@ -51,3 +56,111 @@ def visibility_view(self, _context=None):
         scope=Scope.settings,
         enforce_type=True,
     )
+
+    def editor_saved(self, user, old_metadata, old_content) -> None:  # pylint: disable=unused-argument
+        """
+        Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving.
+
+        By default, is a no-op. Can be overriden in subclasses.
+        """
+
+    def post_editor_saved(self, user, old_metadata, old_content) -> None:  # pylint: disable=unused-argument
+        """
+        Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks.
+
+        By default, is a no-op. Can be overriden in subclasses.
+        """
+
+    def studio_duplicate(
+        self,
+        parent_usage_key,
+        duplicate_source_usage_key,
+        user,
+        store,
+        dest_usage_key=None,
+        display_name=None,
+        shallow=False,
+        is_child=False,
+    ):
+        """
+        Duplicate an existing xblock as a child of the supplied parent_usage_key. You can
+        optionally specify what usage key the new duplicate block will use via dest_usage_key.
+
+        If shallow is True, does not copy children.
+        """
+        from cms.djangoapps.contentstore.utils import gather_block_attributes, load_services_for_studio
+
+        if not dest_usage_key:
+            # Change the blockID to be unique.
+            dest_usage_key = self.location.replace(name=uuid4().hex)
+
+        category = dest_usage_key.block_type
+
+        duplicate_metadata, asides_to_create = gather_block_attributes(
+            self,
+            display_name=display_name,
+            is_child=is_child,
+        )
+
+        dest_block = store.create_item(
+            user.id,
+            dest_usage_key.course_key,
+            dest_usage_key.block_type,
+            block_id=dest_usage_key.block_id,
+            definition_data=self.get_explicitly_set_fields_by_scope(Scope.content),
+            metadata=duplicate_metadata,
+            runtime=self.runtime,
+            asides=asides_to_create,
+        )
+
+        # Allow an XBlock to do anything fancy it may need to when duplicated from another block.
+        load_services_for_studio(self.runtime, user)
+        dest_block.studio_post_duplicate(self, store, user, shallow=shallow)
+        # pylint: disable=protected-access
+        if "detached" not in self.runtime.load_block_type(category)._class_tags:
+            parent = store.get_item(parent_usage_key)
+            # If source was already a child of the parent, add duplicate immediately afterward.
+            # Otherwise, add child to end.
+            if self.location in parent.children:
+                source_index = parent.children.index(self.location)
+                parent.children.insert(source_index + 1, dest_block.location)
+            else:
+                parent.children.append(dest_block.location)
+            store.update_item(parent, user.id)
+
+        # .. event_implemented_name: XBLOCK_DUPLICATED
+        XBLOCK_DUPLICATED.send_event(
+            time=datetime.now(timezone.utc),
+            xblock_info=DuplicatedXBlockData(
+                usage_key=dest_block.location,
+                block_type=dest_block.location.block_type,
+                source_usage_key=duplicate_source_usage_key,
+            ),
+        )
+
+        return dest_block.location
+
+    def studio_post_duplicate(
+        self,
+        source_block,
+        store,
+        user,
+        shallow: bool,
+    ) -> None:  # pylint: disable=unused-argument
+        """
+        Called when after a block is duplicated. Can be used, e.g., for special handling of child duplication.
+
+        Children must always be handled. In case of inheritance it can be done by running this method with super().
+
+        By default, implements standard duplication logic.
+        """
+        if not source_block.has_children or shallow:
+            return
+
+        self.children = self.children or []
+        for child in source_block.children:
+            child_block = store.get_item(child)
+            dupe = child_block.studio_duplicate(self.location, child, user=user, store=store, is_child=True)
+            if dupe not in self.children:  # studio_duplicate may add the child for us.
+                self.children.append(dupe)
+        store.update_item(self, user.id)

xmodule/library_content_block.py

@@ -587,7 +587,12 @@ def children_are_syncing(self, request, suffix=''):  # pylint: disable=unused-ar
             is_updating = False
         return Response(json.dumps(is_updating))
 
-    def studio_post_duplicate(self, store, source_block):
+    def studio_post_duplicate(
+        self,
+        source_block,
+        *_args,
+        **__kwargs,
+    ) -> None:
         """
         Used by the studio after basic duplication of a source block. We handle the children
         ourselves, because we have to properly reference the library upstream and set the overrides.
@@ -596,7 +601,6 @@ def studio_post_duplicate(self, 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 _validate_library_version(self, validation, lib_tools, version, library_key):
         """

xmodule/studio_editable.py

@@ -2,6 +2,7 @@
 Mixin to support editing in Studio.
 """
 from xblock.core import XBlock, XBlockMixin
+
 from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW
 
 
@@ -12,6 +13,7 @@ class StudioEditableBlock(XBlockMixin):
 
     This class is only intended to be used with an XBlock!
     """
+
     has_author_view = True
 
     def render_children(self, context, fragment, can_reorder=False, can_add=False):
@@ -49,36 +51,6 @@ 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.
-    # 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.
-    #
-    # def editor_saved(self, old_metadata, old_content) -> None:  # pylint: disable=unused-argument
-    #     """
-    #     Called right *before* the block is written to the DB. Can be used, e.g., to modify fields before saving.
-    #
-    #     By default, is a no-op. Can be overriden in subclasses.
-    #     """
-    #
-    # def post_editor_saved(self, old_metadata, old_content) -> None:  # pylint: disable=unused-argument
-    #     """
-    #     Called right *after* the block is written to the DB. Can be used, e.g., to spin up followup tasks.
-    #
-    #     By default, is a no-op. Can be overriden in subclasses.
-    #     """
-    #
-    # def studio_post_duplicate(self, dest_block) -> bool:  # pylint: disable=unused-argument
-    #     """
-    #     Called when a the block is duplicated. 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):
     """