Merge pull request #10749 from edx/aj/tnl3779-use-get-courses-summaries-for-staff

Render CMS course listing Using CourseSummary class for staff

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
 

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

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
 

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -266,6 +266,26 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase):
         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):
         '''
         Returns a list containing the top level XModuleDescriptors of the courses in this modulestore.

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
@@ -969,6 +970,40 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         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):
         '''
         Returns a list of course descriptors. This accepts an optional parameter of 'org' which

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')
-        )
+        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)
+            if InheritanceMixin in self.modulestore.xblock_mixins:
+                field_data = inheriting_field_data(kvs)
+            else:
+                field_data = KvsFieldData(kvs)
 
-        try:
             module = self.construct_xblock_from_class(
                 class_,
                 ScopeIds(None, block_key.type, definition_id, block_locator),

common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py

@@ -354,6 +354,26 @@ class MongoConnection(object):
             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):
         """
         Return all structures that were immediately derived from a structure listed in ``ids``.

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.

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.

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):

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