From 1746c103938ef0438ba31c6b7da286d4838ab012 Mon Sep 17 00:00:00 2001 From: kingoftech-v01 Date: Fri, 10 Apr 2026 05:09:56 +0000 Subject: [PATCH] fix(content_libraries): keep collection num_children in sync after item updates update_library_collection_items relied on Collection.post_save to emit LIBRARY_COLLECTION_UPDATED, but post_save fires before the M2M entities commit, so the synchronous reindex in searchable_doc_for_collection computed num_children from the stale membership. Emit the signal explicitly with background=True after the add/remove so the async reindex runs post-commit with the correct count, matching the pattern already used by set_library_item_collections, delete_library_block, restore_library_block, delete_container, and restore_container. Also mark LibraryCollectionsView with non_atomic_requests so Celery tasks dispatched from the view are enqueued after the request transaction commits, matching every other view in openedx/core/djangoapps/content_libraries/rest_api/. Closes #35776 --- .../content_libraries/api/collections.py | 20 ++++++ .../content_libraries/rest_api/collections.py | 5 ++ .../content_libraries/tests/test_api.py | 65 ++++++++++++++++++- 3 files changed, 89 insertions(+), 1 deletion(-) 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