diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index 077b8e6a94c5..e04d224bc4d3 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/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. @@ -837,8 +837,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") @@ -852,11 +854,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 +868,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 d75620714fbd..d36f45ad1b92 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), - }, )