Merge pull request #5473 from edx/dan-f/background-student-profiles-add-cohort

Make student profile CSV download a background task, add cohort column

lms/djangoapps/instructor/features/common.py

@@ -92,9 +92,9 @@ def click_a_button(step, button):  # pylint: disable=unused-argument
 
         # Expect to see a message that grade report is being generated
         expected_msg = "Your grade report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section."
-        world.wait_for_visible('#grade-request-response')
+        world.wait_for_visible('#report-request-response')
         assert_in(
-            expected_msg, world.css_text('#grade-request-response'),
+            expected_msg, world.css_text('#report-request-response'),
             msg="Could not find grade report generation success message."
         )
 
@@ -113,7 +113,8 @@ def click_a_button(step, button):  # pylint: disable=unused-argument
     elif button == "Download profile information as a CSV":
         # Go to the data download section of the instructor dash
         go_to_section("data_download")
-        # Don't do anything else, next step will handle clicking & downloading
+
+        world.css_click('input[name="list-profiles-csv"]')
 
     else:
         raise ValueError("Unrecognized button option " + button)

lms/djangoapps/instructor/features/data_download.feature

@@ -44,3 +44,12 @@ Feature: LMS.Instructor Dash Data Download
        | Role          |
        | instructor    |
        | staff         |
+
+    Scenario: Generate & download a student profile report
+       Given I am "<Role>" for a course
+       When I click "Download profile information as a CSV"
+       Then I see a student profile csv file in the reports table
+       Examples:
+       | Role          |
+       | instructor    |
+       | staff         |

lms/djangoapps/instructor/features/data_download.py

@@ -68,14 +68,23 @@ length=0"""
     assert_in(expected_config, world.css_text('#data-grade-config-text'))
 
 
-@step(u"I see a grade report csv file in the reports table")
-def find_grade_report_csv_link(step):  # pylint: disable=unused-argument
-    # Need to reload the page to see the grades download table
+def verify_report_is_generated(report_name_substring):
+    # Need to reload the page to see the reports table updated
     reload_the_page(step)
     world.wait_for_visible('#report-downloads-table')
     # Find table and assert a .csv file is present
-    expected_file_regexp = 'edx_999_Test_Course_grade_report_\d{4}-\d{2}-\d{2}-\d{4}\.csv'
+    expected_file_regexp = 'edx_999_Test_Course_{0}_'.format(report_name_substring) + '\d{4}-\d{2}-\d{2}-\d{4}\.csv'
     assert_regexp_matches(
         world.css_html('#report-downloads-table'), expected_file_regexp,
-        msg="Expected grade report filename was not found."
+        msg="Expected report filename was not found."
     )
+
+
+@step(u"I see a grade report csv file in the reports table")
+def find_grade_report_csv_link(step):  # pylint: disable=unused-argument
+    verify_report_is_generated('grade_report')
+
+
+@step(u"I see a student profile csv file in the reports table")
+def find_student_profile_report_csv_link(step):  # pylint: disable=unused-argument
+    verify_report_is_generated('student_profile_info')

lms/djangoapps/instructor/tests/test_api.py

@@ -55,6 +55,22 @@ from ..views.tools import get_extended_due
 EXPECTED_CSV_HEADER = '"code","course_id","company_name","created_by","redeemed_by","invoice_id","purchaser","customer_reference_number","internal_reference"'
 EXPECTED_COUPON_CSV_HEADER = '"course_id","percentage_discount","code_redeemed_count","description"'
 
+# ddt data for test cases involving reports
+REPORTS_DATA = (
+    {
+        'report_type': 'grade',
+        'instructor_api_endpoint': 'calculate_grades_csv',
+        'task_api_endpoint': 'instructor_task.api.submit_calculate_grades_csv',
+        'extra_instructor_api_kwargs': {}
+    },
+    {
+        'report_type': 'enrolled student profile',
+        'instructor_api_endpoint': 'get_students_features',
+        'task_api_endpoint': 'instructor_task.api.submit_calculate_students_features_csv',
+        'extra_instructor_api_kwargs': {'csv': '/csv'}
+    }
+)
+
 
 @common_exceptions_400
 def view_success(request):  # pylint: disable=W0613
@@ -156,6 +172,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
             ('list_background_email_tasks', {}),
             ('list_report_downloads', {}),
             ('calculate_grades_csv', {}),
+            ('get_students_features', {}),
         ]
         # Endpoints that only Instructors can access
         self.instructor_level_endpoints = [
@@ -1343,6 +1360,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
             self.assertNotIn(rolename, user_roles)
 
 
+@ddt.ddt
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCase):
     """
