Skip to content
Snippets Groups Projects
Commit 92f4d41f authored by Ben Patterson's avatar Ben Patterson
Browse files

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

parent ddc6b5ac
No related merge requests found
......@@ -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)
......
......@@ -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
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment