Optimize Course Catalog/About API with Course Overviews

lms/djangoapps/course_api/api.py

@@ -3,10 +3,13 @@ Course API
 """
 
 from django.contrib.auth.models import User
-from django.http import Http404
-from rest_framework.exceptions import NotFound, PermissionDenied
+from rest_framework.exceptions import PermissionDenied
 
-from lms.djangoapps.courseware.courses import get_courses, get_course_with_access
+from lms.djangoapps.courseware.courses import (
+    get_courses,
+    get_course_overview_with_access,
+    get_permission_for_course_about,
+)
 
 from .permissions import can_view_courses_for_username
 
@@ -43,11 +46,11 @@ def course_detail(request, username, course_key):
         `CourseDescriptor` object representing the requested course
     """
     user = get_effective_user(request.user, username)
-    try:
-        course = get_course_with_access(user, 'see_exists', course_key)
-    except Http404:
-        raise NotFound()
-    return course
+    return get_course_overview_with_access(
+        user,
+        get_permission_for_course_about(),
+        course_key,
+    )
 
 
 def list_courses(request, username):
@@ -71,5 +74,4 @@ def list_courses(request, username):
         List of `CourseDescriptor` objects representing the collection of courses.
     """
     user = get_effective_user(request.user, username)
-    courses = get_courses(user)
-    return courses
+    return get_courses(user)

lms/djangoapps/course_api/blocks/views.py

@@ -39,8 +39,6 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
         * username: (string) The name of the user on whose behalf we want to
           see the data.
 
-          Default is the logged in user
-
           Example: username=anjali
 
         * student_view_data: (list) Indicates for which block types to return

lms/djangoapps/course_api/serializers.py

@@ -5,42 +5,32 @@ Course API Serializers.  Representing course catalog data
 import urllib
 
 from django.core.urlresolvers import reverse
-from django.template import defaultfilters
-
 from rest_framework import serializers
 
-from lms.djangoapps.courseware.courses import get_course_about_section
-from openedx.core.lib.courses import course_image_url
-from openedx.core.djangoapps.models.course_details import CourseDetails
-from xmodule.course_module import DEFAULT_START_DATE
-
 
 class _MediaSerializer(serializers.Serializer):  # pylint: disable=abstract-method
     """
     Nested serializer to represent a media object.
     """
-    def __init__(self, uri_parser, *args, **kwargs):
+    def __init__(self, uri_attribute, *args, **kwargs):
         super(_MediaSerializer, self).__init__(*args, **kwargs)
-        self.uri_parser = uri_parser
+        self.uri_attribute = uri_attribute
 
     uri = serializers.SerializerMethodField(source='*')
 
-    def get_uri(self, course):
+    def get_uri(self, course_overview):
         """
         Get the representation for the media resource's URI
         """
-        return self.uri_parser(course)
+        return getattr(course_overview, self.uri_attribute)
 
 
 class _CourseApiMediaCollectionSerializer(serializers.Serializer):  # pylint: disable=abstract-method
     """
     Nested serializer to represent a collection of media objects
     """
-    course_image = _MediaSerializer(source='*', uri_parser=course_image_url)
-    course_video = _MediaSerializer(
-        source='*',
-        uri_parser=lambda course: CourseDetails.fetch_video_url(course.id),
-    )
+    course_image = _MediaSerializer(source='*', uri_attribute='course_image_url')
+    course_video = _MediaSerializer(source='*', uri_attribute='course_video_url')
 
 
 class CourseSerializer(serializers.Serializer):  # pylint: disable=abstract-method
@@ -52,14 +42,14 @@ class CourseSerializer(serializers.Serializer):  # pylint: disable=abstract-meth
     name = serializers.CharField(source='display_name_with_default')
     number = serializers.CharField(source='display_number_with_default')
     org = serializers.CharField(source='display_org_with_default')
-    short_description = serializers.SerializerMethodField()
-    effort = serializers.SerializerMethodField()
+    short_description = serializers.CharField()
+    effort = serializers.CharField()
 
     media = _CourseApiMediaCollectionSerializer(source='*')
 
     start = serializers.DateTimeField()
-    start_type = serializers.SerializerMethodField()
-    start_display = serializers.SerializerMethodField()
+    start_type = serializers.CharField()
+    start_display = serializers.CharField()
     end = serializers.DateTimeField()
 
     enrollment_start = serializers.DateTimeField()
@@ -67,46 +57,12 @@ class CourseSerializer(serializers.Serializer):  # pylint: disable=abstract-meth
 
     blocks_url = serializers.SerializerMethodField()
 
-    def get_start_type(self, course):
-        """
-        Get the representation for SerializerMethodField `start_type`
-        """
-        if course.advertised_start is not None:
-            return u'string'
-        elif course.start != DEFAULT_START_DATE:
-            return u'timestamp'
-        else:
-            return u'empty'
-
-    def get_start_display(self, course):
-        """
-        Get the representation for SerializerMethodField `start_display`
-        """
-        if course.advertised_start is not None:
-            return course.advertised_start
-        elif course.start != DEFAULT_START_DATE:
-            return defaultfilters.date(course.start, "DATE_FORMAT")
-        else:
-            return None
-
-    def get_short_description(self, course):
-        """
-        Get the representation for SerializerMethodField `short_description`
-        """
-        return get_course_about_section(self.context['request'], course, 'short_description').strip()
-
-    def get_blocks_url(self, course):
+    def get_blocks_url(self, course_overview):
         """
         Get the representation for SerializerMethodField `blocks_url`
         """
         base_url = '?'.join([
             reverse('blocks_in_course'),
-            urllib.urlencode({'course_id': course.id}),
+            urllib.urlencode({'course_id': course_overview.id}),
         ])
         return self.context['request'].build_absolute_uri(base_url)
-
-    def get_effort(self, course):
-        """
-        Get the representation for SerializerMethodField `effort`
-        """
-        return CourseDetails.fetch_effort(course.id)

lms/djangoapps/course_api/tests/mixins.py

@@ -26,6 +26,7 @@ class CourseApiFactoryMixin(object):
             end=datetime(2015, 9, 19, 18, 0, 0),
             enrollment_start=datetime(2015, 6, 15, 0, 0, 0),
             enrollment_end=datetime(2015, 7, 15, 0, 0, 0),
+            emit_signals=True,
             **kwargs
         )
 

lms/djangoapps/course_api/tests/test_api.py

@@ -3,13 +3,15 @@ Test for course API
 """
 
 from django.contrib.auth.models import AnonymousUser
