Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lms/djangoapps/courseware/tests/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 8 additions & 7 deletions lms/djangoapps/grades/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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):
Expand Down
17 changes: 12 additions & 5 deletions openedx/core/djangoapps/content/block_structure/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Loading