diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 06d0f16ab84d7a043fa3911d9da9fc9fc56a8693..4ae780e061f99281daa55b6ea30ff96433cc09ca 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -102,11 +102,6 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): CourseEnrollment.enroll(self.user, self.course.id) self.client.login(username=self.user.username, password='test') - def test_deny_students_update_enrollment(self): - url = reverse('students_update_enrollment', kwargs={'course_id': self.course.id}) - response = self.client.get(url, {}) - self.assertEqual(response.status_code, 403) - def test_staff_level(self): """ Ensure that an enrolled student can't access staff or instructor endpoints. @@ -126,11 +121,16 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): 'update_forum_role_membership', 'proxy_legacy_analytics', 'send_email', + 'list_background_email_tasks', ] for endpoint in staff_level_endpoints: url = reverse(endpoint, kwargs={'course_id': self.course.id}) response = self.client.get(url, {}) - self.assertEqual(response.status_code, 403) + self.assertEqual( + response.status_code, + 403, + msg="Student should not be allowed to access endpoint " + endpoint + ) def test_instructor_level(self): """ @@ -140,13 +140,16 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): 'modify_access', 'list_course_role_members', 'reset_student_attempts', - 'list_instructor_tasks', 'update_forum_role_membership', ] for endpoint in instructor_level_endpoints: url = reverse(endpoint, kwargs={'course_id': self.course.id}) response = self.client.get(url, {}) - self.assertEqual(response.status_code, 403) + self.assertEqual( + response.status_code, + 403, + msg="Staff should not be allowed to access endpoint " + endpoint + ) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @@ -692,6 +695,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase) }) print response.content self.assertEqual(response.status_code, 200) + self.assertTrue(act.called) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @@ -870,6 +874,25 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase): self.assertDictEqual(exp_task, act_task) self.assertEqual(actual_tasks, expected_tasks) + @patch.object(instructor_task.api, 'get_instructor_task_history') + def test_list_background_email_tasks(self, act): + """Test list of background email tasks.""" + act.return_value = self.tasks + url = reverse('list_background_email_tasks', kwargs={'course_id': self.course.id}) + mock_factory = MockCompletionInfo() + with patch('instructor.views.api.get_task_completion_info') as mock_completion_info: + mock_completion_info.side_effect = mock_factory.mock_get_task_completion_info + response = self.client.get(url, {}) + self.assertEqual(response.status_code, 200) + + # check response + self.assertTrue(act.called) + expected_tasks = [ftask.to_dict() for ftask in self.tasks] + actual_tasks = json.loads(response.content)['tasks'] + for exp_task, act_task in zip(expected_tasks, actual_tasks): + self.assertDictEqual(exp_task, act_task) + self.assertEqual(actual_tasks, expected_tasks) + @patch.object(instructor_task.api, 'get_instructor_task_history') def test_list_instructor_tasks_problem(self, act): """ Test list task history for problem. """ diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 8ab1aa8a76225a913be68d33ab36aafbd24cf41e..3b0f6479c70fe9b4901c472110a6ceac7a082afc 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -157,7 +157,7 @@ def require_level(level): `level` is in ['instructor', 'staff'] if `level` is 'staff', instructors will also be allowed, even - if they are not int he staff group. + if they are not in the staff group. """ if level not in ['instructor', 'staff']: raise ValueError("unrecognized level '{}'".format(level)) @@ -643,13 +643,68 @@ def rescore_problem(request, course_id): return JsonResponse(response_payload) +def extract_task_features(task): + """ + Convert task to dict for json rendering. + Expects tasks have the following features: + * task_type (str, type of task) + * task_input (dict, input(s) to the task) + * task_id (str, celery id of the task) + * requester (str, username who submitted the task) + * task_state (str, state of task eg PROGRESS, COMPLETED) + * created (datetime, when the task was completed) + * task_output (optional) + """ + # Pull out information from the task + features = ['task_type', 'task_input', 'task_id', 'requester', 'task_state'] + task_feature_dict = {feature: str(getattr(task, feature)) for feature in features} + # Some information (created, duration, status, task message) require additional formatting + task_feature_dict['created'] = task.created.isoformat() + + # Get duration info, if known + duration_sec = 'unknown' + if hasattr(task, 'task_output') and task.task_output is not None: + try: + task_output = json.loads(task.task_output) + except ValueError: + log.error("Could not parse task output as valid json; task output: %s", task.task_output) + else: + if 'duration_ms' in task_output: + duration_sec = int(task_output['duration_ms'] / 1000.0) + task_feature_dict['duration_sec'] = duration_sec + + # Get progress status message & success information + success, task_message = get_task_completion_info(task) + status = _("Complete") if success else _("Incomplete") + task_feature_dict['status'] = status + task_feature_dict['task_message'] = task_message + + return task_feature_dict + + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('instructor') +@require_level('staff') +def list_background_email_tasks(request, course_id): # pylint: disable=unused-argument + """ + List background email tasks. + """ + task_type = 'bulk_course_email' + # Specifying for the history of a single task type + tasks = instructor_task.api.get_instructor_task_history(course_id, task_type=task_type) + + response_payload = { + 'tasks': map(extract_task_features, tasks), + } + return JsonResponse(response_payload) + + +@ensure_csrf_cookie +@cache_control(no_cache=True, no_store=True, must_revalidate=True) +@require_level('staff') def list_instructor_tasks(request, course_id): """ List instructor tasks. - Limited to instructor access. Takes optional query paremeters. - With no arguments, lists running tasks. @@ -670,50 +725,15 @@ def list_instructor_tasks(request, course_id): if problem_urlname: module_state_key = _msk_from_problem_urlname(course_id, problem_urlname) if student: + # Specifying for a single student's history on this problem tasks = instructor_task.api.get_instructor_task_history(course_id, module_state_key, student) else: + # Specifying for single problem's history tasks = instructor_task.api.get_instructor_task_history(course_id, module_state_key) else: + # If no problem or student, just get currently running tasks tasks = instructor_task.api.get_running_instructor_tasks(course_id) - def extract_task_features(task): - """ - Convert task to dict for json rendering. - Expects tasks have the following features: - * task_type (str, type of task) - * task_input (dict, input(s) to the task) - * task_id (str, celery id of the task) - * requester (str, username who submitted the task) - * task_state (str, state of task eg PROGRESS, COMPLETED) - * created (datetime, when the task was completed) - * task_output (optional) - """ - # Pull out information from the task - features = ['task_type', 'task_input', 'task_id', 'requester', 'task_state'] - task_feature_dict = {feature: str(getattr(task, feature)) for feature in features} - # Some information (created, duration, status, task message) require additional formatting - task_feature_dict['created'] = task.created.isoformat() - - # Get duration info, if known - duration_sec = 'unknown' - if hasattr(task, 'task_output') and task.task_output is not None: - try: - task_output = json.loads(task.task_output) - except ValueError: - log.error("Could not parse task output as valid json; task output: %s", task.task_output) - else: - if 'duration_ms' in task_output: - duration_sec = int(task_output['duration_ms'] / 1000.0) - task_feature_dict['duration_sec'] = duration_sec - - # Get progress status message & success information - success, task_message = get_task_completion_info(task) - status = _("Complete") if success else _("Incomplete") - task_feature_dict['status'] = status - task_feature_dict['task_message'] = task_message - - return task_feature_dict - response_payload = { 'tasks': map(extract_task_features, tasks), } @@ -901,7 +921,7 @@ def proxy_legacy_analytics(request, course_id): try: res = requests.get(url) - except Exception: + except Exception: # pylint: disable=broad-except log.exception("Error requesting from analytics server at %s", url) return HttpResponse("Error requesting from analytics server.", status=500) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 79cdcf7e69c64d0fbbd907275745a93e2eb18b4d..3f05f1cc70ecfff1eb9abfb4934bcd6989ac8552 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -27,6 +27,8 @@ urlpatterns = patterns('', # nopep8 'instructor.views.api.rescore_problem', name="rescore_problem"), url(r'^list_instructor_tasks$', 'instructor.views.api.list_instructor_tasks', name="list_instructor_tasks"), + url(r'^list_background_email_tasks$', + 'instructor.views.api.list_background_email_tasks', name="list_background_email_tasks"), url(r'^list_forum_members$', 'instructor.views.api.list_forum_members', name="list_forum_members"), url(r'^update_forum_role_membership$', @@ -34,5 +36,5 @@ urlpatterns = patterns('', # nopep8 url(r'^proxy_legacy_analytics$', 'instructor.views.api.proxy_legacy_analytics', name="proxy_legacy_analytics"), url(r'^send_email$', - 'instructor.views.api.send_email', name="send_email") + 'instructor.views.api.send_email', name="send_email"), ) diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 8b5c44ebf01a946d8e54b4d572edbc7095a31b5c..401d08b6df363aa7e0d0e3dd504e228628a5e951 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -27,7 +27,7 @@ from bulk_email.models import CourseAuthorization @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) def instructor_dashboard_2(request, course_id): - """ Display the instructor dashboard for a course. """ + """Display the instructor dashboard for a course.""" course = get_course_by_id(course_id, depth=None) is_studio_course = (modulestore().get_modulestore_type(course_id) == MONGO_MODULESTORE_TYPE) @@ -45,11 +45,11 @@ def instructor_dashboard_2(request, course_id): raise Http404() sections = [ - _section_course_info(course_id), + _section_course_info(course_id, access), _section_membership(course_id, access), _section_student_admin(course_id, access), - _section_data_download(course_id), - _section_analytics(course_id), + _section_data_download(course_id, access), + _section_analytics(course_id, access), ] # Gate access to course email by feature flag & by course-specific authorization @@ -91,7 +91,7 @@ section_display_name will be used to generate link titles in the nav bar. """ # pylint: disable=W0105 -def _section_course_info(course_id): +def _section_course_info(course_id, access): """ Provide data for the corresponding dashboard section """ course = get_course_by_id(course_id, depth=None) @@ -100,6 +100,7 @@ def _section_course_info(course_id): section_data = { 'section_key': 'course_info', 'section_display_name': _('Course Info'), + 'access': access, 'course_id': course_id, 'course_org': course_org, 'course_num': course_num, @@ -157,11 +158,12 @@ def _section_student_admin(course_id, access): return section_data -def _section_data_download(course_id): +def _section_data_download(course_id, access): """ Provide data for the corresponding dashboard section """ section_data = { 'section_key': 'data_download', 'section_display_name': _('Data Download'), + 'access': access, 'get_grading_config_url': reverse('get_grading_config', kwargs={'course_id': course_id}), 'get_students_features_url': reverse('get_students_features', kwargs={'course_id': course_id}), 'get_anon_ids_url': reverse('get_anon_ids', kwargs={'course_id': course_id}), @@ -183,15 +185,17 @@ def _section_send_email(course_id, access, course): 'send_email': reverse('send_email', kwargs={'course_id': course_id}), 'editor': email_editor, 'list_instructor_tasks_url': reverse('list_instructor_tasks', kwargs={'course_id': course_id}), + 'email_background_tasks_url': reverse('list_background_email_tasks', kwargs={'course_id': course_id}), } return section_data -def _section_analytics(course_id): +def _section_analytics(course_id, access): """ Provide data for the corresponding dashboard section """ section_data = { 'section_key': 'analytics', 'section_display_name': _('Analytics'), + 'access': access, 'get_distribution_url': reverse('get_distribution', kwargs={'course_id': course_id}), 'proxy_legacy_analytics_url': reverse('proxy_legacy_analytics', kwargs={'course_id': course_id}), } diff --git a/lms/envs/common.py b/lms/envs/common.py index 0457e7149de8bbd47da838649c4e07990e92a2c3..df83bc92188ac36db32c00befab93f37aa2e8ee0 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -118,7 +118,8 @@ MITX_FEATURES = { 'ENABLE_INSTRUCTOR_EMAIL': True, # If True and ENABLE_INSTRUCTOR_EMAIL: Forces email to be explicitly turned on # for each course via django-admin interface. - # If False and ENABLE_INSTRUCTOR_EMAIL: Email will be turned on by default for all courses. + # If False and ENABLE_INSTRUCTOR_EMAIL: Email will be turned on by default + # for all Mongo-backed courses. 'REQUIRE_COURSE_EMAIL_AUTH': True, # enable analytics server. diff --git a/lms/static/coffee/src/instructor_dashboard/send_email.coffee b/lms/static/coffee/src/instructor_dashboard/send_email.coffee index 7bdb37c0e9e3b7c52532cc32df52bdc27cc2a7b8..edf6a07003c4901a9ba8c05e84953eaf73272957 100644 --- a/lms/static/coffee/src/instructor_dashboard/send_email.coffee +++ b/lms/static/coffee/src/instructor_dashboard/send_email.coffee @@ -10,6 +10,7 @@ such that the value can be defined later than this assignment (file load order). plantTimeout = -> window.InstructorDashboard.util.plantTimeout.apply this, arguments std_ajax_err = -> window.InstructorDashboard.util.std_ajax_err.apply this, arguments PendingInstructorTasks = -> window.InstructorDashboard.util.PendingInstructorTasks +create_task_list_table = -> window.InstructorDashboard.util.create_task_list_table.apply this, arguments class SendEmail constructor: (@$container) -> @@ -20,6 +21,9 @@ class SendEmail @$btn_send = @$container.find("input[name='send']'") @$task_response = @$container.find(".request-response") @$request_response_error = @$container.find(".request-response-error") + @$history_request_response_error = @$container.find(".history-request-response-error") + @$btn_task_history_email = @$container.find("input[name='task-history-email']'") + @$table_task_history_email = @$container.find(".task-history-email-table") # attach click handlers @@ -63,6 +67,22 @@ class SendEmail @$task_response.empty() @$request_response_error.empty() + # list task history for email + @$btn_task_history_email.click => + url = @$btn_task_history_email.data 'endpoint' + $.ajax + dataType: 'json' + url: url + success: (data) => + if data.tasks.length + create_task_list_table @$table_task_history_email, data.tasks + else + @$history_request_response_error.text gettext("There is no email history for this course.") + # Enable the msg-warning css display + $(".msg-warning").css({"display":"block"}) + error: std_ajax_err => + @$history_request_response_error.text gettext("There was an error obtaining email task history for this course.") + fail_with_error: (msg) -> console.warn msg @$task_response.empty() diff --git a/lms/static/coffee/src/instructor_dashboard/util.coffee b/lms/static/coffee/src/instructor_dashboard/util.coffee index 09d2ae26f328933b0eaaf45c10a4c92cfad77f76..af5b99f4198ae503b2707c038e0d4b179b5cb8d2 100644 --- a/lms/static/coffee/src/instructor_dashboard/util.coffee +++ b/lms/static/coffee/src/instructor_dashboard/util.coffee @@ -36,7 +36,7 @@ create_task_list_table = ($table_tasks, tasks_data) -> enableCellNavigation: true enableColumnReorder: false autoHeight: true - rowHeight: 60 + rowHeight: 100 forceFitColumns: true columns = [ @@ -120,7 +120,7 @@ class PendingInstructorTasks # start polling for task list # if the list is in the DOM - if @$table_running_tasks.length > 0 + if @$table_running_tasks.length # reload every 20 seconds. TASK_LIST_POLL_INTERVAL = 20000 @reload_running_tasks_list() @@ -132,8 +132,12 @@ class PendingInstructorTasks $.ajax dataType: 'json' url: list_endpoint - success: (data) => create_task_list_table @$table_running_tasks, data.tasks - error: std_ajax_err => console.warn "error listing all instructor tasks" + success: (data) => + if data.tasks.length + create_task_list_table @$table_running_tasks, data.tasks + else + console.log "No pending instructor tasks to display" + error: std_ajax_err => console.error "Error finding pending instructor tasks to display" ### /Pending Instructor Tasks Section #### # export for use diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss index f6a9dd2663a42246994b9bb28180081f41b5728e..e0c0e3a6bb2c4be396dc791d91e0125ad0848a03 100644 --- a/lms/static/sass/course/instructor/_instructor_2.scss +++ b/lms/static/sass/course/instructor/_instructor_2.scss @@ -42,10 +42,8 @@ .msg-warning { border-top: 2px solid $warning-color; background: tint($warning-color,95%); - - .copy { - color: $warning-color; - } + display: none; + color: $warning-color; } // TYPE: confirm diff --git a/lms/templates/instructor/instructor_dashboard_2/send_email.html b/lms/templates/instructor/instructor_dashboard_2/send_email.html index b3970c509130cd1efe9c00ae4ceb59556e7e007e..4d49530b5c68116aee41685633b7ab4d85a3a1a7 100644 --- a/lms/templates/instructor/instructor_dashboard_2/send_email.html +++ b/lms/templates/instructor/instructor_dashboard_2/send_email.html @@ -59,11 +59,23 @@ <div class="running-tasks-container action-type-container"> <hr> <h2> ${_("Pending Instructor Tasks")} </h2> - <p>${_("The status for any active tasks appears in a table below.")} </p> + <p>${_("Email actions run in the background. The status for any active tasks - including email tasks - appears in a table below.")} </p> <br /> <div class="running-tasks-table" data-endpoint="${ section_data['list_instructor_tasks_url'] }"></div> </div> -%endif + +<hr> + +<div class="vert-left email-background" id="section-task-history"> + <h2> ${_("Email Task History")} </h2> + <p>${_("To see the status for all bulk email tasks ever submitted for this course, click on this button:")}</p> + <br/> + <input type="button" name="task-history-email" value="${_("Show Email Task History")}" data-endpoint="${ section_data['email_background_tasks_url'] }" > + <div class="history-request-response-error msg msg-warning copy"></div> + <div class="task-history-email-table"></div> </div> +%endif + +</div> <!-- end section send-email -->