From b9456c76b6b446da7991aa61e69f0315b917bbc9 Mon Sep 17 00:00:00 2001 From: Andy Shultz Date: Mon, 9 Aug 2021 11:34:27 -0400 Subject: [PATCH 1/2] fix: actually summarize masters count and put masters card check into index test MST-868 --- acceptance_tests/test_course_index.py | 4 ++++ analytics_dashboard/courses/presenters/course_summaries.py | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/acceptance_tests/test_course_index.py b/acceptance_tests/test_course_index.py index 552f3f0b1..9e1cfea77 100644 --- a/acceptance_tests/test_course_index.py +++ b/acceptance_tests/test_course_index.py @@ -317,6 +317,7 @@ def _test_summary_metrics(self): i = 7 count_change_i_days = course_summaries[0]['count_change_%s_days' % i] verified_enrollment = course_summaries[0]['enrollment_modes']['verified']['count'] + masters_enrollment = course_summaries[0]['enrollment_modes']['masters']['count'] tooltip = 'Current enrollments across all of your courses.' self.assertMetricTileValid('current_enrollment', current_enrollment, tooltip) @@ -329,3 +330,6 @@ def _test_summary_metrics(self): tooltip = 'Verified enrollments across all of your courses.' self.assertMetricTileValid('verified_enrollment', verified_enrollment, tooltip) + + tooltip = "Master's enrollments across all of your courses." + self.assertMetricTileValid('masters_enrollment', masters_enrollment, tooltip) diff --git a/analytics_dashboard/courses/presenters/course_summaries.py b/analytics_dashboard/courses/presenters/course_summaries.py index 45613ea15..80711fa3a 100644 --- a/analytics_dashboard/courses/presenters/course_summaries.py +++ b/analytics_dashboard/courses/presenters/course_summaries.py @@ -1,5 +1,6 @@ from functools import reduce # pylint: disable=redefined-builtin +from analyticsclient.constants import enrollment_modes from django.conf import settings from django.core.cache import cache from waffle import switch_is_active @@ -81,7 +82,10 @@ def get_course_summary_metrics(self, summaries): 'total_enrollment': reduce(lambda x, y: x + y.get('cumulative_count', 0), summaries, 0), 'current_enrollment': reduce(lambda x, y: x + y.get('count', 0), summaries, 0), 'enrollment_change_7_days': reduce(lambda x, y: x + y.get('count_change_7_days', 0), summaries, 0), - 'verified_enrollment': reduce(lambda x, y: x + y.get('enrollment_modes', {}).get('verified', + 'verified_enrollment': reduce(lambda x, y: x + y.get('enrollment_modes', {}).get(enrollment_modes.VERIFIED, + {}).get('count', 0), + summaries, 0), + 'masters_enrollment': reduce(lambda x, y: x + y.get('enrollment_modes', {}).get(enrollment_modes.MASTERS, {}).get('count', 0), summaries, 0), } From 940a6d9965e21b81e5b9f1f7235cd57b3b4c5971 Mon Sep 17 00:00:00 2001 From: Andy Shultz Date: Mon, 9 Aug 2021 11:56:49 -0400 Subject: [PATCH 2/2] fix: do not walk the same list five times to avoid writing a for loop Now that this function is in a more readable form puts a test in place for this function and masters test data. Fixes one unexpected test data field where count is None rather than missing or 0. MST-868 --- .../courses/presenters/course_summaries.py | 29 ++++++++++++------- .../test_presenters/test_course_summaries.py | 25 +++++++++++++++- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/analytics_dashboard/courses/presenters/course_summaries.py b/analytics_dashboard/courses/presenters/course_summaries.py index 80711fa3a..726a5b385 100644 --- a/analytics_dashboard/courses/presenters/course_summaries.py +++ b/analytics_dashboard/courses/presenters/course_summaries.py @@ -1,5 +1,3 @@ -from functools import reduce # pylint: disable=redefined-builtin - from analyticsclient.constants import enrollment_modes from django.conf import settings from django.core.cache import cache @@ -78,16 +76,25 @@ def get_course_summaries(self, course_ids=None): return summaries, self._get_last_updated(summaries) def get_course_summary_metrics(self, summaries): + total = 0 + current = 0 + week_change = 0 + verified = 0 + masters = 0 + for s in summaries: + total += s.get('cumulative_count', 0) + current += s.get('count', 0) + week_change += s.get('count_change_7_days', 0) + modes = s.get('enrollment_modes', {}) + verified += modes.get(enrollment_modes.VERIFIED, {}).get('count', 0) + masters += modes.get(enrollment_modes.MASTERS, {}).get('count', 0) + summary = { - 'total_enrollment': reduce(lambda x, y: x + y.get('cumulative_count', 0), summaries, 0), - 'current_enrollment': reduce(lambda x, y: x + y.get('count', 0), summaries, 0), - 'enrollment_change_7_days': reduce(lambda x, y: x + y.get('count_change_7_days', 0), summaries, 0), - 'verified_enrollment': reduce(lambda x, y: x + y.get('enrollment_modes', {}).get(enrollment_modes.VERIFIED, - {}).get('count', 0), - summaries, 0), - 'masters_enrollment': reduce(lambda x, y: x + y.get('enrollment_modes', {}).get(enrollment_modes.MASTERS, - {}).get('count', 0), - summaries, 0), + 'total_enrollment': total, + 'current_enrollment': current, + 'enrollment_change_7_days': week_change, + 'verified_enrollment': verified, + 'masters_enrollment': masters, } return summary diff --git a/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py b/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py index b1daac794..cc48a78f3 100644 --- a/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py +++ b/analytics_dashboard/courses/tests/test_presenters/test_course_summaries.py @@ -103,6 +103,12 @@ class CourseSummariesPresenterTests(TestCase): 'cumulative_count': 4087, 'count_change_7_days': 0, 'passing_users': 100, + }, + 'masters': { + 'count': 1101, + 'cumulative_count': 1103, + 'count_change_7_days': 0, + 'passing_users': 0, } }, 'created': utils.CREATED_DATETIME_STRING, @@ -115,7 +121,7 @@ class CourseSummariesPresenterTests(TestCase): 'end_date': None, 'pacing_type': 'instructor_paced', 'availability': None, - 'count': None, + 'count': 0, 'cumulative_count': 1, 'count_change_7_days': 0, 'enrollment_modes': { @@ -148,6 +154,12 @@ class CourseSummariesPresenterTests(TestCase): 'cumulative_count': 1, 'count_change_7_days': 0, 'passing_users': 0, + }, + 'masters': { + 'count': 10, + 'cumulative_count': 10, + 'count_change_7_days': 0, + 'passing_users': 0, } }, 'created': utils.CREATED_DATETIME_STRING, @@ -233,3 +245,14 @@ def test_no_summaries(self): summaries, last_updated = presenter.get_course_summaries() self.assertListEqual(summaries, []) self.assertIsNone(last_updated) + + def test_get_course_summary_metrics(self): + metrics = CourseSummariesPresenter().get_course_summary_metrics(self._PRESENTER_SUMMARIES.values()) + expected = { + 'total_enrollment': 5111, + 'current_enrollment': 3888, + 'enrollment_change_7_days': 4, + 'verified_enrollment': 13, + 'masters_enrollment': 1111, + } + self.assertEqual(metrics, expected)