From 1f86786b151d0cb880cf9a484b6afe66c46ffc25 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Wed, 16 Oct 2024 13:08:54 +0500 Subject: [PATCH 1/9] fix: bypass access checks when populating course blocks cache --- lms/djangoapps/course_blocks/api.py | 1 + .../content/block_structure/manager.py | 23 +++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index c978f54e02d5..de4e0443fb08 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -108,4 +108,5 @@ def get_course_blocks( transformers, starting_block_usage_key, collected_block_structure, + user, ) diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index 2f8a041c3163..ec2b5489afc4 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -35,7 +35,7 @@ def __init__(self, root_block_usage_key, modulestore, cache): self.modulestore = modulestore self.store = BlockStructureStore(cache) - def get_transformed(self, transformers, starting_block_usage_key=None, collected_block_structure=None): + def get_transformed(self, transformers, starting_block_usage_key=None, collected_block_structure=None, user=None): """ Returns the transformed Block Structure for the root_block_usage_key, starting at starting_block_usage_key, getting block data from the cache @@ -57,11 +57,14 @@ def get_transformed(self, transformers, starting_block_usage_key=None, collected get_collected. Can be optionally provided if already available, for optimization. + user (django.contrib.auth.models.User) - User object for + which the block structure is to be transformed. + Returns: BlockStructureBlockData - A transformed block structure, starting at starting_block_usage_key. """ - block_structure = collected_block_structure.copy() if collected_block_structure else self.get_collected() + block_structure = collected_block_structure.copy() if collected_block_structure else self.get_collected(user) if starting_block_usage_key: # Override the root_block_usage_key so traversals start at the @@ -77,7 +80,7 @@ def get_transformed(self, transformers, starting_block_usage_key=None, collected transformers.transform(block_structure) return block_structure - def get_collected(self): + def get_collected(self, user=None): """ Returns the collected Block Structure for the root_block_usage_key, getting block data from the cache and modulestore, as needed. @@ -86,6 +89,10 @@ def get_collected(self): the modulestore is accessed if needed (at cache miss), and the transformers data is collected if needed. + In the case of a cache miss, the function bypasses runtime access checks for the current + user. This is done to prevent inconsistencies in the data, which can occur when + certain blocks are inaccessible due to access restrictions. + Returns: BlockStructureBlockData - A collected block structure, starting at root_block_usage_key, with collected data @@ -99,7 +106,15 @@ def get_collected(self): BlockStructureTransformers.verify_versions(block_structure) except (BlockStructureNotFound, TransformerDataIncompatible): - block_structure = self._update_collected() + if user: + # 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. + user.known = False + block_structure = self._update_collected() + user.known = True + else: + block_structure = self._update_collected() return block_structure From cf3c10ba34cea8da2e38d20f6f2e4911245b3cb4 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Wed, 23 Oct 2024 14:59:11 +0500 Subject: [PATCH 2/9] test: add tests --- .../content/block_structure/manager.py | 2 +- .../block_structure/tests/test_manager.py | 155 +++++++++++++++++- 2 files changed, 155 insertions(+), 2 deletions(-) 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) + ) From d9a27b6ba7c64f3113a9f3280eaf631fbbdd6a40 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Wed, 23 Oct 2024 15:36:32 +0500 Subject: [PATCH 3/9] style: pylint --- .../djangoapps/content/block_structure/tests/test_manager.py | 1 + 1 file changed, 1 insertion(+) 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 f24a1adf9c84..aa248f093c82 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -1,3 +1,4 @@ +# pylint: disable=attribute-defined-outside-init """ Tests for manager.py """ From 9ed41b075955b57b0e81f675fff79c93ee468a43 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 31 Oct 2024 23:19:52 +0500 Subject: [PATCH 4/9] test: add tests for known and unknown user --- .../course_blocks/tests/test_api.py | 148 +++++++++++++++++ .../block_structure/tests/test_manager.py | 154 +----------------- 2 files changed, 149 insertions(+), 153 deletions(-) create mode 100644 lms/djangoapps/course_blocks/tests/test_api.py diff --git a/lms/djangoapps/course_blocks/tests/test_api.py b/lms/djangoapps/course_blocks/tests/test_api.py new file mode 100644 index 000000000000..25b4f0374707 --- /dev/null +++ b/lms/djangoapps/course_blocks/tests/test_api.py @@ -0,0 +1,148 @@ +# pylint: disable=attribute-defined-outside-init +""" +Tests for course_blocks API +""" + +from unittest.mock import Mock, patch + +import ddt +from django.http.request import HttpRequest + +from common.djangoapps.student.tests.factories import UserFactory +from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase +from lms.djangoapps.course_blocks.transformers.tests.test_user_partitions import UserPartitionTestMixin +from lms.djangoapps.courseware.block_render import make_track_function, prepare_runtime_for_user +from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers +from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort +from xmodule.modulestore.django import modulestore + + +def get_block_side_effect(block_locator, user_known): + """ + Side effect for `CachingDescriptorSystem.get_block` + """ + store = modulestore() + course = store.get_course(block_locator.course_key) + block = store.get_item(block_locator) + runtime = block.runtime + user = UserFactory.create() + user.known = user_known + + prepare_runtime_for_user( + user=user, + student_data=Mock(), + runtime=runtime, + course_id=block_locator.course_key, + track_function=make_track_function(HttpRequest()), + request_token=Mock(), + course=course, + ) + return block.runtime.get_block_for_descriptor(block) + + +def get_block_side_effect_for_known_user(self, *args, **kwargs): + """ + Side effect for known user test. + """ + return get_block_side_effect(self, True) + + +def get_block_side_effect_for_unknown_user(self, *args, **kwargs): + """ + Side effect for unknown user test. + """ + return get_block_side_effect(self, False) + + + +@ddt.ddt +class TestGetCourseBlocks(UserPartitionTestMixin, CourseStructureTestCase): + """ + Tests `get_course_blocks` API + """ + + def setup_partitions_and_course(self): + """ + Setup course structure. + """ + # Set up user partitions and groups. + self.setup_groups_partitions(active=True, num_groups=1) + 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'] + + # Set up cohorts. + self.setup_cohorts(self.course) + + def get_course_hierarchy(self): + """ + Returns a course hierarchy to test with. + """ + # course + # / \ + # / \ + # A[0, 1, 2, 3] B + # | + # | + # 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]}}, + }, + {'#type': 'vertical', '#ref': 'B'}, + ], + }, + { + '#type': 'vertical', + '#ref': 'O', + '#parents': ['B'], + }, + ] + + @ddt.data( + (1, ('course', 'B', 'O'), True), + (1, ('course', 'A', 'B', 'O'), False), + ) + @ddt.unpack + def test_get_course_blocks(self, group_id, expected_blocks, user_known): + """ + Tests that `get_course_blocks` returns blocks without access checks for unknown users. + + Access checks are done through the transformers and through Runtime get_block_for_descriptor. Due + to the runtime limitations during the tests, the Runtime access checks are not performed as + get_block_for_descriptor is never called and Block is returned by CachingDescriptorSystem.get_block. + In this test, we mock the CachingDescriptorSystem.get_block and check block access for known and unknown users. + For known users, it performs the Runtime access checks through get_block_for_descriptor. For unknown, it + skips the access checks. + """ + 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) + + side_effect = get_block_side_effect_for_known_user if user_known else get_block_side_effect_for_unknown_user + with patch('xmodule.modulestore.split_mongo.split.CachingDescriptorSystem.get_block', side_effect=side_effect): + block_structure = get_course_blocks( + self.user, + self.course.location, + BlockStructureTransformers([]), + ) + self.assertSetEqual( + set(block_structure.get_block_keys()), + self.get_block_key_set(self.blocks, *expected_blocks) + ) 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 aa248f093c82..b76e4d30d21d 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -5,15 +5,10 @@ import ddt import pytest +from django.core.cache import cache 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 @@ -227,150 +222,3 @@ 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) - ) From d81c2d8be52bac3e1d159e70964a987a16927047 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 31 Oct 2024 23:24:40 +0500 Subject: [PATCH 5/9] refactor: revert imports --- .../djangoapps/content/block_structure/tests/test_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 b76e4d30d21d..6e0580b7e88b 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -1,11 +1,10 @@ -# pylint: disable=attribute-defined-outside-init """ Tests for manager.py """ +import pytest import ddt import pytest -from django.core.cache import cache from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_switch From 65fd18bdcf16583aa4d0bb39de2e2611add90cd4 Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 31 Oct 2024 23:26:23 +0500 Subject: [PATCH 6/9] refactor: revert imports --- .../djangoapps/content/block_structure/tests/test_manager.py | 1 - 1 file changed, 1 deletion(-) 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 6e0580b7e88b..8e5b585ca879 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -4,7 +4,6 @@ import pytest import ddt -import pytest from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_switch From 29d748de0f9bd1807dd0e53343e42e7c3dcdac4f Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Thu, 31 Oct 2024 23:36:28 +0500 Subject: [PATCH 7/9] style: pycodestyle fix --- lms/djangoapps/course_blocks/tests/test_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/course_blocks/tests/test_api.py b/lms/djangoapps/course_blocks/tests/test_api.py index 25b4f0374707..a9563b2a4298 100644 --- a/lms/djangoapps/course_blocks/tests/test_api.py +++ b/lms/djangoapps/course_blocks/tests/test_api.py @@ -55,7 +55,6 @@ def get_block_side_effect_for_unknown_user(self, *args, **kwargs): return get_block_side_effect(self, False) - @ddt.ddt class TestGetCourseBlocks(UserPartitionTestMixin, CourseStructureTestCase): """ From 61df45712e72155198d452b82fee9d271a874e5a Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Fri, 1 Nov 2024 11:33:48 +0500 Subject: [PATCH 8/9] refactor: update docs --- lms/djangoapps/course_blocks/tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/course_blocks/tests/test_api.py b/lms/djangoapps/course_blocks/tests/test_api.py index a9563b2a4298..b30e7b8b031d 100644 --- a/lms/djangoapps/course_blocks/tests/test_api.py +++ b/lms/djangoapps/course_blocks/tests/test_api.py @@ -84,7 +84,7 @@ def get_course_hierarchy(self): # course # / \ # / \ - # A[0, 1, 2, 3] B + # A[0] B # | # | # O From 361ed61d05861b87391099615423764b44ed810a Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Fri, 1 Nov 2024 11:58:35 +0500 Subject: [PATCH 9/9] test: add tests when user is not in any group --- lms/djangoapps/course_blocks/tests/test_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lms/djangoapps/course_blocks/tests/test_api.py b/lms/djangoapps/course_blocks/tests/test_api.py index b30e7b8b031d..52e9c86ec324 100644 --- a/lms/djangoapps/course_blocks/tests/test_api.py +++ b/lms/djangoapps/course_blocks/tests/test_api.py @@ -116,6 +116,8 @@ def get_course_hierarchy(self): @ddt.data( (1, ('course', 'B', 'O'), True), (1, ('course', 'A', 'B', 'O'), False), + (None, ('course', 'B', 'O'), True), + (None, ('course', 'A', 'B', 'O'), False), ) @ddt.unpack def test_get_course_blocks(self, group_id, expected_blocks, user_known):