Merge pull request #11579 from edx/course_blocks_api/support_substructure

Course Blocks API: Support accessing a substructure MA-1604

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

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'])

lms/djangoapps/course_api/blocks/transformers/tests/test_navigation.py

@@ -88,14 +88,14 @@ class BlockNavigationTransformerCourseTestCase(ModuleStoreTestCase):
         BlockNavigationTransformer.collect(block_structure)
         block_structure._collect_requested_xblock_fields()
 
-        self.assertTrue(block_structure.has_block(chapter_x_key))
+        self.assertIn(chapter_x_key, block_structure)
 
         # transform phase
         BlockDepthTransformer().transform(usage_info=None, block_structure=block_structure)
         BlockNavigationTransformer(0).transform(usage_info=None, block_structure=block_structure)
         block_structure._prune_unreachable()
 
-        self.assertTrue(block_structure.has_block(chapter_x_key))
+        self.assertIn(chapter_x_key, block_structure)
 
         course_descendants = block_structure.get_transformer_block_field(
             course_usage_key,

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

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):

lms/djangoapps/course_blocks/transformers/tests/helpers.py

@@ -311,7 +311,7 @@ class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase)
         for i, xblock_key in enumerate(self.xblock_keys):
 
             # compute access results of the block
-            block_structure_result = block_structure.has_block(xblock_key)
+            block_structure_result = xblock_key in block_structure
             has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id))
 
             # compare with expected value

openedx/core/lib/block_structure/block_structure.py

@@ -79,6 +79,7 @@ class BlockStructure(object):
         Returns the parents of the block identified by the given
         usage_key.
 
+
         Arguments:
             usage_key - The usage key of the block whose parents
                 are to be returned.
@@ -86,7 +87,7 @@ class BlockStructure(object):
         Returns:
             [UsageKey] - A list of usage keys of the block's parents.
         """
-        return self._block_relations[usage_key].parents if self.has_block(usage_key) else []
+        return self._block_relations[usage_key].parents if usage_key in self else []
 
     def get_children(self, usage_key):
         """
@@ -100,9 +101,24 @@ class BlockStructure(object):
         Returns:
             [UsageKey] - A list of usage keys of the block's children.
         """
-        return self._block_relations[usage_key].children if self.has_block(usage_key) else []
+        return self._block_relations[usage_key].children if usage_key in self 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):
+    def __contains__(self, usage_key):
         """
         Returns whether a block with the given usage_key is in this
         block structure.

openedx/core/lib/block_structure/exceptions.py

 """
-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

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 starting_block_usage_key not in block_structure:
+                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
 

openedx/core/lib/block_structure/tests/helpers.py

@@ -219,7 +219,7 @@ class ChildrenMapTestMixin(object):
         for block_key, children in enumerate(children_map):
             # Verify presence
             self.assertEquals(
-                block_structure.has_block(block_key),
+                block_key in block_structure,
                 block_key not in missing_blocks,
                 'Expected presence in block_structure for block_key {} to match absence in missing_blocks.'.format(
                     unicode(block_key)

openedx/core/lib/block_structure/tests/test_block_structure.py

@@ -37,10 +37,10 @@ class TestBlockStructure(TestCase, ChildrenMapTestMixin):
         for child, parents in enumerate(self.get_parents_map(children_map)):
             self.assertSetEqual(set(block_structure.get_parents(child)), set(parents))
 
-        # has_block
+        # __contains__
         for node in range(len(children_map)):
-            self.assertTrue(block_structure.has_block(node))
-        self.assertFalse(block_structure.has_block(len(children_map) + 1))
+            self.assertIn(node, block_structure)
+        self.assertNotIn(len(children_map) + 1, block_structure)
 
 
 @ddt.ddt

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)