-from rest_framework.exceptions import NotFound, PermissionDenied
+from django.http import Http404
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.test import APIRequestFactory
 
 from opaque_keys.edx.keys import CourseKey
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ModuleStoreTestCase
-from xmodule.course_module import CourseDescriptor
+from xmodule.modulestore.tests.factories import check_mongo_calls
 
 from ..api import course_detail, list_courses
 from .mixins import CourseApiFactoryMixin
@@ -23,12 +25,12 @@ class CourseApiTestMixin(CourseApiFactoryMixin):
     def setUpClass(cls):
         super(CourseApiTestMixin, cls).setUpClass()
         cls.request_factory = APIRequestFactory()
+        CourseOverview.get_all_courses()  # seed the CourseOverview table
 
     def verify_course(self, course, course_id=u'edX/toy/2012_Fall'):
         """
         Ensure that the returned course is the course we just created
         """
-        self.assertIsInstance(course, CourseDescriptor)
         self.assertEqual(course_id, str(course.id))
 
 
@@ -43,7 +45,8 @@ class CourseDetailTestMixin(CourseApiTestMixin):
         """
         request = Request(self.request_factory.get('/'))
         request.user = requesting_user
-        return course_detail(request, target_user.username, course_key)
+        with check_mongo_calls(0):
+            return course_detail(request, target_user.username, course_key)
 
 
 class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
@@ -64,11 +67,11 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
 
     def test_get_nonexistent_course(self):
         course_key = CourseKey.from_string(u'edX/toy/nope')
-        with self.assertRaises(NotFound):
+        with self.assertRaises(Http404):
             self._make_api_call(self.honor_user, self.honor_user, course_key)
 
     def test_hidden_course_for_honor(self):
-        with self.assertRaises(NotFound):
+        with self.assertRaises(Http404):
             self._make_api_call(self.honor_user, self.honor_user, self.hidden_course.id)
 
     def test_hidden_course_for_staff(self):
@@ -76,7 +79,7 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
         self.verify_course(course, course_id=u'edX/hidden/2012_Fall')
 
     def test_hidden_course_for_staff_as_honor(self):
-        with self.assertRaises(NotFound):
+        with self.assertRaises(Http404):
             self._make_api_call(self.staff_user, self.honor_user, self.hidden_course.id)
 
 
@@ -91,7 +94,8 @@ class CourseListTestMixin(CourseApiTestMixin):
         """
         request = Request(self.request_factory.get('/'))
         request.user = requesting_user
-        return list_courses(request, specified_user.username)
+        with check_mongo_calls(0):
+            return list_courses(request, specified_user.username)
 
     def verify_courses(self, courses):
         """

lms/djangoapps/course_api/tests/test_serializers.py

@@ -5,6 +5,7 @@ Test data created by CourseSerializer
 from datetime import datetime
 
 from openedx.core.djangoapps.models.course_details import CourseDetails
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from rest_framework.test import APIRequestFactory
 from rest_framework.request import Request
 
@@ -29,7 +30,7 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
 
     def _get_request(self, user=None):
         """
