diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index 0adff9c75e..12636fef52 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -26,7 +26,7 @@ def get_blocks( Arguments: request (HTTPRequest): Used for calling django reverse. - usage_key (UsageKey): Identifies the root block of interest. + usage_key (UsageKey): Identifies the starting block of interest. user (User): Optional user object for whom the blocks are being retrieved. If None, blocks are returned regardless of access checks. depth (integer or None): Identifies the depth of the tree to return diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index fb6bfa91ef..b4ab1112a5 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -58,3 +58,22 @@ class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): self.assertIn(unicode(problem_block.location), vertical_descendants) self.assertNotIn(unicode(self.html_block.location), vertical_descendants) + + def test_sub_structure(self): + sequential_block = self.store.get_item(self.course.id.make_usage_key('sequential', 'sequential_y1')) + + blocks = get_blocks(self.request, sequential_block.location, self.user) + self.assertEquals(blocks['root'], unicode(sequential_block.location)) + self.assertEquals(len(blocks['blocks']), 5) + + for block_type, block_name, is_inside_of_structure in ( + ('vertical', 'vertical_y1a', True), + ('problem', 'problem_y1a_1', True), + ('chapter', 'chapter_y', False), + ('sequential', 'sequential_x1', False), + ): + block = self.store.get_item(self.course.id.make_usage_key(block_type, block_name)) + if is_inside_of_structure: + self.assertIn(unicode(block.location), blocks['blocks']) + else: + self.assertNotIn(unicode(block.location), blocks['blocks']) diff --git a/lms/djangoapps/course_api/blocks/views.py b/lms/djangoapps/course_api/blocks/views.py index a3912ededa..8cbc4dcee4 100644 --- a/lms/djangoapps/course_api/blocks/views.py +++ b/lms/djangoapps/course_api/blocks/views.py @@ -89,7 +89,8 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView): The following fields are returned with a successful response. - * root: The ID of the root node of the course blocks. + * root: The ID of the root node of the requested course block + structure. * blocks: A dictionary that maps block usage IDs to a collection of information about each block. Each block contains the following diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index b41aeb1452..4bf24c3af7 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -28,27 +28,20 @@ COURSE_BLOCK_ACCESS_TRANSFORMERS = [ def get_course_blocks( user, - root_block_usage_key, + starting_block_usage_key, transformers=None, ): """ A higher order function implemented on top of the block_structure.get_blocks function returning a transformed block - structure for the given user starting at root_block_usage_key. - - Note: The current implementation requires the root_block_usage_key - to be the root block of its corresponding course. However, this - is a short-term limitation, which will be addressed in a coming - ticket (https://openedx.atlassian.net/browse/MA-1604). Once that - ticket is implemented, callers will be able to get course blocks - starting at any arbitrary location within a block structure. + structure for the given user starting at starting_block_usage_key. Arguments: user (django.contrib.auth.models.User) - User object for which the block structure is to be transformed. - root_block_usage_key (UsageKey) - The usage_key for the root - of the block structure that is being accessed. + starting_block_usage_key (UsageKey) - Specifies the starting block + of the block structure that is to be transformed. transformers (BlockStructureTransformers) - A collection of transformers whose transform methods are to be called. @@ -56,26 +49,21 @@ def get_course_blocks( Returns: BlockStructureBlockData - A transformed block structure, - starting at root_block_usage_key, that has undergone the + starting at starting_block_usage_key, that has undergone the transform methods for the given user and the course associated with the block structure. If using the default transformers, the transformed block structure will be exactly equivalent to the blocks that the given user has access. """ - if root_block_usage_key != modulestore().make_course_usage_key(root_block_usage_key.course_key): - # Enforce this check for now until MA-1604 is implemented. - # Otherwise, callers will get incorrect block data after a - # new version of the course is published, since - # clear_course_from_cache only clears the cached block - # structures starting at the root block of the course. - raise NotImplementedError - if not transformers: transformers = BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS) - transformers.usage_info = CourseUsageInfo(root_block_usage_key.course_key, user) + transformers.usage_info = CourseUsageInfo(starting_block_usage_key.course_key, user) - return _get_block_structure_manager(root_block_usage_key.course_key).get_transformed(transformers) + return _get_block_structure_manager(starting_block_usage_key.course_key).get_transformed( + transformers, + starting_block_usage_key, + ) def get_course_in_cache(course_key): diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index caef8fb1ba..8fb118d650 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -102,6 +102,21 @@ class BlockStructure(object): """ return self._block_relations[usage_key].children if self.has_block(usage_key) else [] + def set_root_block(self, usage_key): + """ + Sets the given usage key as the new root of the block structure. + + Note: This method does *not* prune the rest of the structure. For + performance reasons, it is left to the caller to decide when exactly + to prune. + + Arguments: + usage_key - The usage key of the block that is to be set as the + new root of the block structure. + """ + self.root_block_usage_key = usage_key + self._block_relations[usage_key].parents = [] + def has_block(self, usage_key): """ Returns whether a block with the given usage_key is in this diff --git a/openedx/core/lib/block_structure/exceptions.py b/openedx/core/lib/block_structure/exceptions.py index 898150fc5a..8d17282464 100644 --- a/openedx/core/lib/block_structure/exceptions.py +++ b/openedx/core/lib/block_structure/exceptions.py @@ -1,5 +1,5 @@ """ -Application-specific exceptions raised by the block cache framework. +Application-specific exceptions raised by the block structure framework. """ @@ -8,3 +8,10 @@ class TransformerException(Exception): Exception class for Transformer related errors. """ pass + + +class UsageKeyNotInBlockStructure(Exception): + """ + Exception for when a usage key is not found within a block structure. + """ + pass diff --git a/openedx/core/lib/block_structure/manager.py b/openedx/core/lib/block_structure/manager.py index 7711a0c98f..7df65d66b5 100644 --- a/openedx/core/lib/block_structure/manager.py +++ b/openedx/core/lib/block_structure/manager.py @@ -2,8 +2,9 @@ Top-level module for the Block Structure framework with a class for managing BlockStructures. """ -from .factory import BlockStructureFactory from .cache import BlockStructureCache +from .factory import BlockStructureFactory +from .exceptions import UsageKeyNotInBlockStructure from .transformers import BlockStructureTransformers @@ -30,23 +31,39 @@ class BlockStructureManager(object): self.modulestore = modulestore self.block_structure_cache = BlockStructureCache(cache) - def get_transformed(self, transformers): + def get_transformed(self, transformers, starting_block_usage_key=None): """ Returns the transformed Block Structure for the root_block_usage_key, - getting block data from the cache and modulestore, as needed. + starting at starting_block_usage_key, getting block data from the cache + and modulestore, as needed. - Details: Same as the get_collected method, except the transformers' + Details: Similar to the get_collected method, except the transformers' transform methods are also called. Arguments: transformers (BlockStructureTransformers) - Collection of transformers to apply. + starting_block_usage_key (UsageKey) - Specifies the starting block + in the block structure that is to be transformed. + If None, root_block_usage_key is used. + Returns: BlockStructureBlockData - A transformed block structure, - starting at self.root_block_usage_key. + starting at starting_block_usage_key. """ block_structure = self.get_collected() + if starting_block_usage_key: + # Override the root_block_usage_key so traversals start at the + # requested location. The rest of the structure will be pruned + # as part of the transformation. + if not block_structure.has_block(starting_block_usage_key): + raise UsageKeyNotInBlockStructure( + "The requested usage_key '{0}' is not found in the block_structure with root '{1}'", + unicode(starting_block_usage_key), + unicode(self.root_block_usage_key), + ) + block_structure.set_root_block(starting_block_usage_key) transformers.transform(block_structure) return block_structure diff --git a/openedx/core/lib/block_structure/tests/test_manager.py b/openedx/core/lib/block_structure/tests/test_manager.py index e0abc45b61..19ea53e5de 100644 --- a/openedx/core/lib/block_structure/tests/test_manager.py +++ b/openedx/core/lib/block_structure/tests/test_manager.py @@ -3,6 +3,7 @@ Tests for manager.py """ from unittest import TestCase +from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager from ..transformers import BlockStructureTransformers from .helpers import ( @@ -127,6 +128,19 @@ class TestBlockStructureManager(TestCase, ChildrenMapTestMixin): TestTransformer1.assert_collected(block_structure) TestTransformer1.assert_transformed(block_structure) + def test_get_transformed_with_starting_block(self): + with mock_registered_transformers(self.registered_transformers): + block_structure = self.bs_manager.get_transformed(self.transformers, starting_block_usage_key=1) + substructure_of_children_map = [[], [3, 4], [], [], []] + self.assert_block_structure(block_structure, substructure_of_children_map, missing_blocks=[0, 2]) + TestTransformer1.assert_collected(block_structure) + TestTransformer1.assert_transformed(block_structure) + + def test_get_transformed_with_nonexistent_starting_block(self): + with mock_registered_transformers(self.registered_transformers): + with self.assertRaises(UsageKeyNotInBlockStructure): + self.bs_manager.get_transformed(self.transformers, starting_block_usage_key=100) + def test_get_collected_cached(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False)