From 92f4d41facf1cea6b663a22c0d56ad7faaff2b62 Mon Sep 17 00:00:00 2001 From: Ben Patterson Date: Mon, 21 Dec 2015 14:33:32 -0500 Subject: [PATCH] Revert "Append block item only if it has path to root" --- .../xmodule/modulestore/split_mongo/split.py | 15 +---- .../tests/test_mixed_modulestore.py | 67 ------------------- 2 files changed, 2 insertions(+), 80 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 933fd4d7bb..e59e52666b 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 84387c3998..569f606fe6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/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