Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

get_items API: have qualifiers be a separate parameter instead of assuming kwargs.

Browse Source
This commit is contained in:
Nimisha Asthagiri
2014-08-08 12:38:42 -04:00
parent 775b50a736
commit d2b59cb6e0
22 changed files with 81 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cms/djangoapps/contentstore/features/video.py
View File

@@ -138,7 +138,7 @@ def xml_only_video(step):
course = world.scenario_dict['COURSE']
store = modulestore()
parent_location = store.get_items(course.id, category='vertical')[0].location
parent_location = store.get_items(course.id, qualifiers={'category': 'vertical'})[0].location
youtube_id = 'ABCDEFG'
world.scenario_dict['YOUTUBE_ID'] = youtube_id

2
cms/djangoapps/contentstore/management/commands/check_course.py
View File

@@ -58,7 +58,7 @@ class Command(BaseCommand):
discussion_items = _get_discussion_items(course)
# now query all discussion items via get_items() and compare with the tree-traversal
queried_discussion_items = store.get_items(course_key=course_key, category='discussion',)
queried_discussion_items = store.get_items(course_key=course_key, qualifiers={'category': 'discussion'})
for item in queried_discussion_items:
if item.location not in discussion_items:

22
cms/djangoapps/contentstore/tests/test_contentstore.py
View File

@@ -95,7 +95,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
store.update_item(course, self.user.id)
# just pick one vertical
descriptor = store.get_items(course.id, category='vertical',)
descriptor = store.get_items(course.id, qualifiers={'category': 'vertical'})
resp = self.client.get_html(get_url('container_handler', descriptor[0].location))
self.assertEqual(resp.status_code, 200)
@@ -128,7 +128,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
"""Verifies the editing HTML in all the verticals in the given test course"""
_, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', [test_course_name])
items = self.store.get_items(course_items[0].id, category='vertical')
items = self.store.get_items(course_items[0].id, qualifiers={'category': 'vertical'})
self._check_verticals(items)
def test_edit_unit_toy(self):
@@ -290,7 +290,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
_, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'])
course_key = course_items[0].id
items = self.store.get_items(course_key, category='poll_question')
items = self.store.get_items(course_key, qualifiers={'category': 'poll_question'})
found = len(items) > 0
self.assertTrue(found)
@@ -644,7 +644,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
filesystem = OSFS(root_dir / 'test_export')
self.assertTrue(filesystem.exists(dirname))
items = store.get_items(course_id, category=category_name)
items = store.get_items(course_id, qualifiers={'category': category_name})
for item in items:
filesystem = OSFS(root_dir / ('test_export/' + dirname))
@@ -743,7 +743,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
# create a new video module and add it as a child to a vertical
# this re-creates a bug whereby since the video template doesn't have
# anything in 'data' field, the export was blowing up
verticals = self.store.get_items(course_id, category='vertical')
verticals = self.store.get_items(course_id, qualifiers={'category': 'vertical'})
self.assertGreater(len(verticals), 0)
@@ -769,7 +769,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
import_from_xml(self.store, self.user.id, 'common/test/data/', ['word_cloud'])
course_id = SlashSeparatedCourseKey('HarvardX', 'ER22x', '2013_Spring')
verticals = self.store.get_items(course_id, category='vertical')
verticals = self.store.get_items(course_id, qualifiers={'category': 'vertical'})
self.assertGreater(len(verticals), 0)
@@ -796,7 +796,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'])
course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
verticals = self.store.get_items(course_id, category='vertical')
verticals = self.store.get_items(course_id, qualifiers={'category': 'vertical'})
self.assertGreater(len(verticals), 0)
@@ -917,8 +917,10 @@ class ContentStoreToyCourseTest(ContentStoreTestCase):
items = self.store.get_items(
course_id,
category='sequential',
name='vertical_sequential'
qualifiers={
'category': 'sequential',
'name': 'vertical_sequential',
}
)
self.assertEqual(len(items), 1)
@@ -1401,7 +1403,7 @@ class ContentStoreTest(ContentStoreTestCase):
_, course_items = import_from_xml(self.store, self.user.id, 'common/test/data/', ['toy'])
course = course_items[0]
verticals = self.store.get_items(course.id, category='vertical')
verticals = self.store.get_items(course.id, qualifiers={'category': 'vertical'})
# let's assert on the metadata_inheritance on an existing vertical
for vertical in verticals:

2
cms/djangoapps/contentstore/tests/test_course_settings.py
View File

@@ -413,7 +413,7 @@ class CourseGradingTest(CourseTestCase):
Populate the course, grab a section, get the url for the assignment type access
"""
self.populate_course()
sections = modulestore().get_items(self.course.id, category="sequential")
sections = modulestore().get_items(self.course.id, qualifiers={'category': "sequential"})
# see if test makes sense
self.assertGreater(len(sections), 0, "No sections found")
section = sections[0] # just take the first one

2
cms/djangoapps/contentstore/tests/utils.py
View File

@@ -190,7 +190,7 @@ class CourseTestCase(ModuleStoreTestCase):
"""
items = self.store.get_items(
course_id,
category='vertical',
qualifiers={'category': 'vertical'},
revision=ModuleStoreEnum.RevisionOption.published_only
)
self.check_verticals(items)

2
cms/djangoapps/contentstore/views/course.py
View File

@@ -1135,7 +1135,7 @@ class GroupConfiguration(object):
}
"""
usage_info = {}
descriptors = store.get_items(course.id, category='split_test')
descriptors = store.get_items(course.id, qualifiers={'category': 'split_test'})
for split_test in descriptors:
if split_test.user_partition_id not in usage_info:
usage_info[split_test.user_partition_id] = []

3
common/lib/xmodule/xmodule/modulestore/__init__.py
View File

@@ -202,9 +202,6 @@ class ModuleStoreRead(object):
return True, field
for key, criteria in qualifiers.iteritems():
if callable(criteria):
# skip over any optional fields that are functions
continue
is_set, value = _is_set_on(key)
if not is_set:
return False

19
common/lib/xmodule/xmodule/modulestore/mixed.py
View File

@@ -26,7 +26,8 @@ log = logging.getLogger(__name__)
def strip_key(func):
"""
A decorator for stripping version and branch information from return values that are, or contain, locations.
A decorator for stripping version and branch information from return values that are, or contain, UsageKeys or
CourseKeys.
Additionally, the decorated function is called with an optional 'field_decorator' parameter that can be used
to strip any location(-containing) fields, which are not directly returned by the function.
@@ -222,14 +223,14 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
and rules as kwargs below
content (dict): fields to look for which have content scope. Follows same syntax and
rules as kwargs below.
additional kwargs (key=value): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
Substring matching pass a regex object.
For some modulestores, ``name`` is another commonly provided key (Location based stores)
For some modulestores,
you can search by ``edited_by``, ``edited_on`` providing either a datetime for == (probably
useless) or a function accepting one arg to do inequality
qualifiers (dict): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
Substring matching pass a regex object.
For some modulestores, ``name`` is another commonly provided key (Location based stores)
For some modulestores,
you can search by ``edited_by``, ``edited_on`` providing either a datetime for == (probably
useless) or a function accepting one arg to do inequality
"""
if not isinstance(course_key, CourseKey):
raise Exception("Must pass in a course_key when calling get_items()")

28
common/lib/xmodule/xmodule/modulestore/mongo/base.py
View File

@@ -846,7 +846,15 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
for key in ('tag', 'org', 'course', 'category', 'name', 'revision')
])
def get_items(self, course_id, settings=None, content=None, key_revision=MongoRevisionKey.published, **kwargs):
def get_items(
self,
course_id,
settings=None,
content=None,
key_revision=MongoRevisionKey.published,
qualifiers=None,
**kwargs
):
"""
Returns:
list of XModuleDescriptor instances for the matching items within the course with
@@ -861,15 +869,15 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
Args:
course_id (CourseKey): the course identifier
settings (dict): fields to look for which have settings scope. Follows same syntax
and rules as kwargs below
and rules as qualifiers below
content (dict): fields to look for which have content scope. Follows same syntax and
rules as kwargs below.
rules as qualifiers below.
key_revision (str): the revision of the items you're looking for.
MongoRevisionKey.draft - only returns drafts
MongoRevisionKey.published (equates to None) - only returns published
If you want one of each matching xblock but preferring draft to published, call this same method
on the draft modulestore with ModuleStoreEnum.RevisionOption.draft_preferred.
kwargs (key=value): what to look for within the course.
qualifiers (dict): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
Substring matching pass a regex object.
@@ -877,21 +885,19 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
This modulestore does not allow searching dates by comparison or edited_by, previous_version,
update_version info.
"""
qualifiers = qualifiers.copy() if qualifiers else {} # copy the qualifiers (destructively manipulated here)
query = self._course_key_to_son(course_id)
query['_id.revision'] = key_revision
for field in ['category', 'name']:
if field in kwargs:
query['_id.' + field] = kwargs.pop(field)
if field in qualifiers:
query['_id.' + field] = qualifiers.pop(field)
for key, value in (settings or {}).iteritems():
query['metadata.' + key] = value
for key, value in (content or {}).iteritems():
query['definition.data.' + key] = value
if 'children' in kwargs:
query['definition.children'] = kwargs.pop('children')
# remove any callable kwargs for qualifiers
qualifiers = {key: val for key, val in kwargs.iteritems() if not callable(val)}
if 'children' in qualifiers:
query['definition.children'] = qualifiers.pop('children')
query.update(qualifiers)
items = self.collection.find(

5
common/lib/xmodule/xmodule/modulestore/mongo/draft.py
View File

@@ -331,11 +331,6 @@ class DraftModuleStore(MongoModuleStore):
returns only Published items
if the branch setting is ModuleStoreEnum.Branch.draft_preferred,
returns either Draft or Published, preferring Draft items.
kwargs (key=value): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
Substring matching pass a regex object.
``name`` is another commonly provided key (Location based stores)
"""
def base_get_items(key_revision):
return super(DraftModuleStore, self).get_items(course_key, key_revision=key_revision, **kwargs)