@@ -1350,6 +1368,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
     """
 
     def setUp(self):
+        super(TestInstructorAPILevelsDataDump, self).setUp()
         self.course = CourseFactory.create()
         self.course_mode = CourseMode(course_id=self.course.id,
                                       mode_slug="honor",
@@ -1613,6 +1632,21 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
             self.assertEqual(student_json['username'], student.username)
             self.assertEqual(student_json['email'], student.email)
 
+    @ddt.data(True, False)
+    def test_get_students_features_cohorted(self, is_cohorted):
+        """
+        Test that get_students_features includes cohort info when the course is
+        cohorted, and does not when the course is not cohorted.
+        """
+        url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)})
+        self.course.cohort_config = {'cohorted': is_cohorted}
+        self.store.update_item(self.course, self.instructor.id)
+
+        response = self.client.get(url, {})
+        res_json = json.loads(response.content)
+
+        self.assertEqual('cohort' in res_json['feature_names'], is_cohorted)
+
     @patch.object(instructor.views.api, 'anonymous_id_for_user', Mock(return_value='42'))
     @patch.object(instructor.views.api, 'unique_id_for_user', Mock(return_value='41'))
     def test_get_anon_ids(self):
@@ -1625,9 +1659,9 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
         body = response.content.replace('\r', '')
         self.assertTrue(body.startswith(
             '"User ID","Anonymized User ID","Course Specific Anonymized User ID"'
-            '\n"2","41","42"\n'
+            '\n"3","41","42"\n'
         ))
-        self.assertTrue(body.endswith('"7","41","42"\n'))
+        self.assertTrue(body.endswith('"8","41","42"\n'))
 
     def test_list_report_downloads(self):
         url = reverse('list_report_downloads', kwargs={'course_id': self.course.id.to_deprecated_string()})
@@ -1655,33 +1689,31 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
         res_json = json.loads(response.content)
         self.assertEqual(res_json, expected_response)
 
-    def test_calculate_grades_csv_success(self):
-        url = reverse('calculate_grades_csv', kwargs={'course_id': self.course.id.to_deprecated_string()})
+    @ddt.data(*REPORTS_DATA)
+    @ddt.unpack
+    def test_calculate_report_csv_success(self, report_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs):
+        kwargs = {'course_id': unicode(self.course.id)}
+        kwargs.update(extra_instructor_api_kwargs)
+        url = reverse(instructor_api_endpoint, kwargs=kwargs)
 
-        with patch('instructor_task.api.submit_calculate_grades_csv') as mock_cal_grades:
-            mock_cal_grades.return_value = True
+        with patch(task_api_endpoint):
             response = self.client.get(url, {})
-        success_status = "Your grade report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section."
+        success_status = "Your {report_type} report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section.".format(report_type=report_type)
         self.assertIn(success_status, response.content)
 
-    def test_calculate_grades_csv_already_running(self):
-        url = reverse('calculate_grades_csv', kwargs={'course_id': self.course.id.to_deprecated_string()})
+    @ddt.data(*REPORTS_DATA)
+    @ddt.unpack
+    def test_calculate_report_csv_already_running(self, report_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs):
+        kwargs = {'course_id': unicode(self.course.id)}
+        kwargs.update(extra_instructor_api_kwargs)
+        url = reverse(instructor_api_endpoint, kwargs=kwargs)
 
-        with patch('instructor_task.api.submit_calculate_grades_csv') as mock_cal_grades:
-            mock_cal_grades.side_effect = AlreadyRunningError()
+        with patch(task_api_endpoint) as mock:
+            mock.side_effect = AlreadyRunningError()
             response = self.client.get(url, {})
-        already_running_status = "A grade report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below."
+        already_running_status = "{report_type} report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below.".format(report_type=report_type)
         self.assertIn(already_running_status, response.content)
 
-    def test_get_students_features_csv(self):
-        """
-        Test that some minimum of information is formatted
-        correctly in the response to get_students_features.
-        """
-        url = reverse('get_students_features', kwargs={'course_id': self.course.id.to_deprecated_string()})
-        response = self.client.get(url + '/csv', {})
-        self.assertEqual(response['Content-Type'], 'text/csv')
-
     def test_get_distribution_no_feature(self):
         """
         Test that get_distribution lists available features

lms/djangoapps/instructor/views/api.py

@@ -80,6 +80,7 @@ from .tools import (
     bulk_email_is_enabled_for_course,
     add_block_ids,
 )
+from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from opaque_keys import InvalidKeyError
 
