diff --git a/README.md b/README.md index 60588223d..5250ded1f 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,16 @@ The following flags are available: |--------------------------------|-------------------------------------------------------| | display_learner_analytics | Display Learner Analytics links | + +Settings describe features which are not expected to be toggled on and off without significant system changes. + +The following setting is available: + +| Flag | Purpose | +|--------------------------------|-------------------------------------------------------| +| ENROLLMENT_AGE_AVAILABLE | Display age as part of enrollment demographics | + + Authentication & Authorization ------------------------------ This section is only necessary if running I stand alone service OAuth2 is automatically configured by provisioning in devstack. diff --git a/analytics_dashboard/courses/tests/test_views/__init__.py b/analytics_dashboard/courses/tests/test_views/__init__.py index 93ad44519..644d1789b 100644 --- a/analytics_dashboard/courses/tests/test_views/__init__.py +++ b/analytics_dashboard/courses/tests/test_views/__init__.py @@ -1,10 +1,11 @@ import json import logging +from unittest import mock from unittest.mock import Mock, patch import httpretty from analyticsclient.exceptions import NotFoundError -from ddt import data, ddt +from ddt import data, ddt, unpack from django.conf import settings from django.contrib.humanize.templatetags.humanize import intcomma from django.core.cache import cache @@ -22,6 +23,8 @@ get_mock_api_enrollment_data, mock_course_name, ) +from analytics_dashboard.courses.views.enrollment import EnrollmentTemplateView, _enrollment_secondary_nav, \ + EnrollmentDemographicsTemplateView, _enrollment_tertiary_nav logger = logging.getLogger(__name__) @@ -157,12 +160,6 @@ def path(self, **kwargs): class NavAssertMixin: - def assertPrimaryNav(self, nav, course_id): - raise NotImplementedError - - def assertSecondaryNavs(self, nav, course_id): - raise NotImplementedError - def assertNavs(self, actual_navs, expected_navs, active_nav_label): for item in expected_navs: if item['text'] == active_nav_label: @@ -185,17 +182,6 @@ def test_invalid_course(self): response = self.client.get(path, follow=True) self.assertEqual(response.status_code, 404) - def getAndValidateView(self, course_id): - raise NotImplementedError - - @httpretty.activate - @data(CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID) - @override_switch('enable_course_api', active=True) - @override_switch('display_course_name_in_nav', active=True) - def test_valid_course(self, course_id): - self.mock_course_detail(course_id) - self.getAndValidateView(course_id) - def assertValidMissingDataContext(self, context): raise NotImplementedError @@ -216,10 +202,31 @@ def assertValidCourseName(self, course_id, context): # pylint: disable=abstract-method +@ddt class CourseEnrollmentViewTestMixin(CourseViewTestMixin): active_secondary_nav_label = None api_method = 'analyticsclient.course.Course.enrollment' + @httpretty.activate + @data( + (CourseSamples.DEMO_COURSE_ID, True), + (CourseSamples.DEMO_COURSE_ID, False), + (CourseSamples.DEPRECATED_DEMO_COURSE_ID, True) + ) + @unpack + @override_switch('enable_course_api', active=True) + @override_switch('display_course_name_in_nav', active=True) + def test_valid_course(self, course_id, age_available): + with mock.patch('analytics_dashboard.courses.views.enrollment.age_available', return_value=age_available): + # we have to rebuild nav according to setting since navs are on the class + EnrollmentTemplateView.secondary_nav_items = _enrollment_secondary_nav() + EnrollmentDemographicsTemplateView.tertiary_nav_items = _enrollment_tertiary_nav() + self.mock_course_detail(course_id) + self.getAndValidateView(course_id, age_available) + # put the navs back to default + EnrollmentTemplateView.secondary_nav_items = _enrollment_secondary_nav() + EnrollmentDemographicsTemplateView.tertiary_nav_items = _enrollment_tertiary_nav() + def assertPrimaryNav(self, nav, course_id): expected = { 'icon': 'fa-child', @@ -235,7 +242,7 @@ def assertPrimaryNav(self, nav, course_id): } self.assertDictEqual(nav, expected) - def assertSecondaryNavs(self, nav, course_id): + def assertSecondaryNavs(self, nav, course_id, age_available): reverse_kwargs = {'course_id': course_id} expected = [ { @@ -270,6 +277,18 @@ def assertSecondaryNavs(self, nav, course_id): } ] + if not age_available: + expected[1] = { + 'name': 'demographics', + 'text': 'Demographics', + 'translated_text': _('Demographics'), + 'href': reverse('courses:enrollment:demographics_education', kwargs=reverse_kwargs), + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'demographics', + 'depth': 'education' + } + self.assertNavs(nav, expected, self.active_secondary_nav_label) def get_mock_data(self, course_id): @@ -288,12 +307,12 @@ def format_tip_percent(self, value): return formatted_percent - def assertAllNavs(self, context, course_id): + def assertAllNavs(self, context, course_id, age_available): self.assertPrimaryNav(context['primary_nav_item'], course_id) - self.assertSecondaryNavs(context['secondary_nav_items'], course_id) - self.assertTertiaryNavs(context['tertiary_nav_items'], course_id) + self.assertSecondaryNavs(context['secondary_nav_items'], course_id, age_available) + self.assertTertiaryNavs(context['tertiary_nav_items'], course_id, age_available) - def assertTertiaryNavs(self, nav, course_id): + def assertTertiaryNavs(self, nav, course_id, age_available): reverse_kwargs = {'course_id': course_id} expected = [ { @@ -327,6 +346,8 @@ def assertTertiaryNavs(self, nav, course_id): 'depth': 'gender' } ] + if not age_available: + expected.pop(0) self.assertNavs(nav, expected, self.active_tertiary_nav_label) diff --git a/analytics_dashboard/courses/tests/test_views/test_engagement.py b/analytics_dashboard/courses/tests/test_views/test_engagement.py index 6764ecfa8..eca21273d 100644 --- a/analytics_dashboard/courses/tests/test_views/test_engagement.py +++ b/analytics_dashboard/courses/tests/test_views/test_engagement.py @@ -1,7 +1,7 @@ from unittest.mock import Mock, patch import analyticsclient.constants.activity_types as AT import httpretty -from ddt import ddt +from ddt import ddt, data from django.test import TestCase from django.urls import reverse from django.utils.translation import ugettext_lazy as _ @@ -88,6 +88,14 @@ class CourseEngagementContentViewTests(CourseViewTestMixin, CourseEngagementView 'analytics_dashboard.courses.presenters.engagement.CourseEngagementActivityPresenter.get_summary_and_trend_data' active_secondary_nav_label = 'Content' + @httpretty.activate + @data(CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID) + @override_switch('enable_course_api', active=True) + @override_switch('display_course_name_in_nav', active=True) + def test_valid_course(self, course_id): + self.mock_course_detail(course_id) + self.getAndValidateView(course_id) + def get_expected_secondary_nav(self, course_id): expected = super().get_expected_secondary_nav(course_id) expected[1].update({ diff --git a/analytics_dashboard/courses/tests/test_views/test_enrollment.py b/analytics_dashboard/courses/tests/test_views/test_enrollment.py index 19f78c9fe..827f2a6c0 100644 --- a/analytics_dashboard/courses/tests/test_views/test_enrollment.py +++ b/analytics_dashboard/courses/tests/test_views/test_enrollment.py @@ -18,7 +18,7 @@ class CourseEnrollmentActivityViewTests(CourseEnrollmentViewTestMixin, TestCase) presenter_method = \ 'analytics_dashboard.courses.presenters.enrollment.CourseEnrollmentPresenter.get_summary_and_trend_data' - def getAndValidateView(self, course_id): + def getAndValidateView(self, course_id, age_available): summary, enrollment_data = utils.get_mock_enrollment_summary_and_trend(course_id) rv = summary, enrollment_data with mock.patch(self.presenter_method, return_value=rv): @@ -42,7 +42,7 @@ def getAndValidateView(self, course_id): self.assertListEqual(trend_data, expected) self.assertPrimaryNav(context['primary_nav_item'], course_id) - self.assertSecondaryNavs(context['secondary_nav_items'], course_id) + self.assertSecondaryNavs(context['secondary_nav_items'], course_id, age_available) self.assertValidCourseName(course_id, response.context) def assertValidMissingDataContext(self, context): @@ -60,7 +60,7 @@ class CourseEnrollmentGeographyViewTests(CourseEnrollmentViewTestMixin, TestCase def get_mock_data(self, course_id): return utils.get_mock_api_enrollment_geography_data(course_id) - def getAndValidateView(self, course_id): + def getAndValidateView(self, course_id, age_available): with mock.patch(self.presenter_method, return_value=utils.get_mock_presenter_enrollment_geography_data()): response = self.client.get(self.path(course_id=course_id)) context = response.context @@ -76,6 +76,8 @@ def getAndValidateView(self, course_id): self.assertEqual(page_data['course']['enrollmentByCountry'], expected_data) self.assertValidCourseName(course_id, response.context) + self.assertPrimaryNav(context['primary_nav_item'], course_id) + self.assertSecondaryNavs(context['secondary_nav_items'], course_id, age_available) def assertValidMissingDataContext(self, context): self.assertIsNone(context['update_message']) @@ -89,7 +91,7 @@ class CourseEnrollmentDemographicsAge(CourseEnrollmentDemographicsMixin, TestCas presenter_method = \ 'analytics_dashboard.courses.presenters.enrollment.CourseEnrollmentDemographicsPresenter.get_ages' - def getAndValidateView(self, course_id): + def getAndValidateView(self, course_id, age_available): last_updated, summary, binned_ages, known_percent = utils.get_presenter_ages() rv = last_updated, summary, binned_ages, known_percent with mock.patch(self.presenter_method, return_value=rv): @@ -109,7 +111,7 @@ def getAndValidateView(self, course_id): actual_ages = page_data['course']['ages'] self.assertListEqual(actual_ages, binned_ages) self.assertDictEqual(context['summary'], summary) - self.assertAllNavs(context, course_id) + self.assertAllNavs(context, course_id, age_available) self.assertValidCourseName(course_id, response.context) @@ -128,7 +130,7 @@ class CourseEnrollmentDemographicsEducation(CourseEnrollmentDemographicsMixin, T presenter_method = \ 'analytics_dashboard.courses.presenters.enrollment.CourseEnrollmentDemographicsPresenter.get_education' - def getAndValidateView(self, course_id): + def getAndValidateView(self, course_id, age_available): last_updated, summary, education_data, known_percent = utils.get_presenter_education() rv = last_updated, summary, education_data, known_percent with mock.patch(self.presenter_method, return_value=rv): @@ -148,7 +150,7 @@ def getAndValidateView(self, course_id): actual_education = page_data['course']['education'] self.assertListEqual(actual_education, education_data) self.assertDictEqual(context['summary'], summary) - self.assertAllNavs(context, course_id) + self.assertAllNavs(context, course_id, age_available) self.assertValidCourseName(course_id, response.context) @@ -167,7 +169,7 @@ class CourseEnrollmentDemographicsGender(CourseEnrollmentDemographicsMixin, Test presenter_method = \ 'analytics_dashboard.courses.presenters.enrollment.CourseEnrollmentDemographicsPresenter.get_gender' - def getAndValidateView(self, course_id): + def getAndValidateView(self, course_id, age_available): last_updated, gender_data, trend, known_percent = utils.get_presenter_gender(course_id) rv = last_updated, gender_data, trend, known_percent with mock.patch(self.presenter_method, return_value=rv): @@ -190,7 +192,7 @@ def getAndValidateView(self, course_id): actual_trends = page_data['course']['genderTrend'] self.assertListEqual(actual_trends, trend) - self.assertAllNavs(context, course_id) + self.assertAllNavs(context, course_id, age_available) self.assertValidCourseName(course_id, response.context) def get_mock_data(self, course_id): diff --git a/analytics_dashboard/courses/tests/test_views/test_pages.py b/analytics_dashboard/courses/tests/test_views/test_pages.py index d24f85deb..1c8c533ee 100644 --- a/analytics_dashboard/courses/tests/test_views/test_pages.py +++ b/analytics_dashboard/courses/tests/test_views/test_pages.py @@ -14,6 +14,14 @@ class CourseHomeViewTests(CourseViewTestMixin, TestCase): viewname = 'courses:home' + @httpretty.activate + @data(CourseSamples.DEMO_COURSE_ID, CourseSamples.DEPRECATED_DEMO_COURSE_ID) + @override_switch('enable_course_api', active=True) + @override_switch('display_course_name_in_nav', active=True) + def test_valid_course(self, course_id): + self.mock_course_detail(course_id) + self.getAndValidateView(course_id) + def getAndValidateView(self, course_id): response = self.client.get(self.path(course_id=course_id)) self.assertEqual(response.status_code, 200) diff --git a/analytics_dashboard/courses/views/__init__.py b/analytics_dashboard/courses/views/__init__.py index c91c8713d..02686504c 100644 --- a/analytics_dashboard/courses/views/__init__.py +++ b/analytics_dashboard/courses/views/__init__.py @@ -38,7 +38,7 @@ from analytics_dashboard.courses.serializers import LazyEncoder from analytics_dashboard.courses.utils import get_page_name, is_feature_enabled from analytics_dashboard.courses.waffle import ( - DISPLAY_LEARNER_ANALYTICS, + DISPLAY_LEARNER_ANALYTICS, age_available, ) from analytics_dashboard.help.views import ContextSensitiveHelpMixin @@ -537,21 +537,19 @@ class CourseHome(AnalyticsV0Mixin, CourseTemplateWithNavView): def get_table_items(self): items = [] - enrollment_items = { - 'name': _('Enrollment'), - 'icon': 'fa-child', - 'heading': _('Who are my learners?'), - 'items': [ - { - 'title': ugettext_noop('How many learners are in my course?'), - 'view': 'courses:enrollment:activity', - 'breadcrumbs': [_('Activity')], - 'fragment': '', - 'scope': 'course', - 'lens': 'enrollment', - 'report': 'activity', - 'depth': '' - }, + enrollment_subitems = [ + { + 'title': ugettext_noop('How many learners are in my course?'), + 'view': 'courses:enrollment:activity', + 'breadcrumbs': [_('Activity')], + 'fragment': '', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'activity', + 'depth': '' + }] + if age_available(): + enrollment_subitems = enrollment_subitems + [ { 'title': ugettext_noop('How old are my learners?'), 'view': 'courses:enrollment:demographics_age', @@ -561,38 +559,45 @@ def get_table_items(self): 'lens': 'enrollment', 'report': 'demographics', 'depth': 'age' - }, - { - 'title': ugettext_noop('What level of education do my learners have?'), - 'view': 'courses:enrollment:demographics_education', - 'breadcrumbs': [_('Demographics'), _('Education')], - 'fragment': '', - 'scope': 'course', - 'lens': 'enrollment', - 'report': 'demographics', - 'depth': 'education' - }, - { - 'title': ugettext_noop('What is the learner gender breakdown?'), - 'view': 'courses:enrollment:demographics_gender', - 'breadcrumbs': [_('Demographics'), _('Gender')], - 'fragment': '', - 'scope': 'course', - 'lens': 'enrollment', - 'report': 'demographics', - 'depth': 'gender' - }, - { - 'title': ugettext_noop('Where are my learners?'), - 'view': 'courses:enrollment:geography', - 'breadcrumbs': [_('Geography')], - 'fragment': '', - 'scope': 'course', - 'lens': 'enrollment', - 'report': 'geography', - 'depth': '' - }, - ], + }] + enrollment_subitems = enrollment_subitems + [ + { + 'title': ugettext_noop('What level of education do my learners have?'), + 'view': 'courses:enrollment:demographics_education', + 'breadcrumbs': [_('Demographics'), _('Education')], + 'fragment': '', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'demographics', + 'depth': 'education' + }, + { + 'title': ugettext_noop('What is the learner gender breakdown?'), + 'view': 'courses:enrollment:demographics_gender', + 'breadcrumbs': [_('Demographics'), _('Gender')], + 'fragment': '', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'demographics', + 'depth': 'gender' + }, + { + 'title': ugettext_noop('Where are my learners?'), + 'view': 'courses:enrollment:geography', + 'breadcrumbs': [_('Geography')], + 'fragment': '', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'geography', + 'depth': '' + }, + ] + + enrollment_items = { + 'name': _('Enrollment'), + 'icon': 'fa-child', + 'heading': _('Who are my learners?'), + 'items': enrollment_subitems, } items.append(enrollment_items) diff --git a/analytics_dashboard/courses/views/enrollment.py b/analytics_dashboard/courses/views/enrollment.py index 0338876e6..86f952d96 100644 --- a/analytics_dashboard/courses/views/enrollment.py +++ b/analytics_dashboard/courses/views/enrollment.py @@ -11,14 +11,31 @@ CourseEnrollmentPresenter, ) from analytics_dashboard.courses.views import CourseTemplateWithNavView, AnalyticsV0Mixin, AnalyticsV1Mixin +from analytics_dashboard.courses.waffle import age_available logger = logging.getLogger(__name__) -class EnrollmentTemplateView(CourseTemplateWithNavView): - """ - Base view for course enrollment pages. - """ +# separated from EnrollmentTemplateView so it can be rerun in test +def _enrollment_secondary_nav(): + demographics_landing_view = { + 'name': 'demographics', + 'text': ugettext_noop('Demographics'), + 'view': 'courses:enrollment:demographics_age', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'demographics', + 'depth': 'age' + } if age_available() else { + 'name': 'demographics', + 'text': ugettext_noop('Demographics'), + 'view': 'courses:enrollment:demographics_education', + 'scope': 'course', + 'lens': 'enrollment', + 'report': 'demographics', + 'depth': 'education' + } + secondary_nav_items = [ { 'name': 'activity', @@ -29,15 +46,7 @@ class EnrollmentTemplateView(CourseTemplateWithNavView): 'report': 'activity', 'depth': '' }, - { - 'name': 'demographics', - 'text': ugettext_noop('Demographics'), - 'view': 'courses:enrollment:demographics_age', - 'scope': 'course', - 'lens': 'enrollment', - 'report': 'demographics', - 'depth': 'age' - }, + demographics_landing_view, { 'name': 'geography', 'text': ugettext_noop('Geography'), @@ -49,15 +58,20 @@ class EnrollmentTemplateView(CourseTemplateWithNavView): }, ] translate_dict_values(secondary_nav_items, ('text',)) - active_primary_nav_item = 'enrollment' + return secondary_nav_items -class EnrollmentDemographicsTemplateView(AnalyticsV0Mixin, EnrollmentTemplateView): +class EnrollmentTemplateView(CourseTemplateWithNavView): """ - Base view for course enrollment demographics pages. + Base view for course enrollment pages. """ - active_secondary_nav_item = 'demographics' - tertiary_nav_items = [ + secondary_nav_items = _enrollment_secondary_nav() + active_primary_nav_item = 'enrollment' + + +# separated from the class so it can be invoked in test with varying settings +def _enrollment_tertiary_nav(): + tertiary_age = [ { 'name': 'age', 'text': ugettext_noop('Age'), @@ -66,7 +80,9 @@ class EnrollmentDemographicsTemplateView(AnalyticsV0Mixin, EnrollmentTemplateVie 'lens': 'enrollment', 'report': 'demographics', 'depth': 'age' - }, + } + ] if age_available() else [] + tertiary_nav_items = tertiary_age + [ { 'name': 'education', 'text': ugettext_noop('Education'), @@ -87,6 +103,16 @@ class EnrollmentDemographicsTemplateView(AnalyticsV0Mixin, EnrollmentTemplateVie } ] translate_dict_values(tertiary_nav_items, ('text',)) + return tertiary_nav_items + + +class EnrollmentDemographicsTemplateView(AnalyticsV0Mixin, EnrollmentTemplateView): + """ + Base view for course enrollment demographics pages. + """ + active_secondary_nav_item = 'demographics' + + tertiary_nav_items = _enrollment_tertiary_nav() # Translators: Do not translate UTC. update_message = _('Demographic learner data was last updated %(update_date)s at %(update_time)s UTC.') diff --git a/analytics_dashboard/courses/waffle.py b/analytics_dashboard/courses/waffle.py index f71c67e86..60b8e5aff 100644 --- a/analytics_dashboard/courses/waffle.py +++ b/analytics_dashboard/courses/waffle.py @@ -4,7 +4,7 @@ """ -from edx_toggles.toggles import NonNamespacedWaffleFlag +from edx_toggles.toggles import NonNamespacedWaffleFlag, SettingToggle # .. toggle_name: display_learner_analytics # .. toggle_implementation: WaffleFlag @@ -20,3 +20,17 @@ # https://github.com/edx/edx-analytics-dashboard/pull/522 DISPLAY_LEARNER_ANALYTICS = NonNamespacedWaffleFlag( 'display_learner_analytics', __name__) + +# .. toggle_name: ENROLLMENT_AGE_AVAILABLE +# .. toggle_implementation: SettingToggle +# .. toggle_default: True +# .. toggle_description: Turn this toggle on to show age demographic information in the analytics dashboard. +# Turn it off if your installation cannot or does not collect or share age information. +# .. toggle_use_cases: opt_out +# .. toggle_creation_date: 2021-01-12 +# .. toggle_tickets: https://openedx.atlassian.net/browse/MST-1241 +ENROLLMENT_AGE_AVAILABLE = SettingToggle('ENROLLMENT_AGE_AVAILABLE', default=True) + + +def age_available(): + return ENROLLMENT_AGE_AVAILABLE.is_enabled() diff --git a/analytics_dashboard/settings/base.py b/analytics_dashboard/settings/base.py index b06167c4d..3067afe1c 100644 --- a/analytics_dashboard/settings/base.py +++ b/analytics_dashboard/settings/base.py @@ -321,6 +321,9 @@ DATA_API_AUTH_TOKEN = 'changeme' ########## END DATA API CONFIGURATION +# can this installation collect and display age info +ENROLLMENT_AGE_AVAILABLE = True + # used to determine if a course ID is valid LMS_COURSE_VALIDATION_BASE_URL = None diff --git a/analytics_dashboard/settings/devstack.py b/analytics_dashboard/settings/devstack.py index 0ebbcdb2d..e90046c62 100644 --- a/analytics_dashboard/settings/devstack.py +++ b/analytics_dashboard/settings/devstack.py @@ -17,6 +17,8 @@ DATA_API_V1_ENABLED = True DATA_API_URL_V1 = os.environ.get("API_SERVER_URL", 'http://edx.devstack.analyticsapi:19001/api/v1') +ENROLLMENT_AGE_AVAILABLE = True + # Set these to the correct values for your OAuth2/OpenID Connect provider (e.g., devstack) SOCIAL_AUTH_EDX_OAUTH2_KEY = os.environ.get('SOCIAL_AUTH_EDX_OAUTH2_KEY', 'insights-sso-key') SOCIAL_AUTH_EDX_OAUTH2_SECRET = os.environ.get('SOCIAL_AUTH_EDX_OAUTH2_SECRET', 'insights-sso-secret')