19
common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
View File

@@ -451,7 +451,7 @@ class SplitMongoModuleStore(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, **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
@@ -462,10 +462,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
Args:
course_locator (CourseLocator): the course identifier
settings (dict): fields to look for which have settings scope. Follows same syntax
and rules as kwargs below
and rules as qualifiers below
content (dict): fields to look for which have content scope. Follows same syntax and
rules as kwargs below.
kwargs (key=value): what to look for within the course.
rules as qualifiers below.
qualifiers (dict): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
For substring matching pass a regex object.
@@ -474,6 +474,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
"""
course = self._lookup_course(course_locator)
items = []
qualifiers = qualifiers.copy() if qualifiers else {} # copy the qualifiers (destructively manipulated here)
def _block_matches_all(block_json):
"""
@@ -481,7 +482,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
"""
# do the checks which don't require loading any additional data
if (
self._block_matches(block_json, kwargs) and
self._block_matches(block_json, qualifiers) and
self._block_matches(block_json.get('fields', {}), settings)
):
if content:
@@ -492,17 +493,17 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
if settings is None:
settings = {}
if 'name' in kwargs:
if 'name' in qualifiers:
# odd case where we don't search just confirm
block_id = kwargs.pop('name')
block_id = qualifiers.pop('name')
block = course['structure']['blocks'].get(block_id)
if _block_matches_all(block):
return self._load_items(course, [block_id], lazy=True, **kwargs)
else:
return []
# don't expect caller to know that children are in fields
if 'children' in kwargs:
settings['children'] = kwargs.pop('children')
if 'children' in qualifiers:
settings['children'] = qualifiers.pop('children')
for block_id, value in course['structure']['blocks'].iteritems():
if _block_matches_all(value):
items.append(block_id)

9
common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py
View File

@@ -171,18 +171,13 @@ class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleS
usage_key = self._map_revision_to_branch(usage_key, revision=revision)
return super(DraftVersioningModuleStore, self).get_item(usage_key, depth=depth, **kwargs)
def get_items(self, course_locator, settings=None, content=None, revision=None, **kwargs):
def get_items(self, course_locator, revision=None, **kwargs):
"""
Returns a list of XModuleDescriptor instances for the matching items within the course with
the given course_locator.
"""
course_locator = self._map_revision_to_branch(course_locator, revision=revision)
return super(DraftVersioningModuleStore, self).get_items(
course_locator,
settings=settings,
content=content,
**kwargs
)
return super(DraftVersioningModuleStore, self).get_items(course_locator, **kwargs)
def get_parent_location(self, location, revision=None, **kwargs):
'''

4
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View File

@@ -326,7 +326,7 @@ class TestMixedModuleStore(unittest.TestCase):
course_locn = self.course_locations[self.XML_COURSEID1]
# NOTE: use get_course if you just want the course. get_items is expensive
modules = self.store.get_items(course_locn.course_key, category='course')
modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'course'})
self.assertEqual(len(modules), 1)
self.assertEqual(modules[0].location, course_locn)
@@ -334,7 +334,7 @@ class TestMixedModuleStore(unittest.TestCase):
course_locn = self.course_locations[self.MONGO_COURSEID]
with check_mongo_calls(mongo_store, max_find, max_send):
# NOTE: use get_course if you just want the course. get_items is expensive
modules = self.store.get_items(course_locn.course_key, category='problem')
modules = self.store.get_items(course_locn.course_key, qualifiers={'category': 'problem'})
self.assertEqual(len(modules), 6)
# verify that an error is raised when the revision is not valid

2
common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py
View File

@@ -151,7 +151,7 @@ class TestMigration(SplitWMongoCourseBoostrapper):
# grab the detached items to compare they should be in both published and draft
for category in ['conditional', 'about', 'course_info', 'static_tab']:
for conditional in presplit.get_items(self.old_course_key, category=category):
for conditional in presplit.get_items(self.old_course_key, qualifiers={'category': category}):
locator = new_course_key.make_usage_key(category, conditional.location.block_id)
self.compare_dags(presplit, conditional, self.split_mongo.get_item(locator), published)

12
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
View File

@@ -895,18 +895,18 @@ class SplitModuleItemTests(SplitModuleTest):
self.assertEqual(len(matches), 6)
matches = modulestore().get_items(locator)
self.assertEqual(len(matches), 6)
matches = modulestore().get_items(locator, category='chapter')
matches = modulestore().get_items(locator, qualifiers={'category': 'chapter'})
self.assertEqual(len(matches), 3)
matches = modulestore().get_items(locator, category='garbage')
matches = modulestore().get_items(locator, qualifiers={'category': 'garbage'})
self.assertEqual(len(matches), 0)
matches = modulestore().get_items(
locator,
category='chapter',
qualifiers={'category': 'chapter'},
settings={'display_name': re.compile(r'Hera')},
)
self.assertEqual(len(matches), 2)
matches = modulestore().get_items(locator, children='chapter2')
matches = modulestore().get_items(locator, qualifiers={'children': 'chapter2'})
self.assertEqual(len(matches), 1)
self.assertEqual(matches[0].location.block_id, 'head12345')
@@ -1324,7 +1324,7 @@ class TestItemCrud(SplitModuleTest):
reusable_location = course.id.version_agnostic().for_branch(BRANCH_NAME_DRAFT)
# delete a leaf
problems = modulestore().get_items(reusable_location, category='problem')
problems = modulestore().get_items(reusable_location, qualifiers={'category': 'problem'})
locn_to_del = problems[0].location
new_course_loc = modulestore().delete_item(locn_to_del, self.user_id)
deleted = locn_to_del.version_agnostic()
@@ -1336,7 +1336,7 @@ class TestItemCrud(SplitModuleTest):
self.assertNotEqual(new_course_loc.version_guid, course.location.version_guid)
# delete a subtree
nodes = modulestore().get_items(reusable_location, category='chapter')
nodes = modulestore().get_items(reusable_location, qualifiers={'category': 'chapter'})
new_course_loc = modulestore().delete_item(nodes[0].location, self.user_id)
# check subtree

15
common/lib/xmodule/xmodule/modulestore/xml.py
View File

@@ -717,7 +717,7 @@ class XMLModuleStore(ModuleStoreReadBase):
except KeyError:
raise ItemNotFoundError(usage_key)
def get_items(self, course_id, settings=None, content=None, revision=None, **kwargs):
def get_items(self, course_id, settings=None, content=None, revision=None, qualifiers=None, **kwargs):
"""
Returns:
list of XModuleDescriptor instances for the matching items within the course with
@@ -729,10 +729,10 @@ class XMLModuleStore(ModuleStoreReadBase):
Args:
course_id (CourseKey): the course identifier
settings (dict): fields to look for which have settings scope. Follows same syntax
and rules as kwargs below
and rules as qualifiers below
content (dict): fields to look for which have content scope. Follows same syntax and
rules as kwargs below.
kwargs (key=value): what to look for within the course.
rules as qualifiers below.
qualifiers (dict): what to look for within the course.
Common qualifiers are ``category`` or any field name. if the target field is a list,
then it searches for the given value in the list not list equivalence.
Substring matching pass a regex object.
@@ -747,8 +747,9 @@ class XMLModuleStore(ModuleStoreReadBase):
items = []
category = kwargs.pop('category', None)
name = kwargs.pop('name', None)
qualifiers = qualifiers.copy() if qualifiers else {} # copy the qualifiers (destructively manipulated here)
category = qualifiers.pop('category', None)
name = qualifiers.pop('name', None)
def _block_matches_all(mod_loc, module):
if category and mod_loc.category != category:
@@ -757,7 +758,7 @@ class XMLModuleStore(ModuleStoreReadBase):
return False
return all(
self._block_matches(module, fields or {})
for fields in [settings, content, kwargs]
for fields in [settings, content, qualifiers]
)
for mod_loc, module in self.modules[course_id].iteritems():

4
common/lib/xmodule/xmodule/modulestore/xml_exporter.py
View File

@@ -106,7 +106,7 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir):
# and index here since the XML modulestore cannot load draft modules
draft_verticals = modulestore.get_items(
course_key,
category='vertical',
qualifiers={'category': 'vertical'},
revision=ModuleStoreEnum.RevisionOption.draft_only
)
if len(draft_verticals) > 0:
@@ -144,7 +144,7 @@ def _export_field_content(xblock_item, item_dir):
def export_extra_content(export_fs, modulestore, course_key, category_type, dirname, file_suffix=''):
items = modulestore.get_items(course_key, category=category_type)
items = modulestore.get_items(course_key, qualifiers={'category': category_type})
if len(items) > 0:
item_dir = export_fs.makeopendir(dirname)