-        Build a Request object for the specified user
+        Build a Request object for the specified user.
         """
         if user is None:
             user = self.honor_user
@@ -37,6 +38,13 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
         request.user = user
         return request
 
+    def _get_result(self, course):
+        """
+        Return the CourseSerializer for the specified course.
+        """
+        course_overview = CourseOverview.get_from_id(course.id)
+        return CourseSerializer(course_overview, context={'request': self._get_request()}).data
+
     def test_basic(self):
         expected_data = {
             'course_id': u'edX/toy/2012_Fall',
@@ -55,15 +63,15 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
             'start': u'2015-07-17T12:00:00Z',
             'start_type': u'timestamp',
             'start_display': u'July 17, 2015',
-            'end': u'2015-09-19T18:00:00',
-            'enrollment_start': u'2015-06-15T00:00:00',
-            'enrollment_end': u'2015-07-15T00:00:00',
+            'end': u'2015-09-19T18:00:00Z',
+            'enrollment_start': u'2015-06-15T00:00:00Z',
+            'enrollment_end': u'2015-07-15T00:00:00Z',
             'blocks_url': u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
             'effort': u'6 hours',
         }
         course = self.create_course()
         CourseDetails.update_about_video(course, 'test_youtube_id', self.staff_user.id)  # pylint: disable=no-member
-        result = CourseSerializer(course, context={'request': self._get_request()}).data
+        result = self._get_result(course)
         self.assertDictEqual(result, expected_data)
 
     def test_advertised_start(self):
@@ -72,14 +80,14 @@ class TestCourseSerializerFields(CourseApiFactoryMixin, ModuleStoreTestCase):
             start=datetime(2015, 3, 15),
             advertised_start=u'The Ides of March'
         )
-        result = CourseSerializer(course, context={'request': self._get_request()}).data
+        result = self._get_result(course)
         self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
         self.assertEqual(result['start_type'], u'string')
         self.assertEqual(result['start_display'], u'The Ides of March')
 
     def test_empty_start(self):
         course = self.create_course(start=DEFAULT_START_DATE, course=u'custom')
-        result = CourseSerializer(course, context={'request': self._get_request()}).data
+        result = self._get_result(course)
         self.assertEqual(result['course_id'], u'edX/custom/2012_Fall')
         self.assertEqual(result['start_type'], u'empty')
         self.assertIsNone(result['start_display'])

lms/djangoapps/course_api/views.py

@@ -2,7 +2,7 @@
 Course API Views
 """
 
-from rest_framework.exceptions import NotFound
+from django.http import Http404
 from rest_framework.generics import ListAPIView, RetrieveAPIView
 
 from opaque_keys import InvalidKeyError
@@ -102,7 +102,7 @@ class CourseDetailView(RetrieveAPIView):
         try:
             course_key = CourseKey.from_string(course_key_string)
         except InvalidKeyError:
-            raise NotFound()
+            raise Http404()
 
         return course_detail(self.request, username, course_key)
 

lms/djangoapps/mobile_api/users/serializers.py

@@ -4,12 +4,9 @@ Serializer for user API
 from rest_framework import serializers
 from rest_framework.reverse import reverse
 
-from django.template import defaultfilters
-
 from courseware.access import has_access
 from student.models import CourseEnrollment, User
 from certificates.api import certificate_downloadable_status
-from xmodule.course_module import DEFAULT_START_DATE
 
 
 class CourseOverviewField(serializers.RelatedField):
@@ -19,17 +16,6 @@ class CourseOverviewField(serializers.RelatedField):
 
     def to_representation(self, course_overview):
         course_id = unicode(course_overview.id)
-
-        if course_overview.advertised_start is not None:
-            start_type = 'string'
-            start_display = course_overview.advertised_start
-        elif course_overview.start != DEFAULT_START_DATE:
-            start_type = 'timestamp'
-            start_display = defaultfilters.date(course_overview.start, 'DATE_FORMAT')
-        else:
-            start_type = 'empty'
-            start_display = None
-
         request = self.context.get('request')
         return {
             # identifiers
@@ -40,8 +26,8 @@ class CourseOverviewField(serializers.RelatedField):
 
             # dates
             'start': course_overview.start,
-            'start_display': start_display,
-            'start_type': start_type,
+            'start_display': course_overview.start_display,
+            'start_type': course_overview.start_type,
             'end': course_overview.end,
 
             # notification info

lms/djangoapps/mobile_api/users/tests.py

@@ -168,8 +168,10 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest
     @ddt.data(
         (NEXT_WEEK, ADVERTISED_START, ADVERTISED_START, "string"),
         (NEXT_WEEK, None, defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"),
+        (NEXT_WEEK, '', defaultfilters.date(NEXT_WEEK, "DATE_FORMAT"), "timestamp"),
         (DEFAULT_START_DATE, ADVERTISED_START, ADVERTISED_START, "string"),
-        (DEFAULT_START_DATE, None, None, "empty")
+        (DEFAULT_START_DATE, '', None, "empty"),
+        (DEFAULT_START_DATE, None, None, "empty"),
     )
     @ddt.unpack
     @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})