@@ -690,7 +691,8 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=W06
 
     TO DO accept requests for different attribute sets.
     """
-    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+    course_key = CourseKey.from_string(course_id)
+    course = get_course_by_id(course_key)
 
     available_features = instructor_analytics.basic.AVAILABLE_FEATURES
 
@@ -704,8 +706,6 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=W06
             'goals'
         ]
 
-    student_data = instructor_analytics.basic.enrolled_students_features(course_id, query_features)
-
     # Provide human-friendly and translatable names for these features. These names
     # will be displayed in the table generated in data_download.coffee. It is not (yet)
     # used as the header row in the CSV, but could be in the future.
@@ -723,9 +723,15 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=W06
         'goals': _('Goals'),
     }
 
+    if course.is_cohorted:
+        # Translators: 'Cohort' refers to a group of students within a course.
+        query_features.append('cohort')
+        query_features_names['cohort'] = _('Cohort')
+
     if not csv:
+        student_data = instructor_analytics.basic.enrolled_students_features(course_key, query_features)
         response_payload = {
-            'course_id': course_id.to_deprecated_string(),
+            'course_id': unicode(course_key),
             'students': student_data,
             'students_count': len(student_data),
             'queried_features': query_features,
@@ -734,8 +740,13 @@ def get_students_features(request, course_id, csv=False):  # pylint: disable=W06
         }
         return JsonResponse(response_payload)
     else:
-        header, datarows = instructor_analytics.csvs.format_dictlist(student_data, query_features)
-        return instructor_analytics.csvs.create_csv_response("enrolled_profiles.csv", header, datarows)
+        try:
+            instructor_task.api.submit_calculate_students_features_csv(request, course_key, query_features)
+            success_status = _("Your enrolled student profile report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section.")
+            return JsonResponse({"status": success_status})
+        except AlreadyRunningError:
+            already_running_status = _("An enrolled student profile report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below.")
+            return JsonResponse({"status": already_running_status})
 
 
 @ensure_csrf_cookie

lms/djangoapps/instructor_analytics/basic.py

@@ -122,22 +122,27 @@ def purchase_transactions(course_id, features):
     return [purchase_transactions_info(purchased_course, features) for purchased_course in purchased_courses]
 
 
-def enrolled_students_features(course_id, features):
+def enrolled_students_features(course_key, features):
     """
     Return list of student features as dictionaries.
 