2
lms/djangoapps/courseware/tests/test_draft_modulestore.py
View File

@@ -13,7 +13,7 @@ class TestDraftModuleStore(TestCase):
store = modulestore()
# fix was to allow get_items() to take the course_id parameter
store.get_items(SlashSeparatedCourseKey('a', 'b', 'c'), category='vertical')
store.get_items(SlashSeparatedCourseKey('a', 'b', 'c'), qualifiers={'category': 'vertical'})
# test success is just getting through the above statement.
# The bug was that 'course_id' argument was

2
lms/djangoapps/courseware/tests/tests.py
View File

@@ -163,7 +163,7 @@ class TestDraftModuleStore(ModuleStoreTestCase):
store = modulestore()
# fix was to allow get_items() to take the course_id parameter
store.get_items(SlashSeparatedCourseKey('abc', 'def', 'ghi'), category='vertical')
store.get_items(SlashSeparatedCourseKey('abc', 'def', 'ghi'), qualifiers={'category': 'vertical'})
# test success is just getting through the above statement.
# The bug was that 'course_id' argument was

4
lms/djangoapps/courseware/views.py
View File

@@ -463,7 +463,7 @@ def jump_to_id(request, course_id, module_id):
passed in. This assumes that id is unique within the course_id namespace
"""
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
items = modulestore().get_items(course_key, name=module_id)
items = modulestore().get_items(course_key, qualifiers={'name': module_id})
if len(items) == 0:
raise Http404(
@@ -937,7 +937,7 @@ def get_course_lti_endpoints(request, course_id):
anonymous_user = AnonymousUser()
anonymous_user.known = False # make these "noauth" requests like module_render.handle_xblock_callback_noauth
lti_descriptors = modulestore().get_items(course.id, category='lti')
lti_descriptors = modulestore().get_items(course.id, qualifiers={'category': 'lti'})
lti_noauth_modules = [
get_module_for_descriptor(

2
lms/djangoapps/django_comment_client/utils.py
View File

@@ -57,7 +57,7 @@ def has_forum_access(uname, course_id, rolename):
def _get_discussion_modules(course):
all_modules = modulestore().get_items(course.id, category='discussion')
all_modules = modulestore().get_items(course.id, qualifiers={'category': 'discussion'})
def has_required_keys(module):
for key in ('discussion_id', 'discussion_category', 'discussion_target'):

2
lms/djangoapps/open_ended_grading/views.py
View File

@@ -92,7 +92,7 @@ def find_peer_grading_module(course):
problem_url = ""
# Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs.
items = modulestore().get_items(course.id, category='peergrading')
items = modulestore().get_items(course.id, qualifiers={'category': 'peergrading'})
# See if any of the modules are centralized modules (ie display info from multiple problems)
items = [i for i in items if not getattr(i, "use_for_single_location", True)]
# Loop through all potential peer grading modules, and find the first one that has a path to it.