diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 3970a8c1e8..08cc5cb29b 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -9,8 +9,11 @@ from mock import patch, Mock import ddt from django.test import RequestFactory +from xmodule.course_module import CourseSummary -from contentstore.views.course import _accessible_courses_list, _accessible_courses_list_from_groups, AccessListFallback +from contentstore.views.course import (_accessible_courses_list, _accessible_courses_list_from_groups, + AccessListFallback, get_courses_accessible_to_user, + _staff_accessible_course_list) from contentstore.utils import delete_course_and_groups from contentstore.tests.utils import AjaxEnabledTestClient from student.tests.factories import UserFactory @@ -19,7 +22,6 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from xmodule.modulestore import ModuleStoreEnum from opaque_keys.edx.locations import CourseLocator -from xmodule.modulestore.django import modulestore from xmodule.error_module import ErrorDescriptor from course_action_state.models import CourseRerunState @@ -46,7 +48,7 @@ class TestCourseListing(ModuleStoreTestCase): self.client = AjaxEnabledTestClient() self.client.login(username=self.user.username, password='test') - def _create_course_with_access_groups(self, course_location, user=None): + def _create_course_with_access_groups(self, course_location, user=None, store=ModuleStoreEnum.Type.split): """ Create dummy course with 'CourseFactory' and role (instructor/staff) groups """ @@ -54,7 +56,7 @@ class TestCourseListing(ModuleStoreTestCase): org=course_location.org, number=course_location.course, run=course_location.run, - default_store=ModuleStoreEnum.Type.mongo + default_store=store ) if user is not None: @@ -87,54 +89,101 @@ class TestCourseListing(ModuleStoreTestCase): # check both course lists have same courses self.assertEqual(courses_list, courses_list_by_groups) - def test_errored_course_global_staff(self): + @ddt.data( + (ModuleStoreEnum.Type.split, 'xmodule.modulestore.split_mongo.split_mongo_kvs.SplitMongoKVS'), + (ModuleStoreEnum.Type.mongo, 'xmodule.modulestore.mongo.base.MongoKeyValueStore') + ) + @ddt.unpack + def test_errored_course_global_staff(self, store, path_to_patch): """ Test the course list for global staff when get_course returns an ErrorDescriptor """ GlobalStaff().add_users(self.user) - course_key = self.store.make_course_key('Org1', 'Course1', 'Run1') - self._create_course_with_access_groups(course_key, self.user) + with self.store.default_store(store): + course_key = self.store.make_course_key('Org1', 'Course1', 'Run1') + self._create_course_with_access_groups(course_key, self.user, store=store) - with patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', Mock(side_effect=Exception)): - self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor) + with patch(path_to_patch, Mock(side_effect=Exception)): + self.assertIsInstance(self.store.get_course(course_key), ErrorDescriptor) - # get courses through iterating all courses - courses_list, __ = _accessible_courses_list(self.request) - self.assertEqual(courses_list, []) + # get courses through iterating all courses + courses_list, __ = _accessible_courses_list(self.request) + self.assertEqual(courses_list, []) - # get courses by reversing group name formats - courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) - self.assertEqual(courses_list_by_groups, []) + # get courses by reversing group name formats + courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) + self.assertEqual(courses_list_by_groups, []) - def test_errored_course_regular_access(self): + @ddt.data( + (ModuleStoreEnum.Type.split, 5), + (ModuleStoreEnum.Type.mongo, 3) + ) + @ddt.unpack + def test_staff_course_listing(self, default_store, mongo_calls): + """ + Create courses and verify they take certain amount of mongo calls to call get_courses_accessible_to_user. + Also verify that fetch accessible courses list for staff user returns CourseSummary instances. + """ + + # Assign & verify staff role to the user + GlobalStaff().add_users(self.user) + self.assertTrue(GlobalStaff().has_user(self.user)) + + with self.store.default_store(default_store): + # Create few courses + for num in xrange(TOTAL_COURSES_COUNT): + course_location = self.store.make_course_key('Org', 'CreatedCourse' + str(num), 'Run') + self._create_course_with_access_groups(course_location, self.user, default_store) + + # Fetch accessible courses list & verify their count + courses_list_by_staff, __ = get_courses_accessible_to_user(self.request) + self.assertEqual(len(courses_list_by_staff), TOTAL_COURSES_COUNT) + + # Verify fetched accessible courses list is a list of CourseSummery instances + self.assertTrue(all(isinstance(course, CourseSummary) for course in courses_list_by_staff)) + + # Now count the db queries for staff + with check_mongo_calls(mongo_calls): + _staff_accessible_course_list(self.request) + + @ddt.data( + (ModuleStoreEnum.Type.split, 'xmodule.modulestore.split_mongo.split_mongo_kvs.SplitMongoKVS'), + (ModuleStoreEnum.Type.mongo, 'xmodule.modulestore.mongo.base.MongoKeyValueStore') + ) + @ddt.unpack + def test_errored_course_regular_access(self, store, path_to_patch): """ Test the course list for regular staff when get_course returns an ErrorDescriptor """ GlobalStaff().remove_users(self.user) - CourseStaffRole(self.store.make_course_key('Non', 'Existent', 'Course')).add_users(self.user) - course_key = self.store.make_course_key('Org1', 'Course1', 'Run1') - self._create_course_with_access_groups(course_key, self.user) + with self.store.default_store(store): + CourseStaffRole(self.store.make_course_key('Non', 'Existent', 'Course')).add_users(self.user) - with patch('xmodule.modulestore.mongo.base.MongoKeyValueStore', Mock(side_effect=Exception)): - self.assertIsInstance(modulestore().get_course(course_key), ErrorDescriptor) + course_key = self.store.make_course_key('Org1', 'Course1', 'Run1') + self._create_course_with_access_groups(course_key, self.user, store) - # get courses through iterating all courses - courses_list, __ = _accessible_courses_list(self.request) - self.assertEqual(courses_list, []) + with patch(path_to_patch, Mock(side_effect=Exception)): + self.assertIsInstance(self.store.get_course(course_key), ErrorDescriptor) - # get courses by reversing group name formats - courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) - self.assertEqual(courses_list_by_groups, []) - self.assertEqual(courses_list, courses_list_by_groups) + # get courses through iterating all courses + courses_list, __ = _accessible_courses_list(self.request) + self.assertEqual(courses_list, []) - def test_get_course_list_with_invalid_course_location(self): + # get courses by reversing group name formats + courses_list_by_groups, __ = _accessible_courses_list_from_groups(self.request) + self.assertEqual(courses_list_by_groups, []) + self.assertEqual(courses_list, courses_list_by_groups) + + @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) + def test_get_course_list_with_invalid_course_location(self, store): """ Test getting courses with invalid course location (course deleted from modulestore). """ - course_key = self.store.make_course_key('Org', 'Course', 'Run') - self._create_course_with_access_groups(course_key, self.user) + with self.store.default_store(store): + course_key = self.store.make_course_key('Org', 'Course', 'Run') + self._create_course_with_access_groups(course_key, self.user, store) # get courses through iterating all courses courses_list, __ = _accessible_courses_list(self.request) @@ -155,7 +204,12 @@ class TestCourseListing(ModuleStoreTestCase): courses_list, __ = _accessible_courses_list(self.request) self.assertEqual(len(courses_list), 0) - def test_course_listing_performance(self): + @ddt.data( + (ModuleStoreEnum.Type.split, 150, 505), + (ModuleStoreEnum.Type.mongo, USER_COURSES_COUNT, 3) + ) + @ddt.unpack + def test_course_listing_performance(self, store, courses_list_from_group_calls, courses_list_calls): """ Create large number of courses and give access of some of these courses to the user and compare the time to fetch accessible courses for the user through traversing all courses and @@ -165,15 +219,16 @@ class TestCourseListing(ModuleStoreTestCase): user_course_ids = random.sample(range(TOTAL_COURSES_COUNT), USER_COURSES_COUNT) # create courses and assign those to the user which have their number in user_course_ids - for number in range(TOTAL_COURSES_COUNT): - org = 'Org{0}'.format(number) - course = 'Course{0}'.format(number) - run = 'Run{0}'.format(number) - course_location = self.store.make_course_key(org, course, run) - if number in user_course_ids: - self._create_course_with_access_groups(course_location, self.user) - else: - self._create_course_with_access_groups(course_location) + with self.store.default_store(store): + for number in range(TOTAL_COURSES_COUNT): + org = 'Org{0}'.format(number) + course = 'Course{0}'.format(number) + run = 'Run{0}'.format(number) + course_location = self.store.make_course_key(org, course, run) + if number in user_course_ids: + self._create_course_with_access_groups(course_location, self.user, store=store) + else: + self._create_course_with_access_groups(course_location, store=store) # time the get courses by iterating through all courses with Timer() as iteration_over_courses_time_1: @@ -201,29 +256,29 @@ class TestCourseListing(ModuleStoreTestCase): self.assertGreaterEqual(iteration_over_courses_time_2.elapsed, iteration_over_groups_time_2.elapsed) # Now count the db queries - with check_mongo_calls(USER_COURSES_COUNT): + with check_mongo_calls(courses_list_from_group_calls): _accessible_courses_list_from_groups(self.request) + with check_mongo_calls(courses_list_calls): + _accessible_courses_list(self.request) # Calls: # 1) query old mongo # 2) get_more on old mongo # 3) query split (but no courses so no fetching of data) - with check_mongo_calls(3): - _accessible_courses_list(self.request) - def test_course_listing_errored_deleted_courses(self): + @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) + def test_course_listing_errored_deleted_courses(self, store): """ Create good courses, courses that won't load, and deleted courses which still have roles. Test course listing. """ - store = modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo) + with self.store.default_store(store): + course_location = self.store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') + self._create_course_with_access_groups(course_location, self.user, store) - course_location = self.store.make_course_key('testOrg', 'testCourse', 'RunBabyRun') - self._create_course_with_access_groups(course_location, self.user) - - course_location = self.store.make_course_key('testOrg', 'doomedCourse', 'RunBabyRun') - self._create_course_with_access_groups(course_location, self.user) - store.delete_course(course_location, self.user.id) + course_location = self.store.make_course_key('testOrg', 'doomedCourse', 'RunBabyRun') + self._create_course_with_access_groups(course_location, self.user, store) + self.store.delete_course(course_location, self.user.id) # pylint: disable=no-member courses_list, __ = _accessible_courses_list_from_groups(self.request) self.assertEqual(len(courses_list), 1, courses_list) @@ -241,7 +296,7 @@ class TestCourseListing(ModuleStoreTestCase): run=org_course_one.run ) - org_course_two = self.store.make_course_key('AwesomeOrg', 'Course2', 'RunRunRun') + org_course_two = self.store.make_course_key('AwesomeOrg', 'Course2', 'RunBabyRun') CourseFactory.create( org=org_course_two.org, number=org_course_two.course, diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 3e82680971..929da3e027 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -17,6 +17,7 @@ import django.utils from django.utils.translation import ugettext as _ from django.views.decorators.http import require_http_methods, require_GET from django.views.decorators.csrf import ensure_csrf_cookie + from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import Location @@ -345,6 +346,39 @@ def _course_outline_json(request, course_module): ) +def get_in_process_course_actions(request): + """ + Get all in-process course actions + """ + return [ + course for course in + CourseRerunState.objects.find_all( + exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True + ) + if has_studio_read_access(request.user, course.course_key) + ] + + +def _staff_accessible_course_list(request): + """ + List all courses available to the logged in user by iterating through all the courses + """ + def course_filter(course_summary): + """ + Filter out unusable and inaccessible courses + """ + # pylint: disable=fixme + # TODO remove this condition when templates purged from db + if course_summary.location.course == 'templates': + return False + + return has_studio_read_access(request.user, course_summary.id) + + courses_summary = filter(course_filter, modulestore().get_course_summaries()) + in_process_course_actions = get_in_process_course_actions(request) + return courses_summary, in_process_course_actions + + def _accessible_courses_list(request): """ List all courses available to the logged in user by iterating through all the courses @@ -364,13 +398,8 @@ def _accessible_courses_list(request): return has_studio_read_access(request.user, course.id) courses = filter(course_filter, modulestore().get_courses()) - in_process_course_actions = [ - course for course in - CourseRerunState.objects.find_all( - exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True - ) - if has_studio_read_access(request.user, course.course_key) - ] + + in_process_course_actions = get_in_process_course_actions(request) return courses, in_process_course_actions @@ -593,7 +622,7 @@ def get_courses_accessible_to_user(request): """ if GlobalStaff().has_user(request.user): # user has global access so no need to get courses from django groups - courses, in_process_course_actions = _accessible_courses_list(request) + courses, in_process_course_actions = _staff_accessible_course_list(request) else: try: courses, in_process_course_actions = _accessible_courses_list_from_groups(request) @@ -626,9 +655,9 @@ def _remove_in_process_courses(courses, in_process_course_actions): in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions] courses = [ - format_course_for_view(c) - for c in courses - if not isinstance(c, ErrorDescriptor) and (c.id not in in_process_action_course_keys) + format_course_for_view(course) + for course in courses + if not isinstance(course, ErrorDescriptor) and (course.id not in in_process_action_course_keys) ] return courses diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index c91ed60fb1..c4a187f670 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -1367,3 +1367,56 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin): bool: False if the course has already started, True otherwise. """ return datetime.now(UTC()) <= self.start + + +class CourseSummary(object): + """ + A lightweight course summary class, which constructs split/mongo course summary without loading + the course. It is used at cms for listing courses to global staff user. + """ + course_info_fields = ['display_name', 'display_coursenumber', 'display_organization'] + + def __init__(self, course_locator, display_name=u"Empty", display_coursenumber=None, display_organization=None): + """ + Initialize and construct course summary + + Arguments: + course_locator (CourseLocator): CourseLocator object of the course. + + display_name (unicode): display name of the course. When you create a course from console, display_name + isn't set (course block has no key `display_name`). "Empty" name is returned when we load the course. + If `display_name` isn't present in the course block, use the `Empty` as default display name. + We can set None as a display_name in Course Advance Settings; Do not use "Empty" when display_name is + set to None. + + display_coursenumber (unicode|None): Course number that is specified & appears in the courseware + + display_organization (unicode|None): Course organization that is specified & appears in the courseware + + """ + self.display_coursenumber = display_coursenumber + self.display_organization = display_organization + self.display_name = display_name + + self.id = course_locator # pylint: disable=invalid-name + self.location = course_locator.make_usage_key('course', 'course') + + @property + def display_org_with_default(self): + """ + Return a display organization if it has been specified, otherwise return the 'org' that + is in the location + """ + if self.display_organization: + return self.display_organization + return self.location.org + + @property + def display_number_with_default(self): + """ + Return a display course number if it has been specified, otherwise return the 'course' that + is in the location + """ + if self.display_coursenumber: + return self.display_coursenumber + return self.location.course diff --git a/common/lib/xmodule/xmodule/modulestore/exceptions.py b/common/lib/xmodule/xmodule/modulestore/exceptions.py index 12be4c398b..10ac1fe858 100644 --- a/common/lib/xmodule/xmodule/modulestore/exceptions.py +++ b/common/lib/xmodule/xmodule/modulestore/exceptions.py @@ -11,6 +11,13 @@ class ItemWriteConflictError(Exception): pass +class MultipleCourseBlocksFound(Exception): + """ + Raise this exception when Iterating over the course blocks return multiple course blocks. + """ + pass + + class InsufficientSpecificationError(Exception): pass diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index ac20f7ad64..3beb3970d5 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -265,6 +265,26 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): store = self._get_modulestore_for_courselike(course_key) return store.get_items(course_key, **kwargs) + @strip_key + def get_course_summaries(self, **kwargs): + """ + Returns a list containing the course information in CourseSummary objects. + Information contains `location`, `display_name`, `locator` of the courses in this modulestore. + """ + course_summaries = {} + for store in self.modulestores: + for course_summary in store.get_course_summaries(**kwargs): + course_id = self._clean_locator_for_mapping(locator=course_summary.id) + + # Check if course is indeed unique. Save it in result if unique + if course_id in course_summaries: + log.warning( + u"Modulestore %s have duplicate courses %s; skipping from result.", store, course_id + ) + else: + course_summaries[course_id] = course_summary + return course_summaries.values() + @strip_key def get_courses(self, **kwargs): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 639d3c236e..840fd48c03 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -39,6 +39,7 @@ from xblock.fields import Scope, ScopeIds, Reference, ReferenceList, ReferenceVa from xblock.runtime import KvsFieldData from xmodule.assetstore import AssetMetadata, CourseAssetsFromStorage +from xmodule.course_module import CourseSummary from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import null_error_tracker, exc_info_to_str from xmodule.exceptions import HeartbeatFailure @@ -968,6 +969,40 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo not (category == 'course' and depth == 0) return apply_cached_metadata + @autoretry_read() + def get_course_summaries(self, **kwargs): + """ + Returns a list of `CourseSummary`. This accepts an optional parameter of 'org' which + will apply an efficient filter to only get courses with the specified ORG + """ + def extract_course_summary(course): + """ + Extract course information from the course block for mongo. + """ + return { + field: course['metadata'][field] + for field in CourseSummary.course_info_fields + if field in course['metadata'] + } + + course_org_filter = kwargs.get('org') + query = {'_id.category': 'course'} + + if course_org_filter: + query['_id.org'] = course_org_filter + + course_records = self.collection.find(query, {'metadata': True}) + + courses_summaries = [] + for course in course_records: + if not (course['_id']['org'] == 'edx' and course['_id']['course'] == 'templates'): + locator = SlashSeparatedCourseKey(course['_id']['org'], course['_id']['course'], course['_id']['name']) + course_summary = extract_course_summary(course) + courses_summaries.append( + CourseSummary(locator, **course_summary) + ) + return courses_summaries + @autoretry_read() def get_courses(self, **kwargs): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index d5e1bbdea9..995ec6fe2c 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -209,20 +209,20 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): parent = course_key.make_usage_key(parent_key.type, parent_key.id) else: parent = None - kvs = SplitMongoKVS( - definition_loader, - converted_fields, - converted_defaults, - parent=parent, - field_decorator=kwargs.get('field_decorator') - ) - - if InheritanceMixin in self.modulestore.xblock_mixins: - field_data = inheriting_field_data(kvs) - else: - field_data = KvsFieldData(kvs) - try: + kvs = SplitMongoKVS( + definition_loader, + converted_fields, + converted_defaults, + parent=parent, + field_decorator=kwargs.get('field_decorator') + ) + + if InheritanceMixin in self.modulestore.xblock_mixins: + field_data = inheriting_field_data(kvs) + else: + field_data = KvsFieldData(kvs) + module = self.construct_xblock_from_class( class_, ScopeIds(None, block_key.type, definition_id, block_locator), diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py index 1352ee95cd..d675de85ed 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -353,6 +353,26 @@ class MongoConnection(object): tagger.measure("structures", len(docs)) return docs + @autoretry_read() + def find_course_blocks_by_id(self, ids, course_context=None): + """ + Find all structures that specified in `ids`. Among the blocks only return block whose type is `course`. + + Arguments: + ids (list): A list of structure ids + """ + with TIMER.timer("find_course_blocks_by_id", course_context) as tagger: + tagger.measure("requested_ids", len(ids)) + docs = [ + structure_from_mongo(structure, course_context) + for structure in self.structures.find( + {'_id': {'$in': ids}}, + {'blocks': {'$elemMatch': {'block_type': 'course'}}, 'root': 1} + ) + ] + tagger.measure("structures", len(docs)) + return docs + @autoretry_read() def find_structures_derived_from(self, ids, course_context=None): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index e59e52666b..95a913255f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -66,13 +66,14 @@ from bson.objectid import ObjectId from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict +from xmodule.course_module import CourseSummary from xmodule.errortracker import null_error_tracker from opaque_keys.edx.locator import ( BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) from ccx_keys.locator import CCXLocator, CCXBlockUsageLocator from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ - DuplicateCourseError + DuplicateCourseError, MultipleCourseBlocksFound from xmodule.modulestore import ( inheritance, ModuleStoreWriteBase, ModuleStoreEnum, BulkOpsRecord, BulkOperationsMixin, SortedAssetList, BlockData @@ -539,6 +540,17 @@ class SplitBulkWriteMixin(BulkOperationsMixin): return indexes + def find_course_blocks_by_id(self, ids): + """ + Find all structures that specified in `ids`. Filter the course blocks to only return whose + `block_type` is `course` + + Arguments: + ids (list): A list of structure ids + """ + ids = set(ids) + return self.db_connection.find_course_blocks_by_id(list(ids)) + def find_structures_by_id(self, ids): """ Return all structures that specified in ``ids``. @@ -849,15 +861,39 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # add it in the envelope for the structure. return CourseEnvelope(course_key.replace(version_guid=version_guid), entry) + def _get_course_blocks_for_branch(self, branch, **kwargs): + """ + Internal generator for fetching lists of courses without loading them. + """ + version_guids, id_version_map = self.collect_ids_from_matching_indexes(branch, **kwargs) + + if not version_guids: + return + + for entry in self.find_course_blocks_by_id(version_guids): + for course_index in id_version_map[entry['_id']]: + yield entry, course_index + def _get_structures_for_branch(self, branch, **kwargs): """ Internal generator for fetching lists of courses, libraries, etc. """ + version_guids, id_version_map = self.collect_ids_from_matching_indexes(branch, **kwargs) - # if we pass in a 'org' parameter that means to - # only get the course which match the passed in - # ORG + if not version_guids: + return + for entry in self.find_structures_by_id(version_guids): + for course_index in id_version_map[entry['_id']]: + yield entry, course_index + + def collect_ids_from_matching_indexes(self, branch, **kwargs): + """ + Find the course_indexes which have the specified branch. if `kwargs` contains `org` + to apply an ORG filter to return only the courses that are part of that ORG. Extract `version_guids` + from the course_indexes. + + """ matching_indexes = self.find_matching_course_indexes( branch, search_targets=None, @@ -871,13 +907,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): version_guid = course_index['versions'][branch] version_guids.append(version_guid) id_version_map[version_guid].append(course_index) - - if not version_guids: - return - - for entry in self.find_structures_by_id(version_guids): - for course_index in id_version_map[entry['_id']]: - yield entry, course_index + return version_guids, id_version_map def _get_structures_for_branch_and_locator(self, branch, locator_factory, **kwargs): @@ -933,6 +963,50 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # get the blocks for each course index (s/b the root) return self._get_structures_for_branch_and_locator(branch, self._create_course_locator, **kwargs) + @autoretry_read() + def get_course_summaries(self, branch, **kwargs): + """ + Returns a list of `CourseSummary` which matching any given qualifiers. + + qualifiers should be a dict of keywords matching the db fields or any + legal query for mongo to use against the active_versions collection. + + Note, this is to find the current head of the named branch type. + To get specific versions via guid use get_course. + + :param branch: the branch for which to return courses. + """ + def extract_course_summary(course): + """ + Extract course information from the course block for split. + """ + return { + field: course.fields[field] + for field in CourseSummary.course_info_fields + if field in course.fields + } + + courses_summaries = [] + for entry, structure_info in self._get_course_blocks_for_branch(branch, **kwargs): + course_locator = self._create_course_locator(structure_info, branch=None) + course_block = [ + block_data + for block_key, block_data in entry['blocks'].items() + if block_key.type == "course" + ] + if not course_block: + raise ItemNotFoundError + + if len(course_block) > 1: + raise MultipleCourseBlocksFound( + "Expected 1 course block to be found in the course, but found {0}".format(len(course_block)) + ) + course_summary = extract_course_summary(course_block[0]) + courses_summaries.append( + CourseSummary(course_locator, **course_summary) + ) + return courses_summaries + def get_libraries(self, branch="library", **kwargs): """ Returns a list of "library" root blocks matching any given qualifiers. 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 8f2554eab1..dc25acf7ac 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -74,6 +74,22 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli source_course_id, dest_course_id, user_id, fields=fields, **kwargs ) + def get_course_summaries(self, **kwargs): + """ + Returns course summaries on the Draft or Published branch depending on the branch setting. + """ + branch_setting = self.get_branch_setting() + if branch_setting == ModuleStoreEnum.Branch.draft_preferred: + return super(DraftVersioningModuleStore, self).get_course_summaries( + ModuleStoreEnum.BranchName.draft, **kwargs + ) + elif branch_setting == ModuleStoreEnum.Branch.published_only: + return super(DraftVersioningModuleStore, self).get_course_summaries( + ModuleStoreEnum.BranchName.published, **kwargs + ) + else: + raise InsufficientSpecificationError() + def get_courses(self, **kwargs): """ Returns all the courses on the Draft or Published branch depending on the branch setting. diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py index 5a3fd41689..fab98f1acd 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py @@ -5,6 +5,7 @@ Tests of modulestore semantics: How do the interfaces methods of ModuleStore rel import ddt import itertools from collections import namedtuple +from xmodule.course_module import CourseSummary from xmodule.modulestore.tests.utils import ( PureModulestoreTestCase, MongoModulestoreBuilder, @@ -177,6 +178,20 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): """ self.assertNotParentOf(self.course.scope_ids.usage_id, block_usage_key, draft=draft) + def assertCourseSummaryFields(self, course_summaries): + """ + Assert that the `course_summary` of a course has all expected fields. + + Arguments: + course_summaries: list of CourseSummary class objects. + """ + def verify_course_summery_fields(course_summary): + """ Verify that every `course_summary` object has all the required fields """ + expected_fields = CourseSummary.course_info_fields + ['id', 'location'] + return all([hasattr(course_summary, field) for field in expected_fields]) + + self.assertTrue(all(verify_course_summery_fields(course_summary) for course_summary in course_summaries)) + def is_detached(self, block_type): """ Return True if ``block_type`` is a detached block. @@ -259,6 +274,21 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase): self.assertCourseDoesntPointToBlock(block_usage_key) self.assertBlockDoesntExist(block_usage_key) + @ddt.data(ModuleStoreEnum.Branch.draft_preferred, ModuleStoreEnum.Branch.published_only) + def test_course_summaries(self, branch): + """ Test that `get_course_summaries` method in modulestore work as expected. """ + with self.store.branch_setting(branch_setting=branch): + course_summaries = self.store.get_course_summaries() + + # Verify course summaries + self.assertEqual(len(course_summaries), 1) + + # Verify that all course summary objects have the required attributes. + self.assertCourseSummaryFields(course_summaries) + + # Verify fetched accessible courses list is a list of CourseSummery instances + self.assertTrue(all(isinstance(course, CourseSummary) for course in course_summaries)) + @ddt.data(*itertools.product(['chapter', 'sequential'], [True, False])) @ddt.unpack def test_delete_child(self, block_type, child_published): diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index a0033951f5..7ea73be558 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -835,6 +835,12 @@ class XMLModuleStore(ModuleStoreReadBase): """ return self.courses.values() + def get_course_summaries(self, **kwargs): + """ + Returns `self.get_courses()`. Use to list courses to the global staff user. + """ + return self.get_courses(**kwargs) + def get_errored_courses(self): """ Return a dictionary of course_dir -> [(msg, exception_str)], for each