-    enrolled_students_features(course_id, ['username, first_name'])
+    enrolled_students_features(course_key, ['username', 'first_name'])
     would return [
         {'username': 'username1', 'first_name': 'firstname1'}
         {'username': 'username2', 'first_name': 'firstname2'}
         {'username': 'username3', 'first_name': 'firstname3'}
     ]
     """
+    include_cohort_column = 'cohort' in features
+
     students = User.objects.filter(
-        courseenrollment__course_id=course_id,
+        courseenrollment__course_id=course_key,
         courseenrollment__is_active=1,
     ).order_by('username').select_related('profile')
 
+    if include_cohort_column:
+        students = students.prefetch_related('course_groups')
+
     def extract_student(student, features):
         """ convert student to dictionary """
         student_features = [x for x in STUDENT_FEATURES if x in features]
@@ -151,6 +156,14 @@ def enrolled_students_features(course_id, features):
                                 for feature in profile_features)
             student_dict.update(profile_dict)
 
+        if include_cohort_column:
+            # Note that we use student.course_groups.all() here instead of
+            # student.course_groups.filter(). The latter creates a fresh query,
+            # therefore negating the performance gain from prefetch_related().
+            student_dict['cohort'] = next(
+                (cohort.name for cohort in student.course_groups.all() if cohort.course_id == course_key),
+                "[unassigned]"
+            )
         return student_dict
 
     return [extract_student(student, features) for student in students]

lms/djangoapps/instructor_analytics/tests/test_basic.py

@@ -13,15 +13,19 @@ from instructor_analytics.basic import (
     sale_record_features, enrolled_students_features, course_registration_features, coupon_codes_features,
     AVAILABLE_FEATURES, STUDENT_FEATURES, PROFILE_FEATURES
 )
+from course_groups.tests.helpers import CohortFactory
+from course_groups.models import CourseUserGroup
 from courseware.tests.factories import InstructorFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
-class TestAnalyticsBasic(TestCase):
+class TestAnalyticsBasic(ModuleStoreTestCase):
     """ Test basic analytics functions. """
 
     def setUp(self):
+        super(TestAnalyticsBasic, self).setUp()
         self.course_key = SlashSeparatedCourseKey('robot', 'course', 'id')
         self.users = tuple(UserFactory() for _ in xrange(30))
         self.ces = tuple(CourseEnrollment.enroll(user, self.course_key)
@@ -40,7 +44,8 @@ class TestAnalyticsBasic(TestCase):
         query_features = ('username', 'name', 'email')
         for feature in query_features:
             self.assertIn(feature, AVAILABLE_FEATURES)
-        userreports = enrolled_students_features(self.course_key, query_features)
+        with self.assertNumQueries(1):
+            userreports = enrolled_students_features(self.course_key, query_features)
         self.assertEqual(len(userreports), len(self.users))
         for userreport in userreports:
             self.assertEqual(set(userreport.keys()), set(query_features))
@@ -48,6 +53,37 @@ class TestAnalyticsBasic(TestCase):
             self.assertIn(userreport['email'], [user.email for user in self.users])
             self.assertIn(userreport['name'], [user.profile.name for user in self.users])
 
+    def test_enrolled_students_features_keys_cohorted(self):
+        course = CourseFactory.create(course_key=self.course_key)
+        course.cohort_config = {'cohorted': True, 'auto_cohort': True, 'auto_cohort_groups': ['cohort']}
+        self.store.update_item(course, self.instructor.id)
+        cohort = CohortFactory.create(name='cohort', course_id=course.id)
+        cohorted_students = [UserFactory.create() for _ in xrange(10)]
+        cohorted_usernames = [student.username for student in cohorted_students]
+        non_cohorted_student = UserFactory.create()
+        for student in cohorted_students:
+            cohort.users.add(student)
+            CourseEnrollment.enroll(student, course.id)
+        CourseEnrollment.enroll(non_cohorted_student, course.id)
+        instructor = InstructorFactory(course_key=course.id)
+        self.client.login(username=instructor.username, password='test')
+
+        query_features = ('username', 'cohort')
+        # There should be a constant of 2 SQL queries when calling
+        # enrolled_students_features.  The first query comes from the call to
+        # User.objects.filter(...), and the second comes from
+        # prefetch_related('course_groups').
+        with self.assertNumQueries(2):
+            userreports = enrolled_students_features(course.id, query_features)
+        self.assertEqual(len([r for r in userreports if r['username'] in cohorted_usernames]), len(cohorted_students))
+        self.assertEqual(len([r for r in userreports if r['username'] == non_cohorted_student.username]), 1)
+        for report in userreports:
+            self.assertEqual(set(report.keys()), set(query_features))
+            if report['username'] in cohorted_usernames:
+                self.assertEqual(report['cohort'], cohort.name)
+            else:
+                self.assertEqual(report['cohort'], '[unassigned]')
+
     def test_available_features(self):
         self.assertEqual(len(AVAILABLE_FEATURES), len(STUDENT_FEATURES + PROFILE_FEATURES))
         self.assertEqual(set(AVAILABLE_FEATURES), set(STUDENT_FEATURES + PROFILE_FEATURES))
@@ -59,6 +95,7 @@ class TestCourseSaleRecordsAnalyticsBasic(ModuleStoreTestCase):
         """
         Fixtures.
         """
+        super(TestCourseSaleRecordsAnalyticsBasic, self).setUp()
         self.course = CourseFactory.create()
         self.instructor = InstructorFactory(course_key=self.course.id)
         self.client.login(username=self.instructor.username, password='test')

lms/djangoapps/instructor_task/api.py

@@ -17,7 +17,8 @@ from instructor_task.tasks import (rescore_problem,
                                    reset_problem_attempts,
                                    delete_problem_state,
                                    send_bulk_course_email,
-                                   calculate_grades_csv)
+                                   calculate_grades_csv,
+                                   calculate_students_features_csv)
 
 from instructor_task.api_helper import (check_arguments_for_rescoring,
                                         encode_problem_and_student_input,
@@ -218,3 +219,17 @@ def submit_calculate_grades_csv(request, course_key):
     task_key = ""
 
     return submit_task(request, task_type, task_class, course_key, task_input, task_key)
+
+
+def submit_calculate_students_features_csv(request, course_key, features):
+    """
+    Submits a task to generate a CSV containing student profile info.
+
+    Raises AlreadyRunningError if said CSV is already being updated.
+    """
+    task_type = 'profile_info_csv'
+    task_class = calculate_students_features_csv
+    task_input = {'features': features}
+    task_key = ""
+
+    return submit_task(request, task_type, task_class, course_key, task_input, task_key)

lms/djangoapps/instructor_task/tasks.py

@@ -31,6 +31,7 @@ from instructor_task.tasks_helper import (
     reset_attempts_module_state,
     delete_problem_module_state,
     push_grades_to_s3,
+    push_students_csv_to_s3
 )
 from bulk_email.tasks import perform_delegate_email_batches
 
@@ -136,6 +137,19 @@ def calculate_grades_csv(entry_id, xmodule_instance_args):
     """
     Grade a course and push the results to an S3 bucket for download.
     """
+    # Translators: This is a past-tense verb that is inserted into task progress messages as {action}.
     action_name = ugettext_noop('graded')
     task_fn = partial(push_grades_to_s3, xmodule_instance_args)
     return run_main_task(entry_id, task_fn, action_name)
+
+
+@task(base=BaseInstructorTask, routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY)  # pylint: disable=E1102
+def calculate_students_features_csv(entry_id, xmodule_instance_args):
+    """
+    Compute student profile information for a course and upload the
+    CSV to an S3 bucket for download.
+    """
+    # Translators: This is a past-tense verb that is inserted into task progress messages as {action}.
+    action_name = ugettext_noop('generated')
+    task_fn = partial(push_students_csv_to_s3, xmodule_instance_args)
+    return run_main_task(entry_id, task_fn, action_name)

lms/djangoapps/instructor_task/tasks_helper.py

@@ -23,6 +23,8 @@ from courseware.grades import iterate_grades_for
 from courseware.models import StudentModule
 from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module_for_descriptor_internal
+from instructor_analytics.basic import enrolled_students_features
+from instructor_analytics.csvs import format_dictlist
 from instructor_task.models import ReportStore, InstructorTask, PROGRESS
 from student.models import CourseEnrollment
 
@@ -477,6 +479,32 @@ def delete_problem_module_state(xmodule_instance_args, _module_descriptor, stude
     return UPDATE_STATUS_SUCCEEDED
 
 
+def upload_csv_to_report_store(rows, csv_name, course_id, timestamp):
+    """
+    Upload data as a CSV using ReportStore.
+
+    Arguments:
+        rows: CSV data in the following format (first column may be a
+            header):
+            [
+                [row1_colum1, row1_colum2, ...],
+                ...
+            ]
+        csv_name: Name of the resulting CSV
+        course_id: ID of the course
+    """
+    report_store = ReportStore.from_config()
+    report_store.store_rows(
+        course_id,
+        u"{course_prefix}_{csv_name}_{timestamp_str}.csv".format(
+            course_prefix=urllib.quote(unicode(course_id).replace("/", "_")),
+            csv_name=csv_name,
+            timestamp_str=timestamp.strftime("%Y-%m-%d-%H%M")
+        ),
+        rows
+    )
+
+
 def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input, action_name):
     """
     For a given `course_id`, generate a grades CSV file for all students that
