diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py index 804f9037791f..72465b299ade 100644 --- a/lms/djangoapps/courseware/tests/test_courses.py +++ b/lms/djangoapps/courseware/tests/test_courses.py @@ -88,7 +88,7 @@ def test_get_course_func_with_access_error(self, course_access_func_name): assert not error.value.access_response.has_access @ddt.data( - (GET_COURSE_WITH_ACCESS, 2), + (GET_COURSE_WITH_ACCESS, 1), (GET_COURSE_OVERVIEW_WITH_ACCESS, 0), ) @ddt.unpack diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 347879105aad..8f32991272e2 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -156,8 +156,8 @@ def test_block_structure_created_only_once(self): assert mock_block_structure_create.call_count == 1 @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42, True), - (ModuleStoreEnum.Type.split, 2, 42, False), + (ModuleStoreEnum.Type.split, 1, 42, True), + (ModuleStoreEnum.Type.split, 1, 42, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -168,7 +168,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat self._apply_recalculate_subsection_grade() @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -200,16 +200,17 @@ def test_other_inaccessible_subsection(self, mock_subsection_signal): with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): self.store.update_item(sequential, self.user.id) - # Make sure the signal is sent for only the 2 accessible sequentials. + # Make sure the signal is sent for only the 1 accessible sequentials. + # Update: draft branch content shouldn't be accessible self._apply_recalculate_subsection_grade() - assert mock_subsection_signal.call_count == 2 + assert mock_subsection_signal.call_count == 1 sequentials_signalled = { args[1]['subsection_grade'].location for args in mock_subsection_signal.call_args_list } self.assertSetEqual( sequentials_signalled, - {self.sequential.location, accessible_seq.location}, + {self.sequential.location}, ) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') @@ -255,7 +256,7 @@ def test_problem_block_with_restricted_access(self, mock_subsection_signal): UserPartition.scheme_extensions = None @ddt.data( - (ModuleStoreEnum.Type.split, 2, 42), + (ModuleStoreEnum.Type.split, 1, 42), ) @ddt.unpack def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index 49f423ce7ac3..b6a447bdf201 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -6,6 +6,8 @@ from contextlib import contextmanager +from xmodule.modulestore import ModuleStoreEnum + from .exceptions import BlockStructureNotFound, TransformerDataIncompatible, UsageKeyNotInBlockStructure from .factory import BlockStructureFactory from .store import BlockStructureStore @@ -104,7 +106,6 @@ def get_collected(self, user=None): self.store, ) BlockStructureTransformers.verify_versions(block_structure) - except (BlockStructureNotFound, TransformerDataIncompatible): if user and getattr(user, "known", True): # This bypasses the runtime access checks. When we are populating the course blocks cache, @@ -133,10 +134,16 @@ def _update_collected(self): the modulestore. """ with self._bulk_operations(): - block_structure = BlockStructureFactory.create_from_modulestore( - self.root_block_usage_key, - self.modulestore, - ) + # Always uses published-only branch regardless of CMS or LMS context. + with self.modulestore.branch_setting( + ModuleStoreEnum.Branch.published_only, + self.root_block_usage_key.course_key + ): + block_structure = BlockStructureFactory.create_from_modulestore( + self.root_block_usage_key, + self.modulestore, + ) + BlockStructureTransformers.collect(block_structure) self.store.add(block_structure) return block_structure diff --git a/openedx/core/djangoapps/content/block_structure/tests/helpers.py b/openedx/core/djangoapps/content/block_structure/tests/helpers.py index f0a20554e6d2..df53b33d7b2e 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/helpers.py +++ b/openedx/core/djangoapps/content/block_structure/tests/helpers.py @@ -110,6 +110,13 @@ def bulk_operations(self, ignore): # pylint: disable=unused-argument """ yield + @contextmanager + def branch_setting(self, branch_settings, course_id=None): # pylint: disable=unused-argument + """ + A context manager for temporarily setting a store's branch value on the current thread. + """ + yield + class MockCache: """ diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py index 493e4c34714f..1c4060871c97 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -4,8 +4,11 @@ import pytest import ddt +from unittest.mock import MagicMock from django.test import TestCase +from xmodule.modulestore import ModuleStoreEnum + from ..block_structure import BlockStructureBlockData from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager @@ -216,3 +219,37 @@ def test_clear(self): self.bs_manager.clear() self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) assert TestTransformer1.collect_call_count == 2 + + def test_update_collected_branch_context_integration(self): + """ + Integration test to verify the published-only branch context works end-to-end. + """ + # Track branch setting calls on our mock modulestore + attr_name = 'branch_setting' + original_branch_setting = getattr(self.modulestore, attr_name, None) + branch_setting_calls = [] + + def mock_branch_setting(branch, course_key): + branch_setting_calls.append((branch, course_key)) + # Return a proper context manager that does nothing + return MagicMock(__enter__=MagicMock(), __exit__=MagicMock()) + + # Add the branch_setting method to our mock modulestore + setattr(self.modulestore, attr_name, mock_branch_setting) + + try: + with mock_registered_transformers(self.registered_transformers): + self.bs_manager.get_collected() + + # Verify branch_setting was called with the correct parameters + self.assertEqual(len(branch_setting_calls), 1) + branch, course_key = branch_setting_calls[0] + self.assertEqual(branch, ModuleStoreEnum.Branch.published_only) + self.assertEqual(course_key, self.block_key_factory(0).course_key) + + finally: + # Restore original method if it existed + if original_branch_setting is not None: + setattr(self.modulestore, attr_name, original_branch_setting) + elif hasattr(self.modulestore, attr_name): + delattr(self.modulestore, attr_name)