From d2b59cb6e086428037822294c242017c64aba7b5 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 8 Aug 2014 12:38:42 -0400 Subject: [PATCH] get_items API: have qualifiers be a separate parameter instead of assuming kwargs. --- cms/djangoapps/contentstore/features/video.py | 2 +- .../management/commands/check_course.py | 2 +- .../contentstore/tests/test_contentstore.py | 22 ++++++++------- .../tests/test_course_settings.py | 2 +- cms/djangoapps/contentstore/tests/utils.py | 2 +- cms/djangoapps/contentstore/views/course.py | 2 +- .../xmodule/xmodule/modulestore/__init__.py | 3 -- .../lib/xmodule/xmodule/modulestore/mixed.py | 19 +++++++------ .../xmodule/xmodule/modulestore/mongo/base.py | 28 +++++++++++-------- .../xmodule/modulestore/mongo/draft.py | 5 ---- .../xmodule/modulestore/split_mongo/split.py | 19 +++++++------ .../modulestore/split_mongo/split_draft.py | 9 ++---- .../tests/test_mixed_modulestore.py | 4 +-- .../modulestore/tests/test_split_migrator.py | 2 +- .../tests/test_split_modulestore.py | 12 ++++---- common/lib/xmodule/xmodule/modulestore/xml.py | 15 +++++----- .../xmodule/modulestore/xml_exporter.py | 4 +-- .../tests/test_draft_modulestore.py | 2 +- lms/djangoapps/courseware/tests/tests.py | 2 +- lms/djangoapps/courseware/views.py | 4 +-- lms/djangoapps/django_comment_client/utils.py | 2 +- lms/djangoapps/open_ended_grading/views.py | 2 +- 22 files changed, 81 insertions(+), 83 deletions(-) diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index ebe94f4a79..39ab626638 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -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 diff --git a/cms/djangoapps/contentstore/management/commands/check_course.py b/cms/djangoapps/contentstore/management/commands/check_course.py index 45bbcad78a..874a2b9a4c 100644 --- a/cms/djangoapps/contentstore/management/commands/check_course.py +++ b/cms/djangoapps/contentstore/management/commands/check_course.py @@ -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: diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index c8fac4fed9..7fea7df3e2 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -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: diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index 8192b5fb70..9e312b4998 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -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 diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index fcfa7ef979..20b21065bf 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -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) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 9373f556e6..3279cf1b8b 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -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] = [] diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index e0b1be966e..3115d2f4c7 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 9bf8b8cc0e..645d7b3b40 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -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()") diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index de740aab4e..b508056099 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -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( diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index e9cb77e977..bafb2cbd07 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -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) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 8f461deeec..3d067fb66e 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -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) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 0dc859a796..1759c547c1 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -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): ''' 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 521615e0f9..3dc1c64b4b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index c92e817533..835375ae7e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -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) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 8e64247c80..466251f418 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 72c9d727b5..b329fdc32a 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -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(): diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 60cb5f6700..7d5342a3b3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -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) diff --git a/lms/djangoapps/courseware/tests/test_draft_modulestore.py b/lms/djangoapps/courseware/tests/test_draft_modulestore.py index d3c6163c55..991c18cc13 100644 --- a/lms/djangoapps/courseware/tests/test_draft_modulestore.py +++ b/lms/djangoapps/courseware/tests/test_draft_modulestore.py @@ -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 diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index e3e13ef9a9..dbb17f061f 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -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 diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 8c23a0512b..70d6dc0d9c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -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( diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 1c912de7c0..8ec578b956 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -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'): diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index ec98c5e1c1..c24c0540a6 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -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.