Merge pull request #5838 from edx/dsjen/feature-flag-analytics-enrollments

Feature flagged enrollment counts on instructor dashboard.
2014-11-12 10:40:04 -05:00
parent 24039fb813 d3e051ce29
commit dd38f52358
6 changed files with 194 additions and 56 deletions
--- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
+++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py
@@ -0,0 +1,131 @@
+"""
+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):
+        """
+        Returns expected dashboard enrollment message with link to Insights.
+        """
+        return 'Enrollment data is now available in <a href="http://example.com/courses/{}" ' \
+               'target="_blank">Example</a>.'.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 <a href="http://example.com/courses/{}" ' \
+               'target="_blank">Example</a>.'.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('<h2>Enrollment Information</h2>' 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('<h2>Enrollment Information</h2>' in response.content)
+        self.assertTrue('<td>Verified</td>' in response.content)
+        self.assertTrue('<td>Audit</td>' in response.content)
+        self.assertTrue('<td>Honor</td>' 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('<td>Verified</td>' in response.content)
+        self.assertFalse('<td>Audit</td>' in response.content)
+        self.assertFalse('<td>Honor</td>' 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)
--- 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,20 @@ 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):
+    """ 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")
    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 +363,14 @@ def _section_send_email(course, access):
    return section_data


+def _get_dashboard_link(course_key):
+    """ Construct a URL to the external analytics dashboard """
+    analytics_dashboard_url = '{0}/courses/{1}'.format(settings.ANALYTICS_DASHBOARD_URL, unicode(course_key))
+    link = "<a href=\"{0}\" target=\"_blank\">{1}</a>".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 +383,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 = "<a href=\"{0}\" target=\"_blank\">{1}</a>".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

--- 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)

--- 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
--- a/lms/templates/courseware/instructor_dashboard.html
+++ b/lms/templates/courseware/instructor_dashboard.html
@@ -594,22 +594,6 @@ function goto( mode)
    <p>${_("No Analytics are available at this time.")}</p>
  %endif

-  %if analytics_results.get("StudentsEnrolled"):
-    <p>
-      ${_("Students enrolled (historical count, includes those who have since unenrolled):")}
-      ${analytics_results["StudentsEnrolled"]['data'][0]['students']}
-      (${analytics_results["StudentsEnrolled"]['time']})
-    </p>
-  %endif
-
-  %if analytics_results.get("StudentsActive"):
-    <p>
-      ${_("Students active in the last week:")}
-      ${analytics_results["StudentsActive"]['data'][0]['active']}
-      (${analytics_results["StudentsActive"]['time']})
-    </p>
-  %endif
-
  %if analytics_results.get("StudentsDropoffPerDay"):
    <p>
      ${_("Student activity day by day")}
--- 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"/>

-<div class="enrollment-wrapper">
-  <h2>${_("Enrollment Information")}</h2>
-  ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit')
-  <span class="tip">${_("Number of enrollees (instructors, staff members, and students) by track")}</span>
-  <br/><br/>
-  <% modes = section_data['enrollment_count'] %>
-  <table>
-    <tr>
-      <td>${_("Verified")}</td><td>${modes['verified']}</td>
-    </tr>
-    <tr>
-      <td>${_("Audit")}</td><td>${modes['audit']}</td>
-    </tr>
-    <tr>
-      <td>${_("Honor")}</td><td>${modes['honor']}</td>
-    </tr>
-    <tr style="color:green;border-top:1px solid #000">
-      <td style="padding-top:10px;"><b>${_("Total")}</b></td><td style="padding-top:10px;"><b>${modes['total']}</b></td>
-    </tr>
-  </table>
-</div>
-<hr>
+%if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS') or section_data.get('enrollment_message'):
+  <div class="enrollment-wrapper">
+    <h2>${_("Enrollment Information")}</h2>
+
+    %if settings.FEATURES.get('DISPLAY_ANALYTICS_ENROLLMENTS'):
+      ## Translators: 'track' refers to the enrollment type ('honor', 'verified', or 'audit')
+      <span class="tip">${_("Number of enrollees (instructors, staff members, and students) by track")}</span>
+      <br/><br/>
+      <% modes = section_data['enrollment_count'] %>
+      <table>
+        <tr>
+          <td>${_("Verified")}</td><td>${modes['verified']}</td>
+        </tr>
+        <tr>
+          <td>${_("Audit")}</td><td>${modes['audit']}</td>
+        </tr>
+        <tr>
+          <td>${_("Honor")}</td><td>${modes['honor']}</td>
+        </tr>
+        <tr style="color:green;border-top:1px solid #000">
+          <td style="padding-top:10px;"><b>${_("Total")}</b></td><td style="padding-top:10px;"><b>${modes['total']}</b></td>
+        </tr>
+      </table>
+      %elif section_data.get('enrollment_message'):
+        <p>${section_data['enrollment_message']}</p>
+      %endif
+  </div>
+  <hr>
+%endif

 <div class="basic-wrapper">
  <h2>${_("Basic Course Information")}</h2>