From cd58c920af3695a8d56e06af5112836882da6d1c Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Tue, 14 Oct 2025 17:23:00 -0500 Subject: [PATCH 1/9] fix: Update course library page after sync with celery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handle_update_xblock_upstream_link` is called asynchronously with celery. In `update_upstream_downstream_link_handler`, the xblock has the updated version, but when calling `handle_update_xblock_upstream_link` inside Celry, the xblock is outdated in a previous version, that is why the error occurs. The immediate fix is ​​to call handle_update_xblock_upstream_link synchronously. --- cms/djangoapps/contentstore/signals/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index ebf47f527f5f..8585668152ea 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -267,7 +267,7 @@ def update_upstream_downstream_link_handler(**kwargs): log.error("Received null or incorrect data for event") return - handle_update_xblock_upstream_link.delay(str(xblock_info.usage_key)) + handle_update_xblock_upstream_link(str(xblock_info.usage_key)) @receiver(XBLOCK_DELETED) From d4c5e79058ff4844c0937d5b9430d43f6de1d688 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Wed, 15 Oct 2025 18:15:24 -0500 Subject: [PATCH 2/9] fix: Add `transaction.on_commit(fn)` in on_commit_changes_to in the modulestore --- cms/djangoapps/contentstore/signals/handlers.py | 2 +- xmodule/modulestore/__init__.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 8585668152ea..ebf47f527f5f 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -267,7 +267,7 @@ def update_upstream_downstream_link_handler(**kwargs): log.error("Received null or incorrect data for event") return - handle_update_xblock_upstream_link(str(xblock_info.usage_key)) + handle_update_xblock_upstream_link.delay(str(xblock_info.usage_key)) @receiver(XBLOCK_DELETED) diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index f3aee2a58f24..51118cc42e67 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -12,6 +12,7 @@ from collections import defaultdict from contextlib import contextmanager from operator import itemgetter +from django.db import transaction from opaque_keys.edx.keys import AssetKey, CourseKey from opaque_keys.edx.locations import Location # For import backwards compatibility @@ -322,6 +323,10 @@ def on_commit_changes_to(self, course_key, fn): """ Call some callback when the currently active bulk operation has saved """ + # If we're in a MySQL transaction, so the new version will only be committed to the + # SplitModulestoreCourseIndex table after the MySQL transaction is closed. + def wrapped_fn(): + transaction.on_commit(fn) # Check if a bulk op is active. If so, defer fn(); otherwise call it immediately. # Note: calling _get_bulk_ops_record() here and then checking .active can have side-effects in some cases # because it creates an entry in the defaultdict if none exists, so we check if the record is active using @@ -329,9 +334,9 @@ def on_commit_changes_to(self, course_key, fn): # so we check it this way: if course_key and course_key.for_branch(None) in self._active_bulk_ops.records: bulk_ops_record = self._active_bulk_ops.records[course_key.for_branch(None)] - bulk_ops_record.defer_until_commit(fn) + bulk_ops_record.defer_until_commit(wrapped_fn) else: - fn() # There is no active bulk operation - call fn() now. + wrapped_fn() # There is no active bulk operation - call wrapped_fn() now. def _is_in_bulk_operation(self, course_key, ignore_case=False): """ From 01d750c285f4e258657a3ba14ec0bef5efd133e0 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 16 Oct 2025 18:52:57 -0500 Subject: [PATCH 3/9] test: Update test to match with new on_commit_changes_to --- .../content/search/tests/test_handlers.py | 2 +- .../content_tagging/tests/test_tasks.py | 28 ++++++++++--------- xmodule/modulestore/__init__.py | 1 + xmodule/tests/__init__.py | 4 +-- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 33d0e4db8378..5022a99dead6 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -4,7 +4,7 @@ from datetime import datetime, timezone from unittest.mock import MagicMock, patch -from django.test import LiveServerTestCase, override_settings +from django.test import LiveServerTestCase, override_settings, TransactionTestCase from freezegun import freeze_time from organizations.tests.factories import OrganizationFactory diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index d0e10ecfb7ae..32203f3929d5 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -199,25 +199,27 @@ def test_update_course(self): def test_create_delete_xblock(self): # Create course - course = self.store.create_course( - self.orgA.short_name, - "test_course", - "test_run", - self.user_id, - fields={"language": "pt-br"}, - ) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"language": "pt-br"}, + ) - # Create XBlocks - sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") - vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") + # Create XBlocks + sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") + vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") - usage_key_str = str(vertical.location) + usage_key_str = str(vertical.location) # Check if the tags are created in the XBlock assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, "Português (Brasil)") - # Delete the XBlock - self.store.delete_item(vertical.location, self.user_id) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + # Delete the XBlock + self.store.delete_item(vertical.location, self.user_id) # Check if the tags are deleted assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None) diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 51118cc42e67..07f0cb513ecb 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -13,6 +13,7 @@ from contextlib import contextmanager from operator import itemgetter from django.db import transaction +from django.conf import settings from opaque_keys.edx.keys import AssetKey, CourseKey from opaque_keys.edx.locations import Location # For import backwards compatibility diff --git a/xmodule/tests/__init__.py b/xmodule/tests/__init__.py index 786836c050b5..d8a203f34ba8 100644 --- a/xmodule/tests/__init__.py +++ b/xmodule/tests/__init__.py @@ -13,7 +13,7 @@ from functools import wraps from unittest.mock import Mock -from django.test import TestCase +from django.test import TransactionTestCase from opaque_keys.edx.keys import CourseKey from path import Path as path @@ -306,7 +306,7 @@ def __getitem__(self, index): return str(self)[index] -class CourseComparisonTest(TestCase): +class CourseComparisonTest(TransactionTestCase): """ Mixin that has methods for comparing courses for equality. """ From 247ec70fdf00db4d88627be662e56cabecce39e3 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 16 Oct 2025 19:41:16 -0500 Subject: [PATCH 4/9] style: fix nits on the code --- openedx/core/djangoapps/content/search/tests/test_handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 5022a99dead6..33d0e4db8378 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -4,7 +4,7 @@ from datetime import datetime, timezone from unittest.mock import MagicMock, patch -from django.test import LiveServerTestCase, override_settings, TransactionTestCase +from django.test import LiveServerTestCase, override_settings from freezegun import freeze_time from organizations.tests.factories import OrganizationFactory From 605cf964f2213d552d6563f83c15ba86a77c85ad Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Thu, 16 Oct 2025 20:48:36 -0500 Subject: [PATCH 5/9] test: Fix broken tests --- .../content/search/tests/test_handlers.py | 139 +++++++++--------- 1 file changed, 73 insertions(+), 66 deletions(-) diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 33d0e4db8378..2157fc9d5284 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -49,83 +49,90 @@ def setUp(self): def test_create_delete_xblock(self, meilisearch_client): # Create course - course = self.store.create_course( - self.orgA.short_name, - "test_course", - "test_run", - self.user_id, - fields={"display_name": "Test Course"}, - ) - course_access, _ = SearchAccess.objects.get_or_create(context_key=course.id) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"display_name": "Test Course"}, + ) + course_access, _ = SearchAccess.objects.get_or_create(context_key=course.id) + + # Create XBlocks + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") + doc_sequential = { + "id": "block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395", + "type": "course_block", + "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + "block_id": "test_sequential", + "display_name": "sequential", + "block_type": "sequential", + "context_key": "course-v1:orgA+test_course+test_run", + "org": "orgA", + "breadcrumbs": [ + { + "display_name": "Test Course", + }, + ], + "content": {}, + "access_id": course_access.id, + "modified": created_date.timestamp(), + } - # Create XBlocks - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): - sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") - doc_sequential = { - "id": "block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395", - "type": "course_block", - "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - "block_id": "test_sequential", - "display_name": "sequential", - "block_type": "sequential", - "context_key": "course-v1:orgA+test_course+test_run", - "org": "orgA", - "breadcrumbs": [ - { - "display_name": "Test Course", - }, - ], - "content": {}, - "access_id": course_access.id, - "modified": created_date.timestamp(), - } meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_sequential]) - with freeze_time(created_date): - vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") - doc_vertical = { - "id": "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b", - "type": "course_block", - "usage_key": "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical", - "block_id": "test_vertical", - "display_name": "vertical", - "block_type": "vertical", - "context_key": "course-v1:orgA+test_course+test_run", - "org": "orgA", - "breadcrumbs": [ - { - "display_name": "Test Course", - }, - { - "display_name": "sequential", - "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - }, - ], - "content": {}, - "access_id": course_access.id, - "modified": created_date.timestamp(), - } + + with self.captureOnCommitCallbacks(execute=True) as callbacks: + with freeze_time(created_date): + vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") + doc_vertical = { + "id": "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b", + "type": "course_block", + "usage_key": "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical", + "block_id": "test_vertical", + "display_name": "vertical", + "block_type": "vertical", + "context_key": "course-v1:orgA+test_course+test_run", + "org": "orgA", + "breadcrumbs": [ + { + "display_name": "Test Course", + }, + { + "display_name": "sequential", + "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + }, + ], + "content": {}, + "access_id": course_access.id, + "modified": created_date.timestamp(), + } meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_vertical]) - # Update the XBlock - sequential = self.store.get_item(sequential.location, self.user_id) # Refresh the XBlock - sequential.display_name = "Updated Sequential" - modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) - with freeze_time(modified_date): - self.store.update_item(sequential, self.user_id) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + # Update the XBlock + sequential = self.store.get_item(sequential.location, self.user_id) # Refresh the XBlock + sequential.display_name = "Updated Sequential" + modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(modified_date): + self.store.update_item(sequential, self.user_id) + + # The display name and the child's breadcrumbs should be updated + doc_sequential["display_name"] = "Updated Sequential" + doc_vertical["breadcrumbs"][1]["display_name"] = "Updated Sequential" + doc_sequential["modified"] = modified_date.timestamp() - # The display name and the child's breadcrumbs should be updated - doc_sequential["display_name"] = "Updated Sequential" - doc_vertical["breadcrumbs"][1]["display_name"] = "Updated Sequential" - doc_sequential["modified"] = modified_date.timestamp() meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ doc_sequential, doc_vertical, ]) - # Delete the XBlock - self.store.delete_item(vertical.location, self.user_id) + with self.captureOnCommitCallbacks(execute=True) as callbacks: + # Delete the XBlock + self.store.delete_item(vertical.location, self.user_id) meilisearch_client.return_value.index.return_value.delete_document.assert_called_with( "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b" From 36c0e3c88ea23c63c4f50262444f917e93485557 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 17 Oct 2025 12:44:47 -0500 Subject: [PATCH 6/9] refactor: Use ImmediateOnCommitMixin on tests that needs call on_commit --- .../tests/test_downstream_sync_integration.py | 4 +- .../v2/views/tests/test_downstreams.py | 6 +- .../tests/test_upstream_downstream_links.py | 4 +- .../views/tests/test_clipboard_paste.py | 4 +- .../course_overviews/tests/test_signals.py | 20 +-- .../content/search/tests/test_handlers.py | 146 +++++++++--------- .../content_tagging/tests/test_tasks.py | 35 +++-- xmodule/modulestore/__init__.py | 1 + xmodule/modulestore/tests/django_utils.py | 29 ++++ 9 files changed, 143 insertions(+), 106 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py index 78730fe6a0dc..d408319d4375 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py @@ -13,12 +13,12 @@ from openedx.core.djangoapps.content_libraries.tests import ContentLibrariesRestApiTest from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import get_block_key_string from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, ImmediateOnCommitMixin from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory @ddt.ddt -class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): +class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ImmediateOnCommitMixin, ModuleStoreTestCase): """ Tests that involve syncing content from libraries to courses. """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index cf3838ac3719..1dcc3ca9031a 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -23,7 +23,7 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as lib_api from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ImmediateOnCommitMixin from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory from .. import downstreams as downstreams_views @@ -406,7 +406,7 @@ def test_400(self, sync: str): assert video_after.upstream is None -class DeleteDownstreamViewTest(SharedErrorTestCases, SharedModuleStoreTestCase): +class DeleteDownstreamViewTest(SharedErrorTestCases, ImmediateOnCommitMixin, SharedModuleStoreTestCase): """ Test that `DELETE /api/v2/contentstore/downstreams/...` severs a downstream's link to an upstream. """ @@ -596,6 +596,7 @@ def test_204(self, mock_decline_sync): @ddt.ddt class GetUpstreamViewTest( _BaseDownstreamViewTestMixin, + ImmediateOnCommitMixin, SharedModuleStoreTestCase, ): """ @@ -1424,6 +1425,7 @@ def test_200_get_ready_to_sync_duplicated_top_level_parents(self): class GetDownstreamSummaryViewTest( _BaseDownstreamViewTestMixin, + ImmediateOnCommitMixin, SharedModuleStoreTestCase, ): """ diff --git a/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py b/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py index 90fec8471651..87ded9107f6e 100644 --- a/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py +++ b/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py @@ -14,7 +14,7 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_cms -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, ImmediateOnCommitMixin from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory from ..models import ContainerLink, LearningContextLinksStatus, LearningContextLinksStatusChoices, ComponentLink @@ -265,7 +265,7 @@ def test_call_for_nonexistent_course(self): @skip_unless_cms -class TestUpstreamLinksEvents(ModuleStoreTestCase, OpenEdxEventsTestMixin, BaseUpstreamLinksHelpers): +class TestUpstreamLinksEvents(ImmediateOnCommitMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin, BaseUpstreamLinksHelpers): """ Test signals related to managing upstream->downstream links. """ diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 3fae9d996fd2..2ca03ccf892b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -16,7 +16,7 @@ from openedx_tagging.core.tagging.models import Tag from organizations.models import Organization from xmodule.modulestore.django import contentstore, modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course, ImmediateOnCommitMixin from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url @@ -400,7 +400,7 @@ def test_paste_with_assets(self): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. -class ClipboardPasteFromV2LibraryTestCase(OpenEdxEventsTestMixin, ModuleStoreTestCase): +class ClipboardPasteFromV2LibraryTestCase(OpenEdxEventsTestMixin, ImmediateOnCommitMixin, ModuleStoreTestCase): """ Test Clipboard Paste functionality with a "new" (as of Sumac) library """ diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py index 49adf5540003..960be6c6eadb 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_signals.py @@ -13,7 +13,11 @@ from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.tests.django_utils import TEST_DATA_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_ONLY_SPLIT_MODULESTORE_DRAFT_PREFERRED, + ModuleStoreTestCase, + ImmediateOnCommitMixin, +) from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from ..models import CourseOverview @@ -23,7 +27,7 @@ @ddt.ddt -class CourseOverviewSignalsTestCase(ModuleStoreTestCase): +class CourseOverviewSignalsTestCase(ImmediateOnCommitMixin, ModuleStoreTestCase): """ Tests for CourseOverview signals. """ @@ -43,16 +47,14 @@ def assert_changed_signal_sent(self, changes, mock_signal): ) # changing display name doesn't fire the signal - with self.captureOnCommitCallbacks(execute=True) as callbacks: - course.display_name = course.display_name + 'changed' - course = self.store.update_item(course, ModuleStoreEnum.UserID.test) + course.display_name = course.display_name + 'changed' + course = self.store.update_item(course, ModuleStoreEnum.UserID.test) assert not mock_signal.called # changing the given field fires the signal - with self.captureOnCommitCallbacks(execute=True) as callbacks: - for change in changes: - setattr(course, change.field_name, change.changed_value) - self.store.update_item(course, ModuleStoreEnum.UserID.test) + for change in changes: + setattr(course, change.field_name, change.changed_value) + self.store.update_item(course, ModuleStoreEnum.UserID.test) assert mock_signal.called def test_caching(self): diff --git a/openedx/core/djangoapps/content/search/tests/test_handlers.py b/openedx/core/djangoapps/content/search/tests/test_handlers.py index 2157fc9d5284..7ccbec687e98 100644 --- a/openedx/core/djangoapps/content/search/tests/test_handlers.py +++ b/openedx/core/djangoapps/content/search/tests/test_handlers.py @@ -11,7 +11,11 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangolib.testing.utils import skip_unless_cms -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_SPLIT_MODULESTORE, + ModuleStoreTestCase, + ImmediateOnCommitMixin, +) try: @@ -26,7 +30,7 @@ @patch("openedx.core.djangoapps.content.search.api.MeilisearchClient") @override_settings(MEILISEARCH_ENABLED=True) @skip_unless_cms -class TestUpdateIndexHandlers(ModuleStoreTestCase, LiveServerTestCase): +class TestUpdateIndexHandlers(ImmediateOnCommitMixin, ModuleStoreTestCase, LiveServerTestCase): """ Test that the search index is updated when XBlocks and Library Blocks are modified """ @@ -49,90 +53,86 @@ def setUp(self): def test_create_delete_xblock(self, meilisearch_client): # Create course - with self.captureOnCommitCallbacks(execute=True) as callbacks: - course = self.store.create_course( - self.orgA.short_name, - "test_course", - "test_run", - self.user_id, - fields={"display_name": "Test Course"}, - ) - course_access, _ = SearchAccess.objects.get_or_create(context_key=course.id) - - # Create XBlocks - created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) - with freeze_time(created_date): - sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") - doc_sequential = { - "id": "block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395", - "type": "course_block", - "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - "block_id": "test_sequential", - "display_name": "sequential", - "block_type": "sequential", - "context_key": "course-v1:orgA+test_course+test_run", - "org": "orgA", - "breadcrumbs": [ - { - "display_name": "Test Course", - }, - ], - "content": {}, - "access_id": course_access.id, - "modified": created_date.timestamp(), - } + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"display_name": "Test Course"}, + ) + course_access, _ = SearchAccess.objects.get_or_create(context_key=course.id) + + # Create XBlocks + created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc) + with freeze_time(created_date): + sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") + doc_sequential = { + "id": "block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395", + "type": "course_block", + "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + "block_id": "test_sequential", + "display_name": "sequential", + "block_type": "sequential", + "context_key": "course-v1:orgA+test_course+test_run", + "org": "orgA", + "breadcrumbs": [ + { + "display_name": "Test Course", + }, + ], + "content": {}, + "access_id": course_access.id, + "modified": created_date.timestamp(), + } meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_sequential]) - with self.captureOnCommitCallbacks(execute=True) as callbacks: - with freeze_time(created_date): - vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") - doc_vertical = { - "id": "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b", - "type": "course_block", - "usage_key": "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical", - "block_id": "test_vertical", - "display_name": "vertical", - "block_type": "vertical", - "context_key": "course-v1:orgA+test_course+test_run", - "org": "orgA", - "breadcrumbs": [ - { - "display_name": "Test Course", - }, - { - "display_name": "sequential", - "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", - }, - ], - "content": {}, - "access_id": course_access.id, - "modified": created_date.timestamp(), - } + with freeze_time(created_date): + vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") + doc_vertical = { + "id": "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b", + "type": "course_block", + "usage_key": "block-v1:orgA+test_course+test_run+type@vertical+block@test_vertical", + "block_id": "test_vertical", + "display_name": "vertical", + "block_type": "vertical", + "context_key": "course-v1:orgA+test_course+test_run", + "org": "orgA", + "breadcrumbs": [ + { + "display_name": "Test Course", + }, + { + "display_name": "sequential", + "usage_key": "block-v1:orgA+test_course+test_run+type@sequential+block@test_sequential", + }, + ], + "content": {}, + "access_id": course_access.id, + "modified": created_date.timestamp(), + } meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_vertical]) - with self.captureOnCommitCallbacks(execute=True) as callbacks: - # Update the XBlock - sequential = self.store.get_item(sequential.location, self.user_id) # Refresh the XBlock - sequential.display_name = "Updated Sequential" - modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) - with freeze_time(modified_date): - self.store.update_item(sequential, self.user_id) + # Update the XBlock + sequential = self.store.get_item(sequential.location, self.user_id) # Refresh the XBlock + sequential.display_name = "Updated Sequential" + modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc) + with freeze_time(modified_date): + self.store.update_item(sequential, self.user_id) - # The display name and the child's breadcrumbs should be updated - doc_sequential["display_name"] = "Updated Sequential" - doc_vertical["breadcrumbs"][1]["display_name"] = "Updated Sequential" - doc_sequential["modified"] = modified_date.timestamp() + # The display name and the child's breadcrumbs should be updated + doc_sequential["display_name"] = "Updated Sequential" + doc_vertical["breadcrumbs"][1]["display_name"] = "Updated Sequential" + doc_sequential["modified"] = modified_date.timestamp() meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([ doc_sequential, doc_vertical, ]) - with self.captureOnCommitCallbacks(execute=True) as callbacks: - # Delete the XBlock - self.store.delete_item(vertical.location, self.user_id) + # Delete the XBlock + self.store.delete_item(vertical.location, self.user_id) meilisearch_client.return_value.index.return_value.delete_document.assert_called_with( "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b" diff --git a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py index 32203f3929d5..997fffbaad8e 100644 --- a/openedx/core/djangoapps/content_tagging/tests/test_tasks.py +++ b/openedx/core/djangoapps/content_tagging/tests/test_tasks.py @@ -13,7 +13,11 @@ from common.djangoapps.student.tests.factories import UserFactory from openedx.core.djangolib.testing.utils import skip_unless_cms -from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase +from xmodule.modulestore.tests.django_utils import ( + TEST_DATA_SPLIT_MODULESTORE, + ModuleStoreTestCase, + ImmediateOnCommitMixin, +) from openedx.core.djangoapps.content_libraries.api import ( create_library, create_library_block, delete_library_block, restore_library_block ) @@ -59,6 +63,7 @@ def setUp(self): @override_waffle_flag(CONTENT_TAGGING_AUTO, active=True) class TestAutoTagging( # type: ignore[misc] LanguageTaxonomyTestMixin, + ImmediateOnCommitMixin, ModuleStoreTestCase, LiveServerTestCase ): @@ -199,27 +204,25 @@ def test_update_course(self): def test_create_delete_xblock(self): # Create course - with self.captureOnCommitCallbacks(execute=True) as callbacks: - course = self.store.create_course( - self.orgA.short_name, - "test_course", - "test_run", - self.user_id, - fields={"language": "pt-br"}, - ) + course = self.store.create_course( + self.orgA.short_name, + "test_course", + "test_run", + self.user_id, + fields={"language": "pt-br"}, + ) - # Create XBlocks - sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") - vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") + # Create XBlocks + sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential") + vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical") - usage_key_str = str(vertical.location) + usage_key_str = str(vertical.location) # Check if the tags are created in the XBlock assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, "Português (Brasil)") - with self.captureOnCommitCallbacks(execute=True) as callbacks: - # Delete the XBlock - self.store.delete_item(vertical.location, self.user_id) + # Delete the XBlock + self.store.delete_item(vertical.location, self.user_id) # Check if the tags are deleted assert self._check_tag(usage_key_str, LANGUAGE_TAXONOMY_ID, None) diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index 07f0cb513ecb..eccfa834d104 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -8,6 +8,7 @@ import logging import re import threading +import sys from abc import ABCMeta, abstractmethod from collections import defaultdict from contextlib import contextmanager diff --git a/xmodule/modulestore/tests/django_utils.py b/xmodule/modulestore/tests/django_utils.py index 952a88800f7c..ba85ce78611b 100644 --- a/xmodule/modulestore/tests/django_utils.py +++ b/xmodule/modulestore/tests/django_utils.py @@ -613,6 +613,35 @@ def update_course(self, course, user_id): return updated_course +class ImmediateOnCommitMixin: + """ + Mixin for tests that want `on_commit` callbacks to run immediately, + even under TestCase (which normally wraps tests in a transaction + that never commits). + Especially useful when the test needs to execute an event that occurs after an `on_commit` + """ + + @classmethod + def setUpClass(cls): + super_cls = super(ImmediateOnCommitMixin, cls) + if hasattr(super_cls, 'setUpClass'): + super_cls.setUpClass() + # Patch `transaction.on_commit` so that callbacks run immediately + cls._on_commit_patcher = patch( + 'django.db.transaction.on_commit', + side_effect=lambda func, **kwargs: func() + ) + cls._on_commit_patcher.start() + + @classmethod + def tearDownClass(cls): + # Stop patching, restore original behavior + cls._on_commit_patcher.stop() + super_cls = super(ImmediateOnCommitMixin, cls) + if hasattr(super_cls, 'tearDownClass'): + super_cls.tearDownClass() + + def upload_file_to_course(course_key, contentstore, source_file, target_filename): ''' Uploads the given source file to the given course, and returns the content of the file. From 6c2835e1607c1301247682963acf920f0ef451c5 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Fri, 17 Oct 2025 13:18:58 -0500 Subject: [PATCH 7/9] style: Fix broken lint --- .../contentstore/tests/test_upstream_downstream_links.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py b/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py index 87ded9107f6e..5c3ba8386480 100644 --- a/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py +++ b/cms/djangoapps/contentstore/tests/test_upstream_downstream_links.py @@ -265,7 +265,12 @@ def test_call_for_nonexistent_course(self): @skip_unless_cms -class TestUpstreamLinksEvents(ImmediateOnCommitMixin, ModuleStoreTestCase, OpenEdxEventsTestMixin, BaseUpstreamLinksHelpers): +class TestUpstreamLinksEvents( + ImmediateOnCommitMixin, + ModuleStoreTestCase, + OpenEdxEventsTestMixin, + BaseUpstreamLinksHelpers, +): """ Test signals related to managing upstream->downstream links. """ From f7cd6cdac622d166048e653ae97ca9e2ca812bc2 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 20 Oct 2025 12:39:21 -0500 Subject: [PATCH 8/9] style: Nits on the code --- xmodule/modulestore/tests/django_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xmodule/modulestore/tests/django_utils.py b/xmodule/modulestore/tests/django_utils.py index ba85ce78611b..f20aaa3a56d3 100644 --- a/xmodule/modulestore/tests/django_utils.py +++ b/xmodule/modulestore/tests/django_utils.py @@ -623,7 +623,7 @@ class ImmediateOnCommitMixin: @classmethod def setUpClass(cls): - super_cls = super(ImmediateOnCommitMixin, cls) + super_cls = super() if hasattr(super_cls, 'setUpClass'): super_cls.setUpClass() # Patch `transaction.on_commit` so that callbacks run immediately @@ -637,7 +637,7 @@ def setUpClass(cls): def tearDownClass(cls): # Stop patching, restore original behavior cls._on_commit_patcher.stop() - super_cls = super(ImmediateOnCommitMixin, cls) + super_cls = super() if hasattr(super_cls, 'tearDownClass'): super_cls.tearDownClass() From 75114f6c3e57ba5204a6763cbd74557a1400d4d3 Mon Sep 17 00:00:00 2001 From: XnpioChV Date: Mon, 20 Oct 2025 13:21:16 -0500 Subject: [PATCH 9/9] style: Nits on the code --- xmodule/modulestore/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/xmodule/modulestore/__init__.py b/xmodule/modulestore/__init__.py index eccfa834d104..51118cc42e67 100644 --- a/xmodule/modulestore/__init__.py +++ b/xmodule/modulestore/__init__.py @@ -8,13 +8,11 @@ import logging import re import threading -import sys from abc import ABCMeta, abstractmethod from collections import defaultdict from contextlib import contextmanager from operator import itemgetter from django.db import transaction -from django.conf import settings from opaque_keys.edx.keys import AssetKey, CourseKey from opaque_keys.edx.locations import Location # For import backwards compatibility