fix: use a bulk context when pasting containers into a content library

Author: bradenmacdonald

openedx/core/djangoapps/content_libraries/api/blocks.py

@@ -591,7 +591,7 @@ def _import_staged_block(
     olx_str: str,
     library_key: LibraryLocatorV2,
     source_context_key: LearningContextKey,
-    user,
+    user: UserType,
     staged_content_id: StagedContentID,
     staged_content_files: list[StagedContentFileData],
     now: datetime,
@@ -697,7 +697,7 @@ def _import_staged_block(
         # This will create the first component version and set the OLX/title
         # appropriately. It will not publish. Once we get the newly created
         # ComponentVersion back from this, we can attach all our files to it.
-        set_library_block_olx(usage_key, olx_str, paths_to_media)
+        set_library_block_olx(usage_key, olx_str, paths_to_media, created_by=user.id)
 
     # Now return the metadata about the new block
     return get_library_block(usage_key)
@@ -713,7 +713,7 @@ def _is_container(block_type: str) -> bool:
 def _import_staged_block_as_container(
     library_key: LibraryLocatorV2,
     source_context_key: LearningContextKey,
-    user,
+    user: UserType,
     staged_content_id: StagedContentID,
     staged_content_files: list[StagedContentFileData],
     now: datetime,
@@ -829,7 +829,7 @@ def _import_staged_block_as_container(
     return container
 
 
-def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, user) -> PublishableItem:
+def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, user: UserType) -> PublishableItem:
     """
     Create a new library item from the staged content from clipboard.
     Can create containers (e.g. units) or XBlocks.
@@ -852,11 +852,11 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use
         raise RuntimeError("olx_str missing")  # Shouldn't happen - mostly here for type checker
 
     now = datetime.now(tz=timezone.utc)  # noqa: UP017
+    lp_id = ContentLibrary.objects.get_by_key(library_key).learning_package_id
 
-    if _is_container(user_clipboard.content.block_type):
-        # This is a container and we can import it as such.
-        # Start an atomic section so the whole paste succeeds or fails together:
-        with transaction.atomic():
+    # Start an atomic section so the whole paste succeeds or fails together and creates a single change log entry:
+    with transaction.atomic(), content_api.bulk_draft_changes_for(lp_id, changed_by=user.id, changed_at=now):
+        if _is_container(user_clipboard.content.block_type):
             return _import_staged_block_as_container(
                 library_key,
                 source_context_key,
@@ -866,17 +866,17 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use
                 now,
                 olx_str=olx_str,
             )
-    else:
-        return _import_staged_block(
-            user_clipboard.content.block_type,
-            olx_str,
-            library_key,
-            source_context_key,
-            user,
-            staged_content_id,
-            staged_content_files,
-            now,
-        )
+        else:
+            return _import_staged_block(
+                user_clipboard.content.block_type,
+                olx_str,
+                library_key,
+                source_context_key,
+                user,
+                staged_content_id,
+                staged_content_files,
+                now,
+            )
 
 def get_or_create_olx_media_type(block_type: str) -> MediaType:
     """

openedx/core/djangoapps/content_libraries/tests/test_api.py

@@ -749,6 +749,11 @@ def test_get_containers_contains_item(self):
 
 
     def test_copy_and_paste_container_same_library(self) -> None:
+        """
+        Test copying and then pasting a container (within the same library)
+        """
+        # Publish the library so we can track what happens from this point forward more easily
+        api.publish_changes(self.lib1.library_key, self.user.id)
         # Copy a section with children
         api.copy_container(self.section1.container_key, self.user.id)
         # Paste the container
@@ -768,7 +773,17 @@ def test_copy_and_paste_container_same_library(self) -> None:
         assert isinstance(subsections[1], api.ContainerMetadata)
         assert subsections[1].container_key == self.subsection2.container_key
 
+        # Verify that everything was pasted in a single draft change:
+        container_history = content_api.get_entity_draft_history(new_container.container_id)
+        assert len(container_history) == 1
+        # The subsections are re-used on paste into the same library, so they aren't modified at all since the publish:
+        child_history = content_api.get_entity_draft_history(subsections[0].container_id)
+        assert len(child_history) == 0
+
     def test_copy_and_paste_container_another_library(self) -> None:
+        """
+        Test copying and then pasting a container (into a different library)
+        """
         # Copy a section with children
         api.copy_container(self.section1.container_key, self.user.id)
 
@@ -824,6 +839,19 @@ def test_copy_and_paste_container_another_library(self) -> None:
         # This is the same unit, so it should not be duplicated
         assert units_subsection1[0].container_key == units_subsection2[0].container_key
 
+        # Verify that everything was pasted in a single draft change:
+        container_history = content_api.get_entity_draft_history(new_container.container_id)
+        assert len(container_history) == 1
+        subsection_history = content_api.get_entity_draft_history(subsections[0].container_id)
+        assert len(subsection_history) == 1
+        component_id = content_api.get_entities_in_container(
+            content_api.get_container(subsections[0].container_id), published=False
+        )[0].entity.id
+        component_history = content_api.get_entity_draft_history(component_id)
+        assert len(component_history) == 1
+        assert container_history[0].draft_change_log.id == subsection_history[0].draft_change_log.id
+        assert container_history[0].draft_change_log.id == component_history[0].draft_change_log.id
+
     def test_set_library_block_olx_no_signal_on_rollback(self) -> None:
         """
         LIBRARY_BLOCK_UPDATED is NOT emitted when set_library_block_olx is called