@@ -557,25 +585,30 @@ def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input,
     curr_step = "Uploading CSVs"
     update_task_progress()
 
-    # Generate parts of the file name
-    timestamp_str = start_time.strftime("%Y-%m-%d-%H%M")
-    course_id_prefix = urllib.quote(course_id.to_deprecated_string().replace("/", "_"))
-
     # Perform the actual upload
-    report_store = ReportStore.from_config()
-    report_store.store_rows(
-        course_id,
-        u"{}_grade_report_{}.csv".format(course_id_prefix, timestamp_str),
-        rows
-    )
+    upload_csv_to_report_store(rows, 'grade_report', course_id, start_time)
 
     # If there are any error rows (don't count the header), write them out as well
     if len(err_rows) > 1:
-        report_store.store_rows(
-            course_id,
-            u"{}_grade_report_{}_err.csv".format(course_id_prefix, timestamp_str),
-            err_rows
-        )
+        upload_csv_to_report_store(err_rows, 'grade_report_err', course_id, start_time)
 
     # One last update before we close out...
     return update_task_progress()
+
+
+def push_students_csv_to_s3(_xmodule_instance_args, _entry_id, course_id, task_input, _action_name):
+    """
+    For a given `course_id`, generate a CSV file containing profile
+    information for all students that are enrolled, and store using a
+    `ReportStore`.
+    """
+    # compute the student features table and format it
+    query_features = task_input.get('features')
+    student_data = enrolled_students_features(course_id, query_features)
+    header, rows = format_dictlist(student_data, query_features)
+    rows.insert(0, header)
+
+    # Perform the upload
+    upload_csv_to_report_store(rows, 'student_profile_info', course_id, datetime.now(UTC))
+
+    return UPDATE_STATUS_SUCCEEDED

lms/djangoapps/instructor_task/tests/test_api.py

@@ -15,6 +15,7 @@ from instructor_task.api import (
     submit_reset_problem_attempts_for_all_students,
     submit_delete_problem_state_for_all_students,
     submit_bulk_course_email,
+    submit_calculate_students_features_csv,
 )
 
 from instructor_task.api_helper import AlreadyRunningError
@@ -170,14 +171,34 @@ class InstructorTaskCourseSubmitTest(InstructorTaskCourseTestCase):
         course_email = CourseEmail.create(self.course.id, self.instructor, SEND_TO_ALL, "Test Subject", "<p>This is a test message</p>")
         return course_email.id  # pylint: disable=E1101
 
-    def test_submit_bulk_email_all(self):
-        email_id = self._define_course_email()
-        instructor_task = submit_bulk_course_email(self.create_task_request(self.instructor), self.course.id, email_id)
-
-        # test resubmitting, by updating the existing record:
+    def _test_resubmission(self, api_call):
+        """
+        Tests the resubmission of an instructor task through the API.
+        The call to the API is a lambda expression passed via
+        `api_call`.  Expects that the API call returns the resulting
+        InstructorTask object, and that its resubmission raises
+        `AlreadyRunningError`.
+        """
+        instructor_task = api_call()
         instructor_task = InstructorTask.objects.get(id=instructor_task.id)  # pylint: disable=E1101
         instructor_task.task_state = PROGRESS
         instructor_task.save()
-
         with self.assertRaises(AlreadyRunningError):
-            instructor_task = submit_bulk_course_email(self.create_task_request(self.instructor), self.course.id, email_id)
+            api_call()
+
+    def test_submit_bulk_email_all(self):
+        email_id = self._define_course_email()
+        api_call = lambda: submit_bulk_course_email(
+            self.create_task_request(self.instructor),
+            self.course.id,
+            email_id
+        )
+        self._test_resubmission(api_call)
+
+    def test_submit_calculate_students_features(self):
+        api_call = lambda: submit_calculate_students_features_csv(
+            self.create_task_request(self.instructor),
+            self.course.id,
+            features=[]
+        )
+        self._test_resubmission(api_call)

lms/djangoapps/instructor_task/tests/test_tasks_helper.py

@@ -13,32 +13,32 @@ from mock import Mock, patch
 from django.conf import settings
 from django.test.testcases import TestCase
 
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 
-from instructor_task.tasks_helper import push_grades_to_s3
+from instructor_task.models import ReportStore
+from instructor_task.tasks_helper import push_grades_to_s3, push_students_csv_to_s3, UPDATE_STATUS_SUCCEEDED
 
 
-TEST_COURSE_ORG = 'edx'
-TEST_COURSE_NAME = 'test_course'
-TEST_COURSE_NUMBER = '1.23x'
-
-
-@ddt.ddt
-class TestInstructorGradeReport(TestCase):
+class TestReport(ModuleStoreTestCase):
     """
-    Tests that CSV grade report generation works.
+    Base class for testing CSV download tasks.
     """
     def setUp(self):
-        self.course = CourseFactory.create(org=TEST_COURSE_ORG,
-                                           number=TEST_COURSE_NUMBER,
-                                           display_name=TEST_COURSE_NAME)
+        self.course = CourseFactory.create()
 
     def tearDown(self):
         if os.path.exists(settings.GRADES_DOWNLOAD['ROOT_PATH']):
             shutil.rmtree(settings.GRADES_DOWNLOAD['ROOT_PATH'])
 
+
+@ddt.ddt
+class TestInstructorGradeReport(TestReport):
+    """
+    Tests that CSV grade report generation works.
+    """
     def create_student(self, username, email):
         student = UserFactory.create(username=username, email=email)
         CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
@@ -58,3 +58,18 @@ class TestInstructorGradeReport(TestCase):
             result = push_grades_to_s3(None, None, self.course.id, None, 'graded')
         #This assertion simply confirms that the generation completed with no errors
         self.assertEquals(result['succeeded'], result['attempted'])
+
+
+class TestStudentReport(TestReport):
+    """
+    Tests that CSV student profile report generation works.
+    """
+    def test_success(self):
+        task_input = {'features': []}
+        with patch('instructor_task.tasks_helper._get_current_task'):
+            result = push_students_csv_to_s3(None, None, self.course.id, task_input, 'calculated')
+        report_store = ReportStore.from_config()
+        links = report_store.links_for(self.course.id)
+
+        self.assertEquals(len(links), 1)
+        self.assertEquals(result, UPDATE_STATUS_SUCCEEDED)

lms/static/coffee/src/instructor_dashboard/data_download.coffee

@@ -26,11 +26,11 @@ class DataDownload
     # response areas
     @$download                        = @$section.find '.data-download-container'
     @$download_display_text           = @$download.find '.data-display-text'
-    @$download_display_table          = @$download.find '.data-display-table'
     @$download_request_response_error = @$download.find '.request-response-error'
-    @$grades                        = @$section.find '.grades-download-container'
-    @$grades_request_response       = @$grades.find '.request-response'
-    @$grades_request_response_error = @$grades.find '.request-response-error'
+    @$reports                         = @$section.find '.reports-download-container'
+    @$download_display_table          = @$reports.find '.data-display-table'
+    @$reports_request_response        = @$reports.find '.request-response'
+    @$reports_request_response_error  = @$reports.find '.request-response-error'
 
     @report_downloads = new ReportDownloads(@$section)
     @instructor_tasks = new (PendingInstructorTasks()) @$section
@@ -45,11 +45,22 @@ class DataDownload
     # this handler binds to both the download
     # and the csv button
     @$list_studs_csv_btn.click (e) =>
+      @clear_display()
+
       url = @$list_studs_csv_btn.data 'endpoint'
       # handle csv special case
       # redirect the document to the csv file.
       url += '/csv'
-      location.href = url
+
+      $.ajax
+        dataType: 'json'
+        url: url
+        error: (std_ajax_err) =>
+          @$reports_request_response_error.text gettext("Error generating student profile information. Please try again.")
+          $(".msg-error").css({"display":"block"})
+        success: (data) =>
+          @$reports_request_response.text data['status']
+          $(".msg-confirm").css({"display":"block"})
 
     @$list_studs_btn.click (e) =>
       url = @$list_studs_btn.data 'endpoint'
@@ -62,7 +73,7 @@ class DataDownload
       $.ajax
         dataType: 'json'
         url: url
-        error: std_ajax_err =>
+        error: (std_ajax_err) =>
           @clear_display()
           @$download_request_response_error.text gettext("Error getting student list.")
         success: (data) =>
@@ -89,7 +100,7 @@ class DataDownload
       $.ajax
         dataType: 'json'
         url: url
-        error: std_ajax_err =>
+        error: (std_ajax_err) =>
           @clear_display()
           @$download_request_response_error.text gettext("Error retrieving grading configuration.")
         success: (data) =>
@@ -105,11 +116,11 @@ class DataDownload
       $.ajax
         dataType: 'json'
         url: url
-        error: std_ajax_err =>
-          @$grades_request_response_error.text gettext("Error generating grades. Please try again.")
+        error: (std_ajax_err) =>
+          @$reports_request_response_error.text gettext("Error generating grades. Please try again.")
           $(".msg-error").css({"display":"block"})
         success: (data) =>
-          @$grades_request_response.text data['status']
+          @$reports_request_response.text data['status']
           $(".msg-confirm").css({"display":"block"})
 
   # handler for when the section title is clicked.
@@ -129,8 +140,8 @@ class DataDownload
     @$download_display_text.empty()
     @$download_display_table.empty()
     @$download_request_response_error.empty()
-    @$grades_request_response.empty()
-    @$grades_request_response_error.empty()
+    @$reports_request_response.empty()
+    @$reports_request_response_error.empty()
     # Clear any CSS styling from the request-response areas
     $(".msg-confirm").css({"display":"none"})
     $(".msg-error").css({"display":"none"})
@@ -157,7 +168,7 @@ class ReportDownloads
           @create_report_downloads_table data.downloads
         else
           console.log "No reports ready for download"
-      error: std_ajax_err => console.error "Error finding report downloads"
+      error: (std_ajax_err) => console.error "Error finding report downloads"
 
   create_report_downloads_table: (report_downloads_data) ->
     @$report_downloads_table.empty()

lms/static/js/views/cohorts.js

@@ -7,7 +7,8 @@
             'change .cohort-select': 'onCohortSelected',
             'click .action-create': 'showAddCohortForm',
             'click .action-cancel': 'cancelAddCohortForm',
-            'click .action-save': 'saveAddCohortForm'
+            'click .action-save': 'saveAddCohortForm',
+            'click .link-cross-reference': 'showSection'
         },
 
         initialize: function(options) {
@@ -170,6 +171,13 @@
             event.preventDefault();
             this.removeNotification();
             this.onSync();
+        },
+
+        showSection: function(event) {
+            event.preventDefault();
+            var section = $(event.currentTarget).data("section");
+            $(".instructor-nav .nav-item a[data-section='" + section + "']").click();
+            $(window).scrollTop(0);
         }
     });
 }).call(this, $, _, Backbone, gettext, interpolate_text, CohortEditorView, NotificationModel, NotificationView);

lms/static/sass/course/instructor/_instructor_2.scss

@@ -894,15 +894,12 @@ section.instructor-dashboard-content-2 {
     line-height: 1.3em;
   }
 
-  .data-download-container {
+  .reports-download-container {
     .data-display-table {
       .slickgrid {
         height: 400px;
       }
     }
-  }
-
-  .grades-download-container {
     .report-downloads-table {
       .slickgrid {
         height: 300px;

lms/templates/instructor/instructor_dashboard_2/cohorts.underscore

@@ -25,3 +25,14 @@
 
 <!-- individual group -->
 <div class="cohort-management-group"></div>
+
+<div class="cohort-management-supplemental">
+    <p class="">
+        <i class="icon icon-info-sign"></i>
+        <%= interpolate(
+        gettext('To review all student cohort group assignments, download course profile information on %(link_start)s the Data Download page. %(link_end)s'),
+            {link_start: '<a href="" class="link-cross-reference" data-section="data_download">', link_end: '</a>'},
+            true
+        ) %>
+    </p>
+</div>

lms/templates/instructor/instructor_dashboard_2/data_download.html

@@ -6,16 +6,6 @@
   <h2>${_("Data Download")}</h2>
   <div class="request-response-error msg msg-error copy" id="data-request-response-error"></div>
 
-  <p>${_("Click to generate a CSV file of all students enrolled in this course, along with profile information such as email address and username:")}</p>
-
-  <p><input type="button" name="list-profiles-csv" value="${_("Download profile information as a CSV")}" data-endpoint="${ section_data['get_students_features_url'] }" data-csv="true"></p>
-
-  % if not disable_buttons:
-    <p>${_("For smaller courses, click to list profile information for enrolled students directly on this page:")}</p>
-    <p><input type="button" name="list-profiles" value="${_("List enrolled students' profile information")}" data-endpoint="${ section_data['get_students_features_url'] }"></p>
-  %endif
-  <div class="data-display-table" id="data-student-profiles-table"></div>
-  
 
   <br>
   <p>${_("Click to display the grading configuration for the course. The grading configuration is the breakdown of graded subsections of the course (such as exams and problem sets), and can be changed on the 'Grading' page (under 'Settings') in Studio.")}</p>
@@ -29,27 +19,37 @@
 </div>
 
 %if settings.FEATURES.get('ENABLE_S3_GRADE_DOWNLOADS'):
-  <div class="grades-download-container action-type-container">
+  <div class="reports-download-container action-type-container">
     <hr>
     <h2> ${_("Reports")}</h2>
 
-  %if settings.FEATURES.get('ALLOW_COURSE_STAFF_GRADE_DOWNLOADS') or section_data['access']['admin']:
-    <p>${_("Click to generate a CSV grade report for all currently enrolled students.  Links to generated reports appear in a table below when report generation is complete.")}</p>
+    <p>${_("For large courses, generating some reports can take several hours. When report generation is complete, a link that includes the date and time of generation appears in the table below. These reports are generated in the background, meaning it is OK to navigate away from this page while your report is generating.")}</p>
 
-    <p>${_("For large courses, generating this report may take several hours. Please be patient and do not click the button multiple times. Clicking the button multiple times will significantly slow the grade generation process.")}</p>
+    <p>${_("Please be patient and do not click these buttons multiple times. Clicking these buttons multiple times will significantly slow the generation process.")}</p>
 
-    <p>${_("The report is generated in the background, meaning it is OK to navigate away from this page while your report is generating.")}</p>
+    <p>${_("Click to generate a CSV file of all students enrolled in this course, along with profile information such as email address and username:")}</p>
 
-    <div class="request-response msg msg-confirm copy" id="grade-request-response"></div>
-    <div class="request-response-error msg msg-warning copy" id="grade-request-response-error"></div>
-    <br>
+    <p><input type="button" name="list-profiles-csv" value="${_("Download profile information as a CSV")}" data-endpoint="${ section_data['get_students_features_url'] }" data-csv="true"></p>
+
+  % if not disable_buttons:
+    <p>${_("For smaller courses, click to list profile information for enrolled students directly on this page:")}</p>
+    <p><input type="button" name="list-profiles" value="${_("List enrolled students' profile information")}" data-endpoint="${ section_data['get_students_features_url'] }"></p>
+  %endif
+  <div class="data-display-table" id="data-student-profiles-table"></div>
+
+  %if settings.FEATURES.get('ALLOW_COURSE_STAFF_GRADE_DOWNLOADS') or section_data['access']['admin']:
+    <p>${_("Click to generate a CSV grade report for all currently enrolled students.")}</p>
 
     <p><input type="button" name="calculate-grades-csv" value="${_("Generate Grade Report")}" data-endpoint="${ section_data['calculate_grades_csv_url'] }"/></p>
   %endif
 
+    <div class="request-response msg msg-confirm copy" id="report-request-response"></div>
+    <div class="request-response-error msg msg-warning copy" id="report-request-response-error"></div>
+    <br>
+
     <p><b>${_("Reports Available for Download")}</b></p>
     <p>
-      ${_("The grade reports listed below are generated each time the <b>Generate Grade Report</b> button is clicked. A link to each grade report remains available on this page, identified by the UTC date and time of generation. Grade reports are not deleted, so you will always be able to access previously generated reports from this page.")}
+      ${_("The reports listed below are available for download. A link to every report remains available on this page, identified by the UTC date and time of generation. Reports are not deleted, so you will always be able to access previously generated reports from this page.")}
     </p>
 
   %if settings.FEATURES.get('ENABLE_ASYNC_ANSWER_DISTRIBUTION'):