From b27401554cb773a00a9ccdc023d820ffbd34094d Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Tue, 4 Nov 2014 10:58:13 -0500 Subject: [PATCH 1/3] Feature flagged enrollment counts on instructor dashboard. * enrollments counts removed from legacy dashboard --- .../instructor/views/instructor_dashboard.py | 41 ++++++++++----- lms/djangoapps/instructor/views/legacy.py | 7 ++- lms/envs/common.py | 5 +- .../courseware/instructor_dashboard.html | 16 ------ .../instructor_dashboard_2/course_info.html | 51 +++++++++++-------- 5 files changed, 64 insertions(+), 56 deletions(-) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 315b78413b..f894304954 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -91,11 +91,7 @@ def instructor_dashboard_2(request, course_id): if course_mode_has_price: sections.append(_section_e_commerce(course, access)) - enrollment_count = sections[0]['enrollment_count']['total'] - disable_buttons = False - max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") - if max_enrollment_for_buttons is not None: - disable_buttons = enrollment_count > max_enrollment_for_buttons + disable_buttons = not _is_small_course(course_key) analytics_dashboard_message = None if settings.ANALYTICS_DASHBOARD_URL: @@ -217,12 +213,19 @@ def _section_course_info(course, access): 'access': access, 'course_id': course_key, 'course_display_name': course.display_name, - 'enrollment_count': CourseEnrollment.enrollment_counts(course_key), 'has_started': course.has_started(), 'has_ended': course.has_ended(), 'list_instructor_tasks_url': reverse('list_instructor_tasks', kwargs={'course_id': course_key.to_deprecated_string()}), } + if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'): + section_data['enrollment_count'] = CourseEnrollment.enrollment_counts(course_key) + + if settings.ANALYTICS_DASHBOARD_URL: + dashboard_link = _get_dashboard_link(course_key) + message = _("Enrollment data is now available in {dashboard_link}.").format(dashboard_link=dashboard_link) + section_data['enrollment_message'] = message + try: advance = lambda memo, (letter, score): "{}: {}, ".format(letter, score) + memo section_data['grade_cutoffs'] = reduce(advance, course.grade_cutoffs.items(), "")[:-2] @@ -259,14 +262,19 @@ def _section_membership(course, access): return section_data -def _section_student_admin(course, access): - """ Provide data for the corresponding dashboard section """ - course_key = course.id +def _is_small_course(course_key): is_small_course = False enrollment_count = CourseEnrollment.num_enrolled_in(course_key) max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") if max_enrollment_for_buttons is not None: is_small_course = enrollment_count <= max_enrollment_for_buttons + return is_small_course + + +def _section_student_admin(course, access): + """ Provide data for the corresponding dashboard section """ + course_key = course.id + is_small_course =_is_small_course(course_key) section_data = { 'section_key': 'student_admin', @@ -354,6 +362,16 @@ def _section_send_email(course, access): return section_data +def _get_dashboard_link(course_key): + # Construct a URL to the external analytics dashboard + link = None + if settings.ANALYTICS_DASHBOARD_URL: + analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) + link = "{1}".format(analytics_dashboard_url, + settings.ANALYTICS_DASHBOARD_NAME) + return link + + def _section_analytics(course, access): """ Provide data for the corresponding dashboard section """ course_key = course.id @@ -366,10 +384,7 @@ def _section_analytics(course, access): } if settings.ANALYTICS_DASHBOARD_URL: - # Construct a URL to the external analytics dashboard - analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) - dashboard_link = "{1}".format(analytics_dashboard_url, - settings.ANALYTICS_DASHBOARD_NAME) + dashboard_link = _get_dashboard_link(course_key) message = _("Demographic data is now available in {dashboard_link}.").format(dashboard_link=dashboard_link) section_data['demographic_message'] = message diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 728b04777a..8bc58c742d 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -134,8 +134,8 @@ def instructor_dashboard(request, course_id): 'header': ['Statistic', 'Value'], 'title': _('Course Statistics At A Glance'), } - data = [['# Enrolled', enrollment_number]] - data += [['Date', timezone.now().isoformat()]] + + data = [['Date', timezone.now().isoformat()]] data += compute_course_stats(course).items() if request.user.is_staff: for field in course.fields.values(): @@ -938,11 +938,10 @@ def instructor_dashboard(request, course_id): "StudentsDailyActivity", # active students by day "StudentsDropoffPerDay", # active students dropoff by day # "OverallGradeDistribution", # overall point distribution for course - "StudentsActive", # num students active in time period (default = 1wk) - "StudentsEnrolled", # num students enrolled # "StudentsPerProblemCorrect", # foreach problem, num students correct "ProblemGradeDistribution", # foreach problem, grade distribution ] + for analytic_name in DASHBOARD_ANALYTICS: analytics_results[analytic_name] = get_analytics_result(analytic_name) diff --git a/lms/envs/common.py b/lms/envs/common.py index f3c427a937..fdf73a86c4 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -286,7 +286,10 @@ FEATURES = { 'ALLOW_AUTOMATED_SIGNUPS': False, # Display demographic data on the analytics tab in the instructor dashboard. - 'DISPLAY_ANALYTICS_DEMOGRAPHICS': True + 'DISPLAY_ANALYTICS_DEMOGRAPHICS': True, + + # Enable display of enrollment counts in instructor and legacy analytics dashboard + 'DISPLAY_ANALYTICS_ENROLLMENTS': True, } # Ignore static asset files on import which match this pattern diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 606b5fbc01..70db13b30a 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -609,22 +609,6 @@ function goto( mode)

${_("No Analytics are available at this time.")}

%endif - %if analytics_results.get("StudentsEnrolled"): -

- ${_("Students enrolled (historical count, includes those who have since unenrolled):")} - ${analytics_results["StudentsEnrolled"]['data'][0]['students']} - (${analytics_results["StudentsEnrolled"]['time']}) -

- %endif - - %if analytics_results.get("StudentsActive"): -

- ${_("Students active in the last week:")} - ${analytics_results["StudentsActive"]['data'][0]['active']} - (${analytics_results["StudentsActive"]['time']}) -

- %endif - %if analytics_results.get("StudentsDropoffPerDay"):

${_("Student activity day by day")} diff --git a/lms/templates/instructor/instructor_dashboard_2/course_info.html b/lms/templates/instructor/instructor_dashboard_2/course_info.html index bd6462934f..f71fb53646 100644 --- a/lms/templates/instructor/instructor_dashboard_2/course_info.html +++ b/lms/templates/instructor/instructor_dashboard_2/course_info.html @@ -1,28 +1,35 @@ <%! from django.utils.translation import ugettext as _ %> <%page args="section_data"/> -

-

${_("Enrollment Information")}

- ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit') - ${_("Number of enrollees (instructors, staff members, and students) by track")} -

- <% modes = section_data['enrollment_count'] %> - - - - - - - - - - - - - -
${_("Verified")}${modes['verified']}
${_("Audit")}${modes['audit']}
${_("Honor")}${modes['honor']}
${_("Total")}${modes['total']}
-
-
+%if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS') or section_data.get('enrollment_message'): +
+

${_("Enrollment Information")}

+ + %if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'): + ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit') + ${_("Number of enrollees (instructors, staff members, and students) by track")} +

+ <% modes = section_data['enrollment_count'] %> + + + + + + + + + + + + + +
${_("Verified")}${modes['verified']}
${_("Audit")}${modes['audit']}
${_("Honor")}${modes['honor']}
${_("Total")}${modes['total']}
+ %elif section_data.get('enrollment_message'): +

${section_data['enrollment_message']}

+ %endif +
+
+%endif

${_("Basic Course Information")}

From 66b3e3828f034fca695676d2a1f481d10f240f3c Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Wed, 5 Nov 2014 17:14:33 -0500 Subject: [PATCH 2/3] added tests for instructor_dashboard.py, added docstring for _is_small_course, removed check for settings.ANALYTICS_DASHBOARD_URL when creating dashboard link --- .../tests/test_instructor_dashboard.py | 125 ++++++++++++++++++ .../instructor/views/instructor_dashboard.py | 9 +- 2 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 lms/djangoapps/instructor/tests/test_instructor_dashboard.py diff --git a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/test_instructor_dashboard.py new file mode 100644 index 0000000000..957c577433 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_instructor_dashboard.py @@ -0,0 +1,125 @@ +""" +Unit tests for instructor_dashboard.py. +""" +from mock import patch + +from django.conf import settings +from django.core.urlresolvers import reverse +from django.test.utils import override_settings +from courseware.tests.helpers import LoginEnrollmentTestCase + +from student.tests.factories import AdminFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): + """ + Tests for the instructor dashboard (not legacy). + """ + + def setUp(self): + """ + Set up tests + """ + self.course = CourseFactory.create() + + # Create instructor account + instructor = AdminFactory.create() + self.client.login(username=instructor.username, password="test") + + # URL for instructor dash + self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id.to_deprecated_string()}) + + def tearDown(self): + """ + Undo patches. + """ + patch.stopall() + + def get_dashboard_enrollment_message(self): + return 'Enrollment data is now available in Example.'.format(unicode(self.course.id)) + + def get_dashboard_demographic_message(self): + return 'Demographic data is now available in Example.'.format(unicode(self.course.id)) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + def test_no_enrollments(self): + """ + Test enrollment section is hidden. + """ + response = self.client.get(self.url) + # no enrollment information should be visible + self.assertFalse('

Enrollment Information

' in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': True}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + def test_show_enrollments_data(self): + """ + Test enrollment data is shown. + """ + response = self.client.get(self.url) + + # enrollment information visible + self.assertTrue('

Enrollment Information

' in response.content) + self.assertTrue('Verified' in response.content) + self.assertTrue('Audit' in response.content) + self.assertTrue('Honor' in response.content) + + # dashboard link hidden + self.assertFalse(self.get_dashboard_enrollment_message() in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_ENROLLMENTS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='http://example.com') + @override_settings(ANALYTICS_DASHBOARD_NAME='Example') + def test_show_dashboard_enrollment_message(self): + """ + Test enrollment dashboard message is shown and data is hidden. + """ + response = self.client.get(self.url) + + # enrollment information hidden + self.assertFalse('Verified' in response.content) + self.assertFalse('Audit' in response.content) + self.assertFalse('Honor' in response.content) + + # link to dashboard shown + expected_message = self.get_dashboard_enrollment_message() + self.assertTrue(expected_message in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_DEMOGRAPHICS': True}) + @override_settings(ANALYTICS_DASHBOARD_URL='') + @override_settings(ANALYTICS_DASHBOARD_NAME='') + def test_show_dashboard_demographic_data(self): + """ + Test enrollment demographic data is shown. + """ + response = self.client.get(self.url) + # demographic information displayed + self.assertTrue('data-feature="year_of_birth"' in response.content) + self.assertTrue('data-feature="gender"' in response.content) + self.assertTrue('data-feature="level_of_education"' in response.content) + + # dashboard link hidden + self.assertFalse(self.get_dashboard_demographic_message() in response.content) + + @patch.dict(settings.FEATURES, {'DISPLAY_ANALYTICS_DEMOGRAPHICS': False}) + @override_settings(ANALYTICS_DASHBOARD_URL='http://example.com') + @override_settings(ANALYTICS_DASHBOARD_NAME='Example') + def test_show_dashboard_demographic_message(self): + """ + Test enrollment demographic dashboard message is shown and data is hidden. + """ + response = self.client.get(self.url) + + # demographics are hidden + self.assertFalse('data-feature="year_of_birth"' in response.content) + self.assertFalse('data-feature="gender"' in response.content) + self.assertFalse('data-feature="level_of_education"' in response.content) + + # link to dashboard shown + expected_message = self.get_dashboard_demographic_message() + self.assertTrue(expected_message in response.content) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index f894304954..585b2fffb6 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -263,6 +263,7 @@ def _section_membership(course, access): def _is_small_course(course_key): + """ Compares against MAX_ENROLLMENT_INSTR_BUTTONS to determine if course enrollment is considered small. """ is_small_course = False enrollment_count = CourseEnrollment.num_enrolled_in(course_key) max_enrollment_for_buttons = settings.FEATURES.get("MAX_ENROLLMENT_INSTR_BUTTONS") @@ -364,11 +365,9 @@ def _section_send_email(course, access): def _get_dashboard_link(course_key): # Construct a URL to the external analytics dashboard - link = None - if settings.ANALYTICS_DASHBOARD_URL: - analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) - link = "{1}".format(analytics_dashboard_url, - settings.ANALYTICS_DASHBOARD_NAME) + analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) + link = "{1}".format(analytics_dashboard_url, + settings.ANALYTICS_DASHBOARD_NAME) return link From d3e051ce2973b05698e906ded8a2660779b10199 Mon Sep 17 00:00:00 2001 From: Dennis Jen Date: Thu, 6 Nov 2014 13:42:00 -0500 Subject: [PATCH 3/3] fixed quaility errors in instructor_dashboard.py, moved test_instructor_dashboard under tests/views directory --- .../tests/{ => views}/test_instructor_dashboard.py | 6 ++++++ lms/djangoapps/instructor/views/instructor_dashboard.py | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) rename lms/djangoapps/instructor/tests/{ => views}/test_instructor_dashboard.py (96%) diff --git a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py similarity index 96% rename from lms/djangoapps/instructor/tests/test_instructor_dashboard.py rename to lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 957c577433..535676a320 100644 --- a/lms/djangoapps/instructor/tests/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -38,10 +38,16 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): patch.stopall() def get_dashboard_enrollment_message(self): + """ + Returns expected dashboard enrollment message with link to Insights. + """ return 'Enrollment data is now available in Example.'.format(unicode(self.course.id)) def get_dashboard_demographic_message(self): + """ + Returns expected dashboard demographic message with link to Insights. + """ return 'Demographic data is now available in Example.'.format(unicode(self.course.id)) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 585b2fffb6..8495e0c5de 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -275,7 +275,7 @@ def _is_small_course(course_key): def _section_student_admin(course, access): """ Provide data for the corresponding dashboard section """ course_key = course.id - is_small_course =_is_small_course(course_key) + is_small_course = _is_small_course(course_key) section_data = { 'section_key': 'student_admin', @@ -364,7 +364,7 @@ def _section_send_email(course, access): def _get_dashboard_link(course_key): - # Construct a URL to the external analytics dashboard + """ Construct a URL to the external analytics dashboard """ analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key)) link = "{1}".format(analytics_dashboard_url, settings.ANALYTICS_DASHBOARD_NAME)