Merge pull request #19478 from edx/hide-empty-content-course-blocks

REVE-181: Hide empty container blocks from the course blocks api when called by the mobile app

common/lib/xmodule/xmodule/modulestore/tests/factories.py

@@ -149,6 +149,7 @@ class SampleCourseFactory(CourseFactory):
         block_info_tree = kwargs.pop('block_info_tree', default_block_info_tree)
         store = kwargs.get('modulestore')
         user_id = kwargs.get('user_id', ModuleStoreEnum.UserID.test)
+
         with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, None):
             course = super(SampleCourseFactory, cls)._create(target_class, **kwargs)
 

lms/djangoapps/course_api/blocks/api.py

@@ -4,7 +4,9 @@ API function for retrieving course blocks data
 
 import lms.djangoapps.course_blocks.api as course_blocks_api
 from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer
+from lms.djangoapps.course_blocks.transformers.hide_empty import HideEmptyTransformer
 from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
+from openedx.core.lib.mobile_utils import is_request_from_mobile_app
 
 from .serializers import BlockDictSerializer, BlockSerializer
 from .transformers.blocks_api import BlocksAPITransformer
@@ -60,10 +62,18 @@ def get_blocks(
 
     if user is not None:
         transformers += course_blocks_api.get_course_block_access_transformers(user)
-        transformers += [MilestonesAndSpecialExamsTransformer(
-            include_special_exams=include_special_exams,
-            include_gated_sections=include_gated_sections)]
-        transformers += [HiddenContentTransformer()]
+        transformers += [
+            MilestonesAndSpecialExamsTransformer(
+                include_special_exams=include_special_exams,
+                include_gated_sections=include_gated_sections
+            ),
+            HiddenContentTransformer()
+        ]
+
+    # TODO: Remove this after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
+    if is_request_from_mobile_app(request):
+        transformers += [HideEmptyTransformer()]
+
     transformers += [
         BlocksAPITransformer(
             block_counts,

lms/djangoapps/course_api/blocks/tests/test_api.py

@@ -3,6 +3,7 @@ Tests for Blocks api.py
 """
 
 from itertools import product
+from mock import patch
 
 import ddt
 from django.test.client import RequestFactory
@@ -16,6 +17,7 @@ from student.tests.factories import UserFactory
 from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls
+from xmodule.modulestore.tests.sample_courses import BlockInfo
 
 
 from ..api import get_blocks
@@ -109,6 +111,55 @@ class TestGetBlocks(SharedModuleStoreTestCase):
             self.assertEqual(block['type'], 'problem')
 
 
+# TODO: Remove this class after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
+@ddt.ddt
+class TestGetBlocksMobileHack(SharedModuleStoreTestCase):
+    """
+    Tests that requests from the mobile app don't receive empty containers.
+    """
+    shard = 4
+
+    @classmethod
+    def setUpClass(cls):
+        super(TestGetBlocksMobileHack, cls).setUpClass()
+        with cls.store.default_store(ModuleStoreEnum.Type.split):
+            cls.course = SampleCourseFactory.create(
+                block_info_tree=[
+                    BlockInfo('empty_chapter', 'chapter', {}, [
+                        BlockInfo('empty_sequential', 'sequential', {}, [
+                            BlockInfo('empty_vertical', 'vertical', {}, []),
+                        ]),
+                    ]),
+                    BlockInfo('full_chapter', 'chapter', {}, [
+                        BlockInfo('full_sequential', 'sequential', {}, [
+                            BlockInfo('full_vertical', 'vertical', {}, [
+                                BlockInfo('html', 'html', {}, []),
+                            ]),
+                        ]),
+                    ])
+                ]
+            )
+
+    def setUp(self):
+        super(TestGetBlocksMobileHack, self).setUp()
+        self.user = UserFactory.create()
+        self.request = RequestFactory().get("/dummy")
+        self.request.user = self.user
+
+    @ddt.data(
+        *product([True, False], ['chapter', 'sequential', 'vertical'])
+    )
+    @ddt.unpack
+    def test_empty_containers(self, is_mobile, container_type):
+        with patch('lms.djangoapps.course_api.blocks.api.is_request_from_mobile_app', return_value=is_mobile):
+            blocks = get_blocks(self.request, self.course.location)
+        full_container_key = self.course.id.make_usage_key(container_type, 'full_{}'.format(container_type))
+        self.assertIn(str(full_container_key), blocks['blocks'])
+        empty_container_key = self.course.id.make_usage_key(container_type, 'empty_{}'.format(container_type))
+        assert_containment = self.assertNotIn if is_mobile else self.assertIn
+        assert_containment(str(empty_container_key), blocks['blocks'])
+
+
 @ddt.ddt
 class TestGetBlocksQueryCountsBase(SharedModuleStoreTestCase):
     """

lms/djangoapps/course_blocks/transformers/hide_empty.py

@@ -0,0 +1,43 @@
+"""
+Hide Empty Transformer implementation.
+"""
+# TODO: Remove this file after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
+from openedx.core.djangoapps.content.block_structure.transformer import (
+    BlockStructureTransformer
+)
+
+
+class HideEmptyTransformer(BlockStructureTransformer):
+    """
+    A transformer that removes any block from the course that could have
+    children but doesn't.
+    """
+    WRITE_VERSION = 1
+    READ_VERSION = 1
+
+    @classmethod
+    def name(cls):
+        """
+        Unique identifier for the transformer's class;
+        same identifier used in setup.py.
+        """
+        return "hide_empty"
+
+    @classmethod
+    def collect(cls, block_structure):
+        """
+        Collects any information that's necessary to execute this
+        transformer's transform method.
+        """
+        block_structure.request_xblock_fields('children', 'has_children')
+
+    def transform(self, usage_info, block_structure):
+        def _filter(block_key):
+            has_children = block_structure.get_xblock_field(block_key, 'has_children')
+            children = block_structure.get_xblock_field(block_key, 'children')
+            return has_children and not any(child in block_structure for child in children)
+
+        for _ in block_structure.post_order_traversal(
+            filter_func=block_structure.create_removal_filter(_filter)
+        ):
+            pass

setup.py

@@ -55,6 +55,7 @@ setup(
             "start_date = lms.djangoapps.course_blocks.transformers.start_date:StartDateTransformer",
             "user_partitions = lms.djangoapps.course_blocks.transformers.user_partitions:UserPartitionTransformer",
             "visibility = lms.djangoapps.course_blocks.transformers.visibility:VisibilityTransformer",
+            "hide_empty = lms.djangoapps.course_blocks.transformers.hide_empty:HideEmptyTransformer",
             "hidden_content = lms.djangoapps.course_blocks.transformers.hidden_content:HiddenContentTransformer",
             "course_blocks_api = lms.djangoapps.course_api.blocks.transformers.blocks_api:BlocksAPITransformer",
             "milestones = lms.djangoapps.course_api.blocks.transformers.milestones:MilestonesAndSpecialExamsTransformer",