Skip to content

E
edx-analytics-dashboard
Commit cf763e24 by Tyler Hallada Committed by GitHub
Options

EDUCATOR-431: Improve course summaries performance for small course payloads (#704) 

* Add course_ids cutoff for summaries w/ tests

* Change cutoff to 500
parent 525aa6e0
Showing with 126 additions and 67 deletions
analytics_dashboard/courses/presenters/course_summaries.py
...@@ -19,22 +19,33 @@ class CourseSummariesPresenter(BasePresenter): ...@@ -19,22 +19,33 @@ class CourseSummariesPresenter(BasePresenter):
return all_summaries return all_summaries
return [summary for summary in all_summaries if summary['course_id'] in course_ids] return [summary for summary in all_summaries if summary['course_id'] in course_ids]
def _get_all_summaries(self): def _get_summaries(self, course_ids=None):
""" """Returns list of course summaries.
Returns all course summaries. If not cached, summaries will be fetched
from the analytics data API. If requesting full list and it's not cached or requesting a subset of course_summaries with the course_ids
parameter, summaries will be fetched from the analytics data API.
""" """
all_summaries = cache.get(self.CACHE_KEY) summaries = None
if all_summaries is None: if course_ids is None:
# we only cache the full list of summaries
summaries = cache.get(self.CACHE_KEY)
if summaries is None:
exclude = ['programs'] # we make a separate call to the programs endpoint exclude = ['programs'] # we make a separate call to the programs endpoint
if not switch_is_active('enable_course_passing'): if not switch_is_active('enable_course_passing'):
exclude.append('passing_users') exclude.append('passing_users')
all_summaries = self.client.course_summaries().course_summaries(exclude=exclude) summaries = self.client.course_summaries().course_summaries(course_ids=course_ids, exclude=exclude)
all_summaries = [ 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] field: (
cache.set(self.CACHE_KEY, all_summaries, settings.COURSE_SUMMARIES_CACHE_TIMEOUT) '' if val is None and field in self.NON_NULL_STRING_FIELDS
return all_summaries else val
)
for field, val in summary.items()
} for summary in summaries
]
if course_ids is None:
cache.set(self.CACHE_KEY, summaries, settings.COURSE_SUMMARIES_CACHE_TIMEOUT)
return summaries
def _get_last_updated(self, summaries): def _get_last_updated(self, summaries):
# all the create times should be the same, so just use the first one # all the create times should be the same, so just use the first one
...@@ -48,15 +59,20 @@ class CourseSummariesPresenter(BasePresenter): ...@@ -48,15 +59,20 @@ class CourseSummariesPresenter(BasePresenter):
Returns course summaries that match those listed in course_ids. If Returns course summaries that match those listed in course_ids. If
no course IDs provided, all data will be returned. no course IDs provided, all data will be returned.
""" """
all_summaries = self._get_all_summaries() if course_ids and len(course_ids) > settings.COURSE_SUMMARIES_IDS_CUTOFF:
filtered_summaries = self.filter_summaries(all_summaries, course_ids) # Request all courses from the Analytics API and filter here
all_summaries = self._get_summaries()
summaries = self.filter_summaries(all_summaries, course_ids)
else:
# Request courses only in ID list from the Analytics API
summaries = self._get_summaries(course_ids=course_ids)
# sort by title by default with "None" values at the end # sort by title by default with "None" values at the end
filtered_summaries = sorted( summaries = sorted(
filtered_summaries, summaries,
key=lambda x: (not x['catalog_course_title'], x['catalog_course_title'])) 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): def get_course_summary_metrics(self, summaries):
summary = { summary = {
......
analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py
from ddt import ( from ddt import (
data, data,
ddt ddt,
unpack
) )
import mock import mock
from django.test import ( from django.conf import settings
override_settings, from django.core.cache import cache
TestCase from django.test import TestCase
)
from courses.presenters.course_summaries import CourseSummariesPresenter from courses.presenters.course_summaries import CourseSummariesPresenter
from courses.tests import utils from courses.tests import utils
from courses.tests.utils import CourseSamples from courses.tests.utils import CourseSamples
_ANOTHER_DEPRECATED_COURSE_ID = 'another/course/id'
@ddt @ddt
class CourseSummariesPresenterTests(TestCase): class CourseSummariesPresenterTests(TestCase):
@property _API_SUMMARIES = {
def mock_api_response(self): CourseSamples.DEPRECATED_DEMO_COURSE_ID: {
'''
Returns a mocked API response for two courses including some null fields.
'''
return [{
'course_id': CourseSamples.DEPRECATED_DEMO_COURSE_ID, 'course_id': CourseSamples.DEPRECATED_DEMO_COURSE_ID,
'catalog_course_title': 'Deprecated demo course', 'catalog_course_title': 'Deprecated demo course',
'catalog_course': 'edX+demo.1x', 'catalog_course': 'edX+demo.1x',
...@@ -67,7 +66,8 @@ class CourseSummariesPresenterTests(TestCase): ...@@ -67,7 +66,8 @@ class CourseSummariesPresenterTests(TestCase):
} }
}, },
'created': utils.CREATED_DATETIME_STRING, 'created': utils.CREATED_DATETIME_STRING,
}, { },
CourseSamples.DEMO_COURSE_ID: {
'course_id': CourseSamples.DEMO_COURSE_ID, 'course_id': CourseSamples.DEMO_COURSE_ID,
'catalog_course_title': 'Demo Course', 'catalog_course_title': 'Demo Course',
'catalog_course': None, 'catalog_course': None,
...@@ -112,15 +112,16 @@ class CourseSummariesPresenterTests(TestCase): ...@@ -112,15 +112,16 @@ class CourseSummariesPresenterTests(TestCase):
} }
}, },
'created': utils.CREATED_DATETIME_STRING, '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_title': None,
'catalog_course': None, 'catalog_course': None,
'start_date': None, 'start_date': None,
'end_date': None, 'end_date': None,
'pacing_type': None, 'pacing_type': 'instructor_paced',
'availability': None, 'availability': None,
'count': 1, 'count': None,
'cumulative_count': 1, 'cumulative_count': 1,
'count_change_7_days': 0, 'count_change_7_days': 0,
'enrollment_modes': { 'enrollment_modes': {
...@@ -156,45 +157,85 @@ class CourseSummariesPresenterTests(TestCase): ...@@ -156,45 +157,85 @@ class CourseSummariesPresenterTests(TestCase):
} }
}, },
'created': utils.CREATED_DATETIME_STRING, '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 _PRESENTER_SUMMARIES = {
for summary in summaries: CourseSamples.DEPRECATED_DEMO_COURSE_ID:
for field in CourseSummariesPresenter.NON_NULL_STRING_FIELDS: _API_SUMMARIES[CourseSamples.DEPRECATED_DEMO_COURSE_ID],
if summary[field] is None: CourseSamples.DEMO_COURSE_ID:
summary[field] = '' _API_SUMMARIES[CourseSamples.DEMO_COURSE_ID],
_ANOTHER_DEPRECATED_COURSE_ID:
# sort by title _API_SUMMARIES[_ANOTHER_DEPRECATED_COURSE_ID],
return sorted( }
summaries, _PRESENTER_SUMMARIES[CourseSamples.DEMO_COURSE_ID].update({
key=lambda x: (not x['catalog_course_title'], x['catalog_course_title'])) 'catalog_course': '',
'start_date': '',
@override_settings(CACHES={ 'end_date': '',
'default': { 'pacing_type': '',
'BACKEND': 'django.core.cache.backends.dummy.DummyCache', 'availability': '',
}
}) })
_PRESENTER_SUMMARIES[_ANOTHER_DEPRECATED_COURSE_ID].update({
'catalog_course': '',
'catalog_course_title': '',
'start_date': '',
'end_date': '',
'availability': '',
})
@data( @data(
None, (
[CourseSamples.DEMO_COURSE_ID], None,
[CourseSamples.DEPRECATED_DEMO_COURSE_ID], [
[CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID], 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,
],
),
(
[
CourseSamples.DEMO_COURSE_ID,
] * (settings.COURSE_SUMMARIES_IDS_CUTOFF + 1), # tests filter_summaries
[
CourseSamples.DEMO_COURSE_ID,
],
),
) )
def test_get_summaries(self, course_ids): @unpack
''''Test courses filtered from API response.''' def test_get_summaries(self, input_course_ids, ouptut_course_ids):
presenter = CourseSummariesPresenter() 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', with mock.patch('analyticsclient.course_summaries.CourseSummaries.course_summaries',
mock.Mock(return_value=self.mock_api_response)): mock.Mock(return_value=mock_api_response)):
actual_summaries, last_updated = presenter.get_course_summaries(course_ids=course_ids) actual_summaries, last_updated = presenter.get_course_summaries(course_ids=input_course_ids)
self.assertListEqual(actual_summaries, self.get_expected_summaries(course_ids)) for actual, expected in zip(actual_summaries, expected_summaries):
self.assertItemsEqual(actual, expected)
self.assertEqual(last_updated, utils.CREATED_DATETIME) self.assertEqual(last_updated, utils.CREATED_DATETIME)
def test_no_summaries(self):
cache.clear() # previous test has course_ids=None case cached
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)
analytics_dashboard/settings/base.py
...@@ -460,3 +460,5 @@ WEBPACK_LOADER = { ...@@ -460,3 +460,5 @@ WEBPACK_LOADER = {
########## CDN CONFIGURATION ########## CDN CONFIGURATION
CDN_DOMAIN = None # production will not use a CDN for static assets if this is set to a falsy value CDN_DOMAIN = None # production will not use a CDN for static assets if this is set to a falsy value
########## END CDN CONFIGURATION ########## END CDN CONFIGURATION
COURSE_SUMMARIES_IDS_CUTOFF = 500
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment