diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index ec2b5489afc4..49f423ce7ac3 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -106,7 +106,7 @@ def get_collected(self, user=None): BlockStructureTransformers.verify_versions(block_structure) except (BlockStructureNotFound, TransformerDataIncompatible): - if user: + if user and getattr(user, "known", True): # This bypasses the runtime access checks. When we are populating the course blocks cache, # we do not want to perform access checks. Access checks result in inconsistent blocks where # inaccessible blocks are missing from the cache. Cached course blocks are then used for all the users. 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 8e5b585ca879..f24a1adf9c84 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -2,11 +2,17 @@ Tests for manager.py """ -import pytest import ddt +import pytest from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_switch +from common.djangoapps.student.tests.factories import CourseEnrollmentFactory +from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase +from lms.djangoapps.course_blocks.transformers.tests.test_user_partitions import UserPartitionTestMixin +from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager +from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort + from ..block_structure import BlockStructureBlockData from ..config import STORAGE_BACKING_FOR_CACHE from ..exceptions import UsageKeyNotInBlockStructure @@ -220,3 +226,150 @@ 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 + + +@ddt.ddt +class TestBlockStructureManagerGetCollected(UserPartitionTestMixin, CourseStructureTestCase): + """ + Tests `BlockStructureManager.get_collected` + """ + + def setup_partitions_and_course(self): + """ + Setup course structure and create user. + """ + # Set up user partitions and groups. + self.setup_groups_partitions(active=True) + self.user_partition = self.user_partitions[0] + + # Build course. + self.course_hierarchy = self.get_course_hierarchy() + self.blocks = self.build_course(self.course_hierarchy) + self.course = self.blocks['course'] + + # Enroll user in course. + CourseEnrollmentFactory.create( + user=self.user, course_id=self.course.id, is_active=True + ) + + # Set up cohorts. + self.setup_cohorts(self.course) + + def get_course_hierarchy(self): + """ + Returns a course hierarchy to test with. + """ + # course + # / \ + # / \ + # A[1, 2, 3] B + # / | \ | + # / | \ | + # / | \ | + # C[1, 2] D[2, 3] E / + # / | \ | / \ / + # / | \ | / \ / + # / | \ | / \ / + # F G[1] H[2] I J K[4] / + # / \ / / \ / + # / \ / / \ / + # / \ / / \/ + # L[1, 2] M[1, 2, 3] N O + # + return [ + { + 'org': 'UserPartitionTransformer', + 'course': 'UP101F', + 'run': 'test_run', + 'user_partitions': [self.user_partition], + '#type': 'course', + '#ref': 'course', + '#children': [ + { + '#type': 'vertical', + '#ref': 'A', + 'metadata': {'group_access': {self.user_partition.id: [0, 1, 2, 3]}}, + }, + {'#type': 'vertical', '#ref': 'B'}, + ], + }, + { + '#type': 'vertical', + '#ref': 'C', + '#parents': ['A'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2]}}, + '#children': [ + {'#type': 'vertical', '#ref': 'F'}, + { + '#type': 'vertical', + '#ref': 'G', + 'metadata': {'group_access': {self.user_partition.id: [1]}}, + }, + { + '#type': 'vertical', + '#ref': 'H', + 'metadata': {'group_access': {self.user_partition.id: [2]}}, + }, + ], + }, + { + '#type': 'vertical', + '#ref': 'D', + '#parents': ['A'], + 'metadata': {'group_access': {self.user_partition.id: [2, 3]}}, + '#children': [{'#type': 'vertical', '#ref': 'I'}], + }, + { + '#type': 'vertical', + '#ref': 'E', + '#parents': ['A'], + '#children': [{'#type': 'vertical', '#ref': 'J'}], + }, + { + '#type': 'vertical', + '#ref': 'K', + '#parents': ['E'], + 'metadata': {'group_access': {self.user_partition.id: [4, 51]}}, + '#children': [{'#type': 'vertical', '#ref': 'N'}], + }, + { + '#type': 'vertical', + '#ref': 'L', + '#parents': ['G'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2]}}, + }, + { + '#type': 'vertical', + '#ref': 'M', + '#parents': ['G', 'H'], + 'metadata': {'group_access': {self.user_partition.id: [1, 2, 3]}}, + }, + { + '#type': 'vertical', + '#ref': 'O', + '#parents': ['K', 'B'], + }, + ] + + @ddt.data( + (None, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O')), + (1, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O')), + (2, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O')), + (3, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O')), + (4, ('course', 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O')), + ) + @ddt.unpack + def test_get_collected(self, group_id, expected_blocks): + """ + Test that `BlockStructureManager.get_collected` returns all course blocks regardless of the user group. + """ + self.setup_partitions_and_course() + if group_id: + cohort = self.partition_cohorts[self.user_partition.id - 1][group_id - 1] + add_user_to_cohort(cohort, self.user.username) + + trans_block_structure = get_block_structure_manager(self.course.location.course_key).get_collected(self.user) + self.assertSetEqual( + set(trans_block_structure.get_block_keys()), + self.get_block_key_set(self.blocks, *expected_blocks) + )