diff --git a/openedx/core/djangoapps/content_libraries/api/collections.py b/openedx/core/djangoapps/content_libraries/api/collections.py index 9d011bdae363..962f2c840c42 100644 --- a/openedx/core/djangoapps/content_libraries/api/collections.py +++ b/openedx/core/djangoapps/content_libraries/api/collections.py @@ -176,6 +176,26 @@ def update_library_collection_items( created_by=created_by, ) + # Explicitly emit LIBRARY_COLLECTION_UPDATED with background=True after the + # M2M relationship has been committed. The post_save signal on Collection + # fires before the entities M2M commit, so its synchronous reindex would + # see a stale num_children. The async (background) reindex runs after the + # request transaction commits and computes the correct count. This mirrors + # the pattern already used by set_library_item_collections, + # delete_library_block, restore_library_block, delete_container, and + # restore_container. See bug #35776. + # .. event_implemented_name: LIBRARY_COLLECTION_UPDATED + # .. event_type: org.openedx.content_authoring.content_library.collection.updated.v1 + LIBRARY_COLLECTION_UPDATED.send_event( + library_collection=LibraryCollectionData( + collection_key=library_collection_locator( + library_key=library_key, + collection_key=collection.key, + ), + background=True, + ) + ) + return collection diff --git a/openedx/core/djangoapps/content_libraries/rest_api/collections.py b/openedx/core/djangoapps/content_libraries/rest_api/collections.py index 3f67f5e777a8..82218e74d915 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/collections.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/collections.py @@ -5,6 +5,7 @@ from django.db import transaction from django.db.models import QuerySet +from django.utils.decorators import method_decorator from django.utils.text import slugify from opaque_keys.edx.locator import LibraryLocatorV2 from openedx_authz.constants import permissions as authz_permissions @@ -27,6 +28,10 @@ from .utils import convert_exceptions +# LibraryCollectionsView must run outside the per-request atomic transaction so +# that LIBRARY_COLLECTION_UPDATED tasks enqueued from update_library_collection_items +# land after the M2M commit. See bug #35776. +@method_decorator(transaction.non_atomic_requests, name="dispatch") class LibraryCollectionsView(ModelViewSet): """ Views to get, create and update Library Collections. diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index a6528c089bd0..6880361f6c99 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -250,7 +250,11 @@ def test_update_library_collection_components_event(self) -> None: ], ) - assert event_receiver.call_count == 4 + # 3 CONTENT_OBJECT_ASSOCIATIONS_CHANGED (per entity) + 1 implicit + # LIBRARY_COLLECTION_UPDATED from Collection.post_save + + # 1 explicit LIBRARY_COLLECTION_UPDATED with background=True emitted + # after the M2M commit (bug #35776). + assert event_receiver.call_count == 5 self.assertDictContainsEntries( event_receiver.call_args_list[0].kwargs, { @@ -297,6 +301,65 @@ def test_update_library_collection_components_event(self) -> None: ), }, ) + # Regression for bug #35776: explicit background=True emit after M2M commit. + final_call = event_receiver.call_args_list[4].kwargs + assert final_call["signal"] == LIBRARY_COLLECTION_UPDATED + assert final_call["library_collection"] == LibraryCollectionData( + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), + background=True, + ) + + def test_bug_35776_regression_update_items_emits_background_updated(self) -> None: + """ + Regression for bug #35776: update_library_collection_items must emit + LIBRARY_COLLECTION_UPDATED with background=True after the M2M commit + so the async reindex sees the final count. Without this, the search + document num_children reflected the stale pre-M2M state. + """ + collection_update_event_receiver = mock.Mock() + LIBRARY_COLLECTION_UPDATED.connect(collection_update_event_receiver) + + api.update_library_collection_items( + self.lib1.library_key, + self.col1.key, + opaque_keys=[ + LibraryUsageLocatorV2.from_string(self.lib1_problem_block["id"]), + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), + ], + ) + background_emits = [ + call.kwargs for call in collection_update_event_receiver.call_args_list + if call.kwargs.get("library_collection") + and call.kwargs["library_collection"].background is True + ] + assert len(background_emits) >= 1 + assert background_emits[-1]["library_collection"] == LibraryCollectionData( + collection_key=api.library_collection_locator( + self.lib1.library_key, + collection_key="COL1", + ), + background=True, + ) + + # Also fires for removals + collection_update_event_receiver.reset_mock() + api.update_library_collection_items( + self.lib1.library_key, + self.col1.key, + opaque_keys=[ + LibraryUsageLocatorV2.from_string(self.lib1_html_block["id"]), + ], + remove=True, + ) + background_emits = [ + call.kwargs for call in collection_update_event_receiver.call_args_list + if call.kwargs.get("library_collection") + and call.kwargs["library_collection"].background is True + ] + assert len(background_emits) >= 1 def test_update_collection_components_from_wrong_library(self) -> None: with self.assertRaises(api.ContentLibraryBlockNotFound) as exc: # noqa: PT027