Improve performance of CourseSummariesPresenter

CourseSummariesPresenter.get_course_summaries now passes required
course IDs to the course_summaries API call, reducing the size of the
returned list.

EDUCATOR-464

.travis.yml

@@ -8,7 +8,7 @@ services:
 
 env:
   # Make sure to update this string on every Insights or Data API release
-  DATA_API_VERSION: "0.25.1"
+  DATA_API_VERSION: "0.26.0"
   DOCKER_COMPOSE_VERSION: "1.9.0"
 
 before_install:

analytics_dashboard/courses/presenters/course_summaries.py

-from django.conf import settings
-from django.core.cache import cache
 from waffle import switch_is_active
 
 from courses.presenters import BasePresenter
@@ -8,34 +6,9 @@ from courses.presenters import BasePresenter
 class CourseSummariesPresenter(BasePresenter):
     """ Presenter for the course enrollment data. """
 
-    CACHE_KEY = 'summaries'
     NON_NULL_STRING_FIELDS = ['course_id', 'catalog_course', 'catalog_course_title',
                               'start_date', 'end_date', 'pacing_type', 'availability']
 
-    @staticmethod
-    def filter_summaries(all_summaries, course_ids=None):
-        """Filter results to just the course IDs specified."""
-        if course_ids is None:
-            return all_summaries
-        return [summary for summary in all_summaries if summary['course_id'] in course_ids]
-
-    def _get_all_summaries(self):
-        """
-        Returns all course summaries. If not cached, summaries will be fetched
-        from the analytics data API.
-        """
-        all_summaries = cache.get(self.CACHE_KEY)
-        if all_summaries is None:
-            exclude = ['programs']  # we make a separate call to the programs endpoint
-            if not switch_is_active('enable_course_passing'):
-                exclude.append('passing_users')
-            all_summaries = self.client.course_summaries().course_summaries(exclude=exclude)
-            all_summaries = [
-                {field: ('' if val is None and field in self.NON_NULL_STRING_FIELDS else val)
-                 for field, val in summary.items()} for summary in all_summaries]
-            cache.set(self.CACHE_KEY, all_summaries, settings.COURSE_SUMMARIES_CACHE_TIMEOUT)
-        return all_summaries
-
     def _get_last_updated(self, summaries):
         # all the create times should be the same, so just use the first one
         if summaries:
@@ -48,15 +21,27 @@ class CourseSummariesPresenter(BasePresenter):
         Returns course summaries that match those listed in course_ids.  If
         no course IDs provided, all data will be returned.
         """
-        all_summaries = self._get_all_summaries()
-        filtered_summaries = self.filter_summaries(all_summaries, course_ids)
+        exclude = ['programs']  # we make a separate call to the programs endpoint
+        if not switch_is_active('enable_course_passing'):
+            exclude.append('passing_users')
+        summaries = self.client.course_summaries().course_summaries(course_ids=course_ids, exclude=exclude)
+        summaries = [
+            {
+                field: (
+                    '' if val is None and field in self.NON_NULL_STRING_FIELDS
+                    else val
+                )
+                for field, val in summary.items()
+            } for summary in summaries
+        ]
 
         # sort by title by default with "None" values at the end
-        filtered_summaries = sorted(
-            filtered_summaries,
-            key=lambda x: (not x['catalog_course_title'], x['catalog_course_title']))
+        summaries = sorted(
+            summaries,
+            key=lambda x: (not x['catalog_course_title'], x['catalog_course_title'])
+        )
 
-        return filtered_summaries, self._get_last_updated(filtered_summaries)
+        return summaries, self._get_last_updated(summaries)
 
     def get_course_summary_metrics(self, summaries):
         summary = {

analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py

 from ddt import (
     data,
-    ddt
+    ddt,
+    unpack
 )
 import mock
 
-from django.test import (
-    override_settings,
-    TestCase
-)
+from django.test import TestCase
 
 from courses.presenters.course_summaries import CourseSummariesPresenter
 from courses.tests import utils
 from courses.tests.utils import CourseSamples
 
 
+_ANOTHER_DEPRECATED_COURSE_ID = 'another/course/id'
+
+
 @ddt
 class CourseSummariesPresenterTests(TestCase):
 
-    @property
-    def mock_api_response(self):
-        '''
-        Returns a mocked API response for two courses including some null fields.
-        '''
-        return [{
+    _API_SUMMARIES = {
+        CourseSamples.DEPRECATED_DEMO_COURSE_ID: {
             'course_id': CourseSamples.DEPRECATED_DEMO_COURSE_ID,
             'catalog_course_title': 'Deprecated demo course',
             'catalog_course': 'edX+demo.1x',
@@ -67,7 +64,8 @@ class CourseSummariesPresenterTests(TestCase):
                 }
             },
             'created': utils.CREATED_DATETIME_STRING,
-        }, {
+        },
+        CourseSamples.DEMO_COURSE_ID: {
             'course_id': CourseSamples.DEMO_COURSE_ID,
             'catalog_course_title': 'Demo Course',
             'catalog_course': None,
@@ -112,15 +110,16 @@ class CourseSummariesPresenterTests(TestCase):
                 }
             },
             'created': utils.CREATED_DATETIME_STRING,
-        }, {
-            'course_id': 'another/course/id',
+        },
+        _ANOTHER_DEPRECATED_COURSE_ID: {
+            'course_id': _ANOTHER_DEPRECATED_COURSE_ID,
             'catalog_course_title': None,
             'catalog_course': None,
             'start_date': None,
             'end_date': None,
-            'pacing_type': None,
+            'pacing_type': 'instructor_paced',
             'availability': None,
-            'count': 1,
+            'count': None,
             'cumulative_count': 1,
             'count_change_7_days': 0,
             'enrollment_modes': {
@@ -156,45 +155,75 @@ class CourseSummariesPresenterTests(TestCase):
                 }
             },
             'created': utils.CREATED_DATETIME_STRING,
-        }]
-
-    def get_expected_summaries(self, course_ids=None):
-        ''''Expected results with default values, sorted, and filtered to course_ids.'''
-        if course_ids is None:
-            course_ids = [CourseSamples.DEMO_COURSE_ID,
-                          CourseSamples.DEPRECATED_DEMO_COURSE_ID,
-                          'another/course/id']
-
-        summaries = [summary for summary in self.mock_api_response if summary['course_id'] in course_ids]
+        },
+    }
 
-        # fill in with defaults
-        for summary in summaries:
-            for field in CourseSummariesPresenter.NON_NULL_STRING_FIELDS:
-                if summary[field] is None:
-                    summary[field] = ''
-
-        # sort by title
-        return sorted(
-            summaries,
-            key=lambda x: (not x['catalog_course_title'], x['catalog_course_title']))
-
-    @override_settings(CACHES={
-        'default': {
-            'BACKEND': 'django.core.cache.backends.dummy.DummyCache',
-        }
+    _PRESENTER_SUMMARIES = {
+        CourseSamples.DEPRECATED_DEMO_COURSE_ID:
+            _API_SUMMARIES[CourseSamples.DEPRECATED_DEMO_COURSE_ID],
+        CourseSamples.DEMO_COURSE_ID:
+            _API_SUMMARIES[CourseSamples.DEMO_COURSE_ID],
+        _ANOTHER_DEPRECATED_COURSE_ID:
+            _API_SUMMARIES[_ANOTHER_DEPRECATED_COURSE_ID],
+    }
+    _PRESENTER_SUMMARIES[CourseSamples.DEMO_COURSE_ID].update({
+        'catalog_course': '',
+        'start_date': '',
+        'end_date': '',
+        'pacing_type': '',
+        'availability': '',
     })
+    _PRESENTER_SUMMARIES[_ANOTHER_DEPRECATED_COURSE_ID].update({
+        'catalog_course': '',
+        'catalog_course_title': '',
+        'start_date': '',
+        'end_date': '',
+        'availability': '',
+    })
+
     @data(
-        None,
-        [CourseSamples.DEMO_COURSE_ID],
-        [CourseSamples.DEPRECATED_DEMO_COURSE_ID],
-        [CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID],
+        (
+            None,
+            [
+                CourseSamples.DEMO_COURSE_ID,
+                CourseSamples.DEPRECATED_DEMO_COURSE_ID,
+                _ANOTHER_DEPRECATED_COURSE_ID,
+            ],
+        ),
+        (
+            [
+                CourseSamples.DEPRECATED_DEMO_COURSE_ID,
+                CourseSamples.DEMO_COURSE_ID,
+            ],
+            [
+                CourseSamples.DEMO_COURSE_ID,
+                CourseSamples.DEPRECATED_DEMO_COURSE_ID,
+            ],
+        ),
     )
-    def test_get_summaries(self, course_ids):
-        ''''Test courses filtered from API response.'''
+    @unpack
+    def test_get_summaries(self, input_course_ids, ouptut_course_ids):
         presenter = CourseSummariesPresenter()
+        if input_course_ids:
+            mock_api_response = [
+                self._API_SUMMARIES[course_id] for course_id in input_course_ids
+            ]
+        else:
+            mock_api_response = self._API_SUMMARIES.values()
+        expected_summaries = [
+            self._PRESENTER_SUMMARIES[course_id] for course_id in ouptut_course_ids
+        ]
 
         with mock.patch('analyticsclient.course_summaries.CourseSummaries.course_summaries',
-                        mock.Mock(return_value=self.mock_api_response)):
-            actual_summaries, last_updated = presenter.get_course_summaries(course_ids=course_ids)
-            self.assertListEqual(actual_summaries, self.get_expected_summaries(course_ids))
+                        mock.Mock(return_value=mock_api_response)):
+            actual_summaries, last_updated = presenter.get_course_summaries(course_ids=input_course_ids)
+            self.assertListEqual(actual_summaries, expected_summaries)
             self.assertEqual(last_updated, utils.CREATED_DATETIME)
+
+    def test_no_summaries(self):
+        presenter = CourseSummariesPresenter()
+        with mock.patch('analyticsclient.course_summaries.CourseSummaries.course_summaries',
+                        mock.Mock(return_value=[])):
+            summaries, last_updated = presenter.get_course_summaries()
+            self.assertListEqual(summaries, [])
+            self.assertIsNone(last_updated)

requirements/base.txt

@@ -29,7 +29,7 @@ logutils==0.3.4.1			 # BSD
 requests==2.17.3            # Apache 2.0
 
 git+https://github.com/edx/django-lang-pref-middleware.git@0.1.0#egg=django-lang-pref-middleware
-git+https://github.com/edx/edx-analytics-data-api-client.git@0.11.0#egg=edx-analytics-data-api-client==0.11.0  # edX
+git+https://github.com/edx/edx-analytics-data-api-client.git@0.12.0#egg=edx-analytics-data-api-client==0.12.0  # edX
 git+https://github.com/edx/opaque-keys.git@d45d0bd8d64c69531be69178b9505b5d38806ce0#egg=opaque-keys
 # custom opaque-key implementations for ccx
 git+https://github.com/jazkarta/ccx-keys.git@e6b03704b1bb97c1d2f31301ecb4e3a687c536ea#egg=ccx-keys