From 2fafaec053fa01e32b7607f276a338d78e5e6b95 Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Fri, 24 Apr 2015 17:06:45 -0400 Subject: [PATCH] Implement grade report analytics TNL-1988 --- common/djangoapps/util/model_utils.py | 2 - .../pages/lms/instructor_dashboard.py | 39 ++++- .../discussion/test_cohort_management.py | 5 +- .../lms/test_lms_instructor_dashboard.py | 145 ++++++++++++++++-- lms/djangoapps/course_structure_api/v0/api.py | 3 +- lms/djangoapps/instructor_task/tasks.py | 1 - .../instructor_task/tasks_helper.py | 9 ++ .../instructor_dashboard/data_download.coffee | 15 +- 8 files changed, 189 insertions(+), 30 deletions(-) diff --git a/common/djangoapps/util/model_utils.py b/common/djangoapps/util/model_utils.py index 4c9e7b9c77..cbd67a5204 100644 --- a/common/djangoapps/util/model_utils.py +++ b/common/djangoapps/util/model_utils.py @@ -4,8 +4,6 @@ Utilities for django models. from eventtracking import tracker from django.conf import settings -from django.core.exceptions import ObjectDoesNotExist -from django.db.models.fields.related import RelatedField from django_countries.fields import Country diff --git a/common/test/acceptance/pages/lms/instructor_dashboard.py b/common/test/acceptance/pages/lms/instructor_dashboard.py index c1d1c4c043..525b5cd88e 100644 --- a/common/test/acceptance/pages/lms/instructor_dashboard.py +++ b/common/test/acceptance/pages/lms/instructor_dashboard.py @@ -702,12 +702,47 @@ class DataDownloadPage(PageObject): def is_browser_on_page(self): return self.q(css='a[data-section=data_download].active-section').present + @property + def generate_student_profile_report_button(self): + """ + Returns the "Download profile information as a CSV" button. + """ + return self.q(css='input[name=list-profiles-csv]') + + @property + def generate_grade_report_button(self): + """ + Returns the "Generate Grade Report" button. + """ + return self.q(css='input[name=calculate-grades-csv]') + + @property + def generate_weighted_problem_grade_report_button(self): + """ + Returns the "Generate Weighted Problem Grade Report" button. + """ + return self.q(css='input[name=problem-grade-report]') + + @property + def report_download_links(self): + """ + Returns the download links for the current page. + """ + return self.q(css="#report-downloads-table .file-download-link>a") + + def wait_for_available_report(self): + """ + Waits for a downloadable report to be available. + """ + EmptyPromise( + lambda: len(self.report_download_links) >= 1, 'Waiting for downloadable report' + ).fulfill() + def get_available_reports_for_download(self): """ Returns a list of all the available reports for download. """ - reports = self.q(css="#report-downloads-table .file-download-link>a").map(lambda el: el.text) - return reports.results + return self.report_download_links.map(lambda el: el.text) class StudentAdminPage(PageObject): diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py index ea9bd93176..921f471dbf 100644 --- a/common/test/acceptance/tests/discussion/test_cohort_management.py +++ b/common/test/acceptance/tests/discussion/test_cohort_management.py @@ -14,7 +14,6 @@ from xmodule.partitions.partitions import Group from ...fixtures.course import CourseFixture, XBlockFixtureDesc from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.instructor_dashboard import InstructorDashboardPage, DataDownloadPage -from ...pages.studio.settings_advanced import AdvancedSettingsPage from ...pages.studio.settings_group_configurations import GroupConfigurationsPage import uuid @@ -555,9 +554,7 @@ class CohortConfigurationTest(EventsTestMixin, UniqueCourseTest, CohortTestMixin # Verify the results can be downloaded. data_download = self.instructor_dashboard_page.select_data_download() - EmptyPromise( - lambda: 1 == len(data_download.get_available_reports_for_download()), 'Waiting for downloadable report' - ).fulfill() + data_download.wait_for_available_report() report = data_download.get_available_reports_for_download()[0] base_file_name = "cohort_results_" self.assertIn("{}_{}".format( diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index fc78048288..b58299859f 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -5,15 +5,36 @@ End-to-end tests for the LMS Instructor Dashboard. from nose.plugins.attrib import attr -from ..helpers import UniqueCourseTest, get_modal_alert +from ..helpers import UniqueCourseTest, get_modal_alert, EventsTestMixin from ...pages.common.logout import LogoutPage from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.instructor_dashboard import InstructorDashboardPage from ...fixtures.course import CourseFixture +class BaseInstructorDashboardTest(EventsTestMixin, UniqueCourseTest): + """ + Mixin class for testing the instructor dashboard. + """ + def log_in_as_instructor(self): + """ + Logs in as an instructor and returns the id. + """ + username = "test_instructor_{uuid}".format(uuid=self.unique_id[0:6]) + auto_auth_page = AutoAuthPage(self.browser, username=username, course_id=self.course_id, staff=True) + return username, auto_auth_page.visit().get_user_id() + + def visit_instructor_dashboard(self): + """ + Visits the instructor dashboard. + """ + instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) + instructor_dashboard_page.visit() + return instructor_dashboard_page + + @attr('shard_5') -class AutoEnrollmentWithCSVTest(UniqueCourseTest): +class AutoEnrollmentWithCSVTest(BaseInstructorDashboardTest): """ End-to-end tests for Auto-Registration and enrollment functionality via CSV file. """ @@ -21,13 +42,8 @@ class AutoEnrollmentWithCSVTest(UniqueCourseTest): def setUp(self): super(AutoEnrollmentWithCSVTest, self).setUp() self.course_fixture = CourseFixture(**self.course_info).install() - - # login as an instructor - AutoAuthPage(self.browser, course_id=self.course_id, staff=True).visit() - - # go to the membership page on the instructor dashboard - instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) - instructor_dashboard_page.visit() + self.log_in_as_instructor() + instructor_dashboard_page = self.visit_instructor_dashboard() self.auto_enroll_section = instructor_dashboard_page.select_membership().select_auto_enroll_section() def test_browse_and_upload_buttons_are_visible(self): @@ -91,7 +107,7 @@ class AutoEnrollmentWithCSVTest(UniqueCourseTest): @attr('shard_5') -class EntranceExamGradeTest(UniqueCourseTest): +class EntranceExamGradeTest(BaseInstructorDashboardTest): """ Tests for Entrance exam specific student grading tasks. """ @@ -112,13 +128,9 @@ class EntranceExamGradeTest(UniqueCourseTest): LogoutPage(self.browser).visit() - # login as an instructor - AutoAuthPage(self.browser, course_id=self.course_id, staff=True).visit() - # go to the student admin page on the instructor dashboard - instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) - instructor_dashboard_page.visit() - self.student_admin_section = instructor_dashboard_page.select_student_admin() + self.log_in_as_instructor() + self.student_admin_section = self.visit_instructor_dashboard().select_student_admin() def test_input_text_and_buttons_are_visible(self): """ @@ -291,3 +303,104 @@ class EntranceExamGradeTest(UniqueCourseTest): self.student_admin_section.set_student_email(self.student_identifier) self.student_admin_section.click_task_history_button() self.assertTrue(self.student_admin_section.is_background_task_history_table_visible()) + + +class DataDownloadsTest(BaseInstructorDashboardTest): + """ + Bok Choy tests for the "Data Downloads" tab. + """ + def setUp(self): + super(DataDownloadsTest, self).setUp() + self.course_fixture = CourseFixture(**self.course_info).install() + self.instructor_username, self.instructor_id = self.log_in_as_instructor() + instructor_dashboard_page = self.visit_instructor_dashboard() + self.data_download_section = instructor_dashboard_page.select_data_download() + + def verify_report_requested_event(self, report_type): + """ + Verifies that the correct event is emitted when a report is requested. + """ + self.verify_events_of_type( + self.instructor_username, + u"edx.instructor.report.requested", + [{ + u"report_type": report_type + }] + ) + + def verify_report_downloaded_event(self, report_url): + """ + Verifies that the correct event is emitted when a report is downloaded. + """ + self.verify_events_of_type( + self.instructor_username, + u"edx.instructor.report.downloaded", + [{ + u"report_url": report_url + }] + ) + + def verify_report_download(self, report_name): + """ + Verifies that a report can be downloaded and an event fired. + """ + download_links = self.data_download_section.report_download_links + self.assertEquals(len(download_links), 1) + download_links[0].click() + expected_url = download_links.attrs('href')[0] + self.assertIn(report_name, expected_url) + self.verify_report_downloaded_event(expected_url) + + def test_student_profiles_report_download(self): + """ + Scenario: Verify that an instructor can download a student profiles report + + Given that I am an instructor + And I visit the instructor dashboard's "Data Downloads" tab + And I click on the "Download profile information as a CSV" button + Then a report should be generated + And a report requested event should be emitted + When I click on the report + Then a report downloaded event should be emitted + """ + report_name = u"student_profile_info" + self.data_download_section.generate_student_profile_report_button.click() + self.data_download_section.wait_for_available_report() + self.verify_report_requested_event(report_name) + self.verify_report_download(report_name) + + def test_grade_report_download(self): + """ + Scenario: Verify that an instructor can download a grade report + + Given that I am an instructor + And I visit the instructor dashboard's "Data Downloads" tab + And I click on the "Generate Grade Report" button + Then a report should be generated + And a report requested event should be emitted + When I click on the report + Then a report downloaded event should be emitted + """ + report_name = u"grade_report" + self.data_download_section.generate_grade_report_button.click() + self.data_download_section.wait_for_available_report() + self.verify_report_requested_event(report_name) + self.verify_report_download(report_name) + + def test_weighted_problem_grade_report_download(self): + """ + Scenario: Verify that an instructor can download a weighted problem grade report + + Given that I am an instructor + And I visit the instructor dashboard's "Data Downloads" tab + And I click on the "Generate Weighted Problem Grade Report" button + Then a report should be generated + And a report requested event should be emitted + When I click on the report + Then a report downloaded event should be emitted + """ + report_name = u"problem_grade_report" + self.data_download_section.generate_weighted_problem_grade_report_button.click() + self.data_download_section.wait_for_available_report() + self.verify_report_requested_event(report_name) + self.verify_report_download(report_name) diff --git a/lms/djangoapps/course_structure_api/v0/api.py b/lms/djangoapps/course_structure_api/v0/api.py index 5a0523f6b9..c59fbea080 100644 --- a/lms/djangoapps/course_structure_api/v0/api.py +++ b/lms/djangoapps/course_structure_api/v0/api.py @@ -23,8 +23,7 @@ def _retrieve_course(course_key): """ try: - course = courses.get_course(course_key) - return course + return courses.get_course(course_key) except ValueError: raise CourseNotFoundError diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index 16c1021e20..88048f93e4 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -156,7 +156,6 @@ def calculate_grades_csv(entry_id, xmodule_instance_args): return run_main_task(entry_id, task_fn, action_name) -# TODO: GRADES_DOWNLOAD_ROUTING_KEY is the high mem queue. Do we know we need it? @task(base=BaseInstructorTask, routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY) # pylint: disable=not-callable def calculate_problem_grade_report(entry_id, xmodule_instance_args): """ diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py index eb5d99a814..2479420eb6 100644 --- a/lms/djangoapps/instructor_task/tasks_helper.py +++ b/lms/djangoapps/instructor_task/tasks_helper.py @@ -55,6 +55,9 @@ UPDATE_STATUS_SUCCEEDED = 'succeeded' UPDATE_STATUS_FAILED = 'failed' UPDATE_STATUS_SKIPPED = 'skipped' +# The setting name used for events when "settings" (account settings, preferences, profile information) change. +REPORT_REQUESTED_EVENT_NAME = u'edx.instructor.report.requested' + class BaseInstructorTask(Task): """ @@ -553,6 +556,12 @@ def upload_csv_to_report_store(rows, csv_name, course_id, timestamp): ), rows ) + tracker.emit( + REPORT_REQUESTED_EVENT_NAME, + { + "report_type": csv_name, + } + ) def upload_grades_csv(_xmodule_instance_args, _entry_id, course_id, _task_input, action_name): # pylint: disable=too-many-statements diff --git a/lms/static/coffee/src/instructor_dashboard/data_download.coffee b/lms/static/coffee/src/instructor_dashboard/data_download.coffee index ad9de769f9..dcd2e7ba91 100644 --- a/lms/static/coffee/src/instructor_dashboard/data_download.coffee +++ b/lms/static/coffee/src/instructor_dashboard/data_download.coffee @@ -109,10 +109,10 @@ class DataDownload @$download_display_text.html data['grading_config_summary'] @$calculate_grades_csv_btn.click (e) => - @onClickGradeDownload @$calculate_grades_csv_btn, "Error generating grades. Please try again." + @onClickGradeDownload @$calculate_grades_csv_btn, gettext("Error generating grades. Please try again.") @$problem_grade_report_csv_btn.click (e) => - @onClickGradeDownload @$problem_grade_report_csv_btn, "Error generating weighted problem report. Please try again." + @onClickGradeDownload @$problem_grade_report_csv_btn, gettext("Error generating weighted problem report. Please try again.") onClickGradeDownload: (button, errorMessage) -> # Clear any CSS styling from the request-response areas @@ -124,7 +124,7 @@ class DataDownload dataType: 'json' url: url error: (std_ajax_err) => - @$reports_request_response_error.text gettext(errorMessage) + @$reports_request_response_error.text errorMessage $(".msg-error").css({"display":"block"}) success: (data) => @$reports_request_response.text data['status'] @@ -201,6 +201,15 @@ class ReportDownloads $table_placeholder = $ '
', class: 'slickgrid' @$report_downloads_table.append $table_placeholder grid = new Slick.Grid($table_placeholder, report_downloads_data, columns, options) + grid.onClick.subscribe( + (event) => + report_url = event.target.href + if report_url + # Record that the user requested to download a report + Logger.log('edx.instructor.report.downloaded', { + report_url: report_url + }) + ) grid.autosizeColumns()