From ebce9e589427fa2d5d96008b0b4071f4645f3278 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 22 Jun 2026 14:33:06 -0700 Subject: [PATCH] fix: use a bulk context when pasting containers into a content library (#38790) Now, when pasting a container into a library: * Only a single draft change log is created, regardless of how many "things" are in the container, and * The change is properly attributed to the current user who pressed "paste" --- .../content_libraries/api/blocks.py | 42 ++++++++++--------- .../content_libraries/tests/test_api.py | 28 +++++++++++++ .../tests/test_content_libraries.py | 2 +- .../content_libraries/tests/test_events.py | 4 -- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 39fac27ca12c..6cbe96af8b33 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -577,7 +577,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, @@ -683,7 +683,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) @@ -699,7 +699,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, @@ -815,7 +815,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. @@ -823,8 +823,10 @@ def import_staged_content_from_user_clipboard(library_key: LibraryLocatorV2, use Returns the newly created item metadata """ from openedx.core.djangoapps.content_staging import api as content_staging_api + if user is None or user.id is None: + raise RuntimeError("A user is required.") # Shouldn't happen - mostly here for type checker - user_clipboard = content_staging_api.get_user_clipboard(user) + user_clipboard = content_staging_api.get_user_clipboard(user.id) if not user_clipboard: raise ValidationError("The user's clipboard is empty") @@ -838,11 +840,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, @@ -852,17 +854,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: """ diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 1060ec42713e..d2c333fce120 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/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 diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 6d947aa786d4..3ffc5117bd95 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -830,7 +830,7 @@ def test_library_paste_xblock(self): # the the block in the clipboard self.assertDictContainsEntries(self._get_library_block(paste_data["id"]), { **block_data, - "last_draft_created_by": None, + "last_draft_created_by": "Author", "last_draft_created": paste_data["last_draft_created"], "created": paste_data["created"], "modified": paste_data["modified"], diff --git a/openedx/core/djangoapps/content_libraries/tests/test_events.py b/openedx/core/djangoapps/content_libraries/tests/test_events.py index c89905d7dd37..3603124eab8e 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_events.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_events.py @@ -1425,8 +1425,4 @@ def test_signals_emitted_on_import_container_success(self) -> None: object_id=str(self.html_block_usage_key), changes=["units"], ), }, - { - "signal": LIBRARY_CONTAINER_UPDATED, - "library_container": LibraryContainerData(container_key=new_container_key), - }, )