Merge pull request #11044 from edx/revert-10994-mushtaq/improve_get_item

Revert "Append block item only if it has path to root"

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -1059,7 +1059,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                 log.debug("Found more than one item for '{}'".format(usage_key))
             return items[0]
 
-    def get_items(self, course_locator, settings=None, content=None, qualifiers=None, include_orphans=True, **kwargs):
+    def get_items(self, course_locator, settings=None, content=None, qualifiers=None, **kwargs):
         """
         Returns:
             list of XModuleDescriptor instances for the matching items within the course with
@@ -1079,11 +1079,6 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                 For substring matching pass a regex object.
                 For split,
                 you can search by ``edited_by``, ``edited_on`` providing a function testing limits.
-            include_orphans (boolean): Returns all items in a course, including orphans if present.
-                True - This would return all items irrespective of course in tree checking. It may fetch orphans
-                if present in the course.
-                False - if we want only those items which are in the course tree. This would ensure no orphans are
-                fetched.
         """
         if not isinstance(course_locator, CourseLocator) or course_locator.deprecated:
             # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
@@ -1123,18 +1118,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         if 'category' in qualifiers:
             qualifiers['block_type'] = qualifiers.pop('category')
 
-        detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")]
-
         # don't expect caller to know that children are in fields
         if 'children' in qualifiers:
             settings['children'] = qualifiers.pop('children')
         for block_id, value in course.structure['blocks'].iteritems():
             if _block_matches_all(value):
-                if not include_orphans:
-                    if self.has_path_to_root(block_id, course) or block_id.type in detached_categories:
-                        items.append(block_id)
-                else:
-                    items.append(block_id)
+                items.append(block_id)
 
         if len(items) > 0:
             return self._load_items(course, items, depth=0, **kwargs)

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

@@ -22,8 +22,6 @@ from pytz import UTC
 from shutil import rmtree
 from tempfile import mkdtemp
 
-from xblock.core import XBlock
-
 from xmodule.x_module import XModuleMixin
 from xmodule.modulestore.edit_info import EditInfoMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
@@ -438,71 +436,6 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
                 revision=ModuleStoreEnum.RevisionOption.draft_preferred
             )
 
-    @ddt.data((ModuleStoreEnum.Type.split, 2, False), (ModuleStoreEnum.Type.mongo, 3, True))
-    @ddt.unpack
-    def test_get_items_include_orphans(self, default_ms, expected_items_in_tree, orphan_in_items):
-        """
-        Test `include_orphans` option helps in returning only those items which are present in course tree.
-        It tests that orphans are not fetched when calling `get_item` with `include_orphans`.
-
-        Params:
-            expected_items_in_tree:
-                Number of items that will be returned after `get_items` would be called with `include_orphans`.
-                In split, it would not get orphan items.
-                In mongo, it would still get orphan items because `include_orphans` would not have any impact on mongo
-                    modulestore which will return same number of items as called without `include_orphans` kwarg.
-
-            orphan_in_items:
-                When `get_items` is called with `include_orphans` kwarg, then check if an orphan is returned or not.
-                False when called in split modulestore because in split get_items is expected to not retrieve orphans
-                    now because of `include_orphans`.
-                True when called in mongo modulstore because `include_orphans` does not have any effect on mongo.
-        """
-        self.initdb(default_ms)
-
-        test_course = self.store.create_course('testx', 'GreekHero', 'test_run', self.user_id)
-        course_key = test_course.id
-
-        # get detached category list
-        detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")]
-
-        items = self.store.get_items(course_key)
-        # Check items found are either course or about type
-        self.assertTrue(set(['course', 'about']).issubset(set([item.location.block_type for item in items])))
-        # Assert that about is a detached category found in get_items
-        self.assertIn(
-            [item.location.block_type for item in items if item.location.block_type == 'about'][0],
-            detached_categories
-        )
-        self.assertEqual(len(items), 2)
-
-        # Check that orphans are not found
-        orphans = self.store.get_orphans(course_key)
-        self.assertEqual(len(orphans), 0)
-
-        # Add an orphan to test course
-        orphan = course_key.make_usage_key('chapter', 'OrphanChapter')
-        self.store.create_item(self.user_id, orphan.course_key, orphan.block_type, block_id=orphan.block_id)
-
-        # Check that now an orphan is found
-        orphans = self.store.get_orphans(course_key)
-        self.assertIn(orphan, orphans)
-        self.assertEqual(len(orphans), 1)
-
-        # Check now `get_items` retrieves an extra item added above which is an orphan.
-        items = self.store.get_items(course_key)
-        self.assertIn(orphan, [item.location for item in items])
-        self.assertEqual(len(items), 3)
-
-        # Check now `get_items` with `include_orphans` kwarg does not retrieves an orphan block.
-        items_in_tree = self.store.get_items(course_key, include_orphans=False)
-
-        # Check that course and about blocks are found in get_items
-        self.assertTrue(set(['course', 'about']).issubset(set([item.location.block_type for item in items_in_tree])))
-        # Check orphan is found or not - this is based on mongo/split modulestore. It should be found in mongo.
-        self.assertEqual(orphan in [item.location for item in items_in_tree], orphan_in_items)
-        self.assertEqual(len(items_in_tree), expected_items_in_tree)
-
     # draft: get draft, get ancestors up to course (2-6), compute inheritance
     #    sends: update problem and then each ancestor up to course (edit info)
     # split: active_versions, definitions (calculator field), structures