diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index ba07cbade0..f05228627b 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -52,7 +52,11 @@ from lms.djangoapps.instructor.views.api import ( generate_unique_password, require_finance_admin ) -from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError +from lms.djangoapps.instructor_task.api_helper import ( + AlreadyRunningError, + QueueConnectionError, + generate_already_running_error_message +) from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin @@ -143,6 +147,7 @@ REPORTS_DATA = ( EXECUTIVE_SUMMARY_DATA = ( { 'report_type': 'executive summary', + 'task_type': 'exec_summary_report', 'instructor_api_endpoint': 'get_exec_summary_report', 'task_api_endpoint': 'lms.djangoapps.instructor_task.api.submit_executive_summary_report', 'extra_instructor_api_kwargs': {} @@ -244,7 +249,16 @@ def view_alreadyrunningerror(request): # pylint: disable=unused-argument raise AlreadyRunningError() +@common_exceptions_400 +def view_queue_connection_error(request): # pylint: disable=unused-argument + """ + A dummy view that raises a QueueConnectionError exception. + """ + raise QueueConnectionError() + + @attr(shard=1) +@ddt.ddt class TestCommonExceptions400(TestCase): """ Testing the common_exceptions_400 decorator. @@ -269,21 +283,29 @@ class TestCommonExceptions400(TestCase): self.request.is_ajax.return_value = True resp = view_user_doesnotexist(self.request) # pylint: disable=assignment-from-no-return self.assertEqual(resp.status_code, 400) - result = json.loads(resp.content) - self.assertIn("User does not exist", result["error"]) + self.assertIn("User does not exist", resp.content) def test_alreadyrunningerror(self): self.request.is_ajax.return_value = False resp = view_alreadyrunningerror(self.request) # pylint: disable=assignment-from-no-return self.assertEqual(resp.status_code, 400) - self.assertIn("Task is already running", resp.content) + self.assertIn("Requested task is already running", resp.content) def test_alreadyrunningerror_ajax(self): self.request.is_ajax.return_value = True resp = view_alreadyrunningerror(self.request) # pylint: disable=assignment-from-no-return self.assertEqual(resp.status_code, 400) - result = json.loads(resp.content) - self.assertIn("Task is already running", result["error"]) + self.assertIn("Requested task is already running", resp.content) + + @ddt.data(True, False) + def test_queue_connection_error(self, is_ajax): + """ + Tests that QueueConnectionError exception is handled in common_exception_400. + """ + self.request.is_ajax.return_value = is_ajax + resp = view_queue_connection_error(self.request) # pylint: disable=assignment-from-no-return + self.assertEqual(resp.status_code, 400) + self.assertIn('Error occured. Please try again later', resp.content) @attr(shard=1) @@ -401,6 +423,7 @@ class TestInstructorAPIDenyLevels(SharedModuleStoreTestCase, LoginEnrollmentTest ('get_proctored_exam_results', {}), ('get_problem_responses', {}), ('export_ora2_data', {}), + ] # Endpoints that only Instructors can access self.instructor_level_endpoints = [ @@ -2680,14 +2703,15 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment 'get_problem_responses', kwargs={'course_id': unicode(self.course.id)} ) - + task_type = 'problem_responses_csv' + already_running_status = generate_already_running_error_message(task_type) with patch('lms.djangoapps.instructor_task.api.submit_calculate_problem_responses_csv') as submit_task_function: - error = AlreadyRunningError() + error = AlreadyRunningError(already_running_status) submit_task_function.side_effect = error response = self.client.post(url, {}) - res_json = json.loads(response.content) - self.assertIn('status', res_json) - self.assertIn('already in progress', res_json['status']) + + self.assertEqual(response.status_code, 400) + self.assertIn(already_running_status, response.content) def test_get_students_features(self): """ @@ -2757,17 +2781,16 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment ) # Successful case: response = self.client.post(url, {}) - res_json = json.loads(response.content) - self.assertIn('status', res_json) - self.assertNotIn('currently being created', res_json['status']) + self.assertEqual(response.status_code, 200) # CSV generation already in progress: + task_type = 'may_enroll_info_csv' + already_running_status = generate_already_running_error_message(task_type) with patch('lms.djangoapps.instructor_task.api.submit_calculate_may_enroll_csv') as submit_task_function: - error = AlreadyRunningError() + error = AlreadyRunningError(already_running_status) submit_task_function.side_effect = error response = self.client.post(url, {}) - res_json = json.loads(response.content) - self.assertIn('status', res_json) - self.assertIn('currently being created', res_json['status']) + self.assertEqual(response.status_code, 400) + self.assertIn(already_running_status, response.content) def test_get_student_exam_results(self): """ @@ -2778,19 +2801,19 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment 'get_proctored_exam_results', kwargs={'course_id': unicode(self.course.id)} ) + # Successful case: response = self.client.post(url, {}) - res_json = json.loads(response.content) - self.assertIn('status', res_json) - self.assertNotIn('currently being created', res_json['status']) + self.assertEqual(response.status_code, 200) # CSV generation already in progress: + task_type = 'proctored_exam_results_report' + already_running_status = generate_already_running_error_message(task_type) with patch('lms.djangoapps.instructor_task.api.submit_proctored_exam_results_report') as submit_task_function: - error = AlreadyRunningError() + error = AlreadyRunningError(already_running_status) submit_task_function.side_effect = error response = self.client.post(url, {}) - res_json = json.loads(response.content) - self.assertIn('status', res_json) - self.assertIn('currently being created', res_json['status']) + self.assertEqual(response.status_code, 400) + self.assertIn(already_running_status, response.content) def test_access_course_finance_admin_with_invalid_course_key(self): """ @@ -3045,10 +3068,11 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment def test_executive_summary_report_success( self, report_type, + task_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs - ): + ): # pylint: disable=unused-argument kwargs = {'course_id': unicode(self.course.id)} kwargs.update(extra_instructor_api_kwargs) url = reverse(instructor_api_endpoint, kwargs=kwargs) @@ -3066,6 +3090,7 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment def test_executive_summary_report_already_running( self, report_type, + task_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs @@ -3075,14 +3100,12 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment url = reverse(instructor_api_endpoint, kwargs=kwargs) CourseFinanceAdminRole(self.course.id).add_users(self.instructor) + already_running_status = generate_already_running_error_message(task_type) with patch(task_api_endpoint) as mock: - mock.side_effect = AlreadyRunningError() + mock.side_effect = AlreadyRunningError(already_running_status) response = self.client.post(url, {}) - already_running_status = "The {report_type} report is currently being created." \ - " To view the status of the report, see Pending Tasks below." \ - " You will be able to download the report" \ - " when it is" \ - " complete.".format(report_type=report_type) + + self.assertEqual(response.status_code, 400) self.assertIn(already_running_status, response.content) def test_get_ora2_responses_success(self): @@ -3091,16 +3114,19 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment with patch('lms.djangoapps.instructor_task.api.submit_export_ora2_data') as mock_submit_ora2_task: mock_submit_ora2_task.return_value = True response = self.client.post(url, {}) - success_status = "The ORA data report is being generated." + success_status = "The ORA data report is being created." self.assertIn(success_status, response.content) def test_get_ora2_responses_already_running(self): url = reverse('export_ora2_data', kwargs={'course_id': unicode(self.course.id)}) + task_type = 'export_ora2_data' + already_running_status = generate_already_running_error_message(task_type) with patch('lms.djangoapps.instructor_task.api.submit_export_ora2_data') as mock_submit_ora2_task: - mock_submit_ora2_task.side_effect = AlreadyRunningError() + mock_submit_ora2_task.side_effect = AlreadyRunningError(already_running_status) response = self.client.post(url, {}) - already_running_status = "An ORA data report generation task is already in progress." + + self.assertEqual(response.status_code, 400) self.assertIn(already_running_status, response.content) def test_get_student_progress_url(self): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index f9911ddb4e..cb9e2852b8 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -71,7 +71,7 @@ from lms.djangoapps.instructor.enrollment import ( from lms.djangoapps.instructor.views import INVOICE_KEY from lms.djangoapps.instructor.views.instructor_task_helpers import extract_email_features, extract_task_features from lms.djangoapps.instructor_task.api import submit_override_score -from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError +from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError from lms.djangoapps.instructor_task.models import ReportStore from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.course_groups.cohorts import is_course_cohorted @@ -133,29 +133,31 @@ log = logging.getLogger(__name__) TASK_SUBMISSION_OK = 'created' +SUCCESS_MESSAGE_TEMPLATE = _("The {report_type} report is being created. " + "To view the status of the report, see Pending Tasks below.") + def common_exceptions_400(func): """ Catches common exceptions and renders matching 400 errors. (decorator without arguments) """ + def wrapped(request, *args, **kwargs): # pylint: disable=missing-docstring use_json = (request.is_ajax() or request.META.get("HTTP_ACCEPT", "").startswith("application/json")) try: return func(request, *args, **kwargs) except User.DoesNotExist: - message = _("User does not exist.") - if use_json: - return JsonResponse({"error": message}, 400) - else: - return HttpResponseBadRequest(message) - except AlreadyRunningError: - message = _("Task is already running.") - if use_json: - return JsonResponse({"error": message}, 400) - else: - return HttpResponseBadRequest(message) + message = _('User does not exist.') + except (AlreadyRunningError, QueueConnectionError) as err: + message = str(err) + + if use_json: + return JsonResponseBadRequest(message) + else: + return HttpResponseBadRequest(message) + return wrapped @@ -829,12 +831,12 @@ def bulk_beta_modify_access(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('instructor') -@common_exceptions_400 @require_post_params( unique_student_identifier="email or username of user to change access", rolename="'instructor', 'staff', 'beta', or 'ccx_coach'", action="'allow' or 'revoke'" ) +@common_exceptions_400 def modify_access(request, course_id): """ Modify staff/instructor access of other user. @@ -964,6 +966,7 @@ def list_course_role_members(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def get_problem_responses(request, course_id): """ Initiate generation of a CSV file containing all student answers @@ -978,6 +981,7 @@ def get_problem_responses(request, course_id): """ course_key = CourseKey.from_string(course_id) problem_location = request.POST.get('problem_location', '') + report_type = _('problem responses') try: problem_key = UsageKey.from_string(problem_location) @@ -990,20 +994,10 @@ def get_problem_responses(request, course_id): except InvalidKeyError: return JsonResponseBadRequest(_("Could not find problem with this location.")) - try: - lms.djangoapps.instructor_task.api.submit_calculate_problem_responses_csv(request, course_key, problem_location) - success_status = _( - "The problem responses report is being created." - " To view the status of the report, see Pending Tasks below." - ) - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _( - "A problem responses report generation task is already in progress. " - "Check the 'Pending 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}) + lms.djangoapps.instructor_task.api.submit_calculate_problem_responses_csv(request, course_key, problem_location) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @require_POST @@ -1209,6 +1203,7 @@ def get_issued_certificates(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def get_students_features(request, course_id, csv=False): # pylint: disable=redefined-outer-name """ Respond with json which contains a summary of all enrolled students profile information. @@ -1220,7 +1215,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red """ course_key = CourseKey.from_string(course_id) course = get_course_by_id(course_key) - + report_type = _('enrolled learner profile') available_features = instructor_analytics.basic.AVAILABLE_FEATURES # Allow for sites to be able to define additional columns. @@ -1285,22 +1280,16 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red 'available_features': available_features, } return JsonResponse(response_payload) + else: - try: - lms.djangoapps.instructor_task.api.submit_calculate_students_features_csv( - request, - course_key, - query_features - ) - success_status = _("The enrolled learner profile report is being created." - " To view the status of the report, see Pending Tasks below.") - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _( - "This enrollment report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete.") - return JsonResponse({"status": already_running_status}) + lms.djangoapps.instructor_task.api.submit_calculate_students_features_csv( + request, + course_key, + query_features + ) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -1308,6 +1297,7 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=red @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def get_students_who_may_enroll(request, course_id): """ Initiate generation of a CSV file containing information about @@ -1319,21 +1309,11 @@ def get_students_who_may_enroll(request, course_id): """ course_key = CourseKey.from_string(course_id) query_features = ['email'] - try: - lms.djangoapps.instructor_task.api.submit_calculate_may_enroll_csv(request, course_key, query_features) - success_status = _( - "The enrollment report is being created. This report contains" - " information about learners who can enroll in the course." - " To view the status of the report, see Pending Tasks below." - ) - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _( - "This enrollment report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete." - ) - return JsonResponse({"status": already_running_status}) + report_type = _('enrollment') + lms.djangoapps.instructor_task.api.submit_calculate_may_enroll_csv(request, course_key, query_features) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -1341,6 +1321,7 @@ def get_students_who_may_enroll(request, course_id): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_POST @require_level('staff') +@common_exceptions_400 def add_users_to_cohorts(request, course_id): """ View method that accepts an uploaded file (using key "uploaded-file") @@ -1417,23 +1398,17 @@ def get_coupon_codes(request, course_id): # pylint: disable=unused-argument @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') @require_finance_admin +@common_exceptions_400 def get_enrollment_report(request, course_id): """ get the enrollment report for the particular course. """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_detailed_enrollment_features_csv(request, course_key) - success_status = _("The detailed enrollment report is being created." - " To view the status of the report, see Pending Tasks below.") - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _("The detailed enrollment report is being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete.") - return JsonResponse({ - "status": already_running_status - }) + report_type = _('detailed enrollment') + lms.djangoapps.instructor_task.api.submit_detailed_enrollment_features_csv(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -1442,24 +1417,17 @@ def get_enrollment_report(request, course_id): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') @require_finance_admin +@common_exceptions_400 def get_exec_summary_report(request, course_id): """ get the executive summary report for the particular course. """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_executive_summary_report(request, course_key) - status_response = _("The executive summary report is being created." - " To view the status of the report, see Pending Tasks below.") - except AlreadyRunningError: - status_response = _( - "The executive summary report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete." - ) - return JsonResponse({ - "status": status_response - }) + report_type = _('executive summary') + lms.djangoapps.instructor_task.api.submit_executive_summary_report(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -1467,24 +1435,17 @@ def get_exec_summary_report(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def get_course_survey_results(request, course_id): """ get the survey results report for the particular course. """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_course_survey_report(request, course_key) - status_response = _("The survey report is being created." - " To view the status of the report, see Pending Tasks below.") - except AlreadyRunningError: - status_response = _( - "The survey report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete." - ) - return JsonResponse({ - "status": status_response - }) + report_type = _('survey') + lms.djangoapps.instructor_task.api.submit_course_survey_report(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -1492,6 +1453,7 @@ def get_course_survey_results(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def get_proctored_exam_results(request, course_id): """ get the proctored exam resultsreport for the particular course. @@ -1508,19 +1470,11 @@ def get_proctored_exam_results(request, course_id): ] course_key = CourseKey.from_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_proctored_exam_results_report(request, course_key, query_features) - status_response = _("The proctored exam results report is being created." - " To view the status of the report, see Pending Tasks below.") - except AlreadyRunningError: - status_response = _( - "The proctored exam results report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete." - ) - return JsonResponse({ - "status": status_response - }) + report_type = _('proctored exam results') + lms.djangoapps.instructor_task.api.submit_proctored_exam_results_report(request, course_key, query_features) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) def save_registration_code(user, course_id, mode_slug, invoice=None, order=None, invoice_item=None): @@ -1901,11 +1855,11 @@ def get_anon_ids(request, course_id): # pylint: disable=unused-argument @require_POST @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@common_exceptions_400 @require_level('staff') @require_post_params( unique_student_identifier="email or username of student for whom to get progress url" ) +@common_exceptions_400 def get_student_progress_url(request, course_id): """ Get the progress url of a student. @@ -2147,6 +2101,7 @@ def rescore_problem(request, course_id): ) except NotImplementedError as exc: return HttpResponseBadRequest(exc.message) + elif all_students: try: lms.djangoapps.instructor_task.api.submit_rescore_problem_for_all_students( @@ -2453,25 +2408,17 @@ def list_financial_report_downloads(_request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def export_ora2_data(request, course_id): """ Pushes a Celery task which will aggregate ora2 responses for a course into a .csv """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_export_ora2_data(request, course_key) - success_status = _("The ORA data report is being generated.") + report_type = _('ORA data') + lms.djangoapps.instructor_task.api.submit_export_ora2_data(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _( - "An ORA data report generation task is already in " - "progress. Check the 'Pending 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}) + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -2479,21 +2426,17 @@ def export_ora2_data(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def calculate_grades_csv(request, course_id): """ AlreadyRunningError is raised if the course's grades are already being updated. """ + report_type = _('grade') course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_calculate_grades_csv(request, course_key) - success_status = _("The grade report is being created." - " To view the status of the report, see Pending Tasks below.") - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _("The grade report is currently being created." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete.") - return JsonResponse({"status": already_running_status}) + lms.djangoapps.instructor_task.api.submit_calculate_grades_csv(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @transaction.non_atomic_requests @@ -2501,6 +2444,7 @@ def calculate_grades_csv(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') +@common_exceptions_400 def problem_grade_report(request, course_id): """ Request a CSV showing students' grades for all problems in the @@ -2510,18 +2454,11 @@ def problem_grade_report(request, course_id): updated. """ course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - try: - lms.djangoapps.instructor_task.api.submit_problem_grade_report(request, course_key) - success_status = _("The problem grade report is being created." - " To view the status of the report, see Pending Tasks below.") - return JsonResponse({"status": success_status}) - except AlreadyRunningError: - already_running_status = _("A problem grade report is already being generated." - " To view the status of the report, see Pending Tasks below." - " You will be able to download the report when it is complete.") - return JsonResponse({ - "status": already_running_status - }) + report_type = _('problem grade') + lms.djangoapps.instructor_task.api.submit_problem_grade_report(request, course_key) + success_status = SUCCESS_MESSAGE_TEMPLATE.format(report_type=report_type) + + return JsonResponse({"status": success_status}) @require_POST @@ -2601,6 +2538,7 @@ def list_forum_members(request, course_id): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_level('staff') @require_post_params(send_to="sending to whom", subject="subject line", message="message text") +@common_exceptions_400 def send_email(request, course_id): """ Send an email to self, staff, cohorts, or everyone involved in a course. @@ -2667,6 +2605,7 @@ def send_email(request, course_id): 'course_id': course_id.to_deprecated_string(), 'success': True, } + return JsonResponse(response_payload) @@ -2944,6 +2883,7 @@ def mark_student_can_skip_entrance_exam(request, course_id): # pylint: disable= @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_global_staff @require_POST +@common_exceptions_400 def start_certificate_generation(request, course_id): """ Start generating certificates for all students enrolled in given course. @@ -2956,6 +2896,7 @@ def start_certificate_generation(request, course_id): 'message': message, 'task_id': task.task_id } + return JsonResponse(response_payload) @@ -2964,6 +2905,7 @@ def start_certificate_generation(request, course_id): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_global_staff @require_POST +@common_exceptions_400 def start_certificate_regeneration(request, course_id): """ Start regenerating certificates for students whose certificate statuses lie with in 'certificate_statuses' @@ -2990,11 +2932,8 @@ def start_certificate_regeneration(request, course_id): {'message': _('Please select certificate statuses from the list only.')}, status=400 ) - try: - lms.djangoapps.instructor_task.api.regenerate_certificates(request, course_key, certificates_statuses) - except AlreadyRunningError as error: - return JsonResponse({'message': error.message}, status=400) + lms.djangoapps.instructor_task.api.regenerate_certificates(request, course_key, certificates_statuses) response_payload = { 'message': _('Certificate regeneration task has been started. ' 'You can view the status of the generation task in the "Pending Tasks" section.'), @@ -3183,6 +3122,7 @@ def get_student(username_or_email, course_key): @cache_control(no_cache=True, no_store=True, must_revalidate=True) @require_global_staff @require_POST +@common_exceptions_400 def generate_certificate_exceptions(request, course_id, generate_for=None): """ Generate Certificate for students in the Certificate White List. @@ -3213,7 +3153,6 @@ def generate_certificate_exceptions(request, course_id, generate_for=None): ) lms.djangoapps.instructor_task.api.generate_certificates_for_students(request, course_key, student_set=students) - response_payload = { 'success': True, 'message': _('Certificate generation started for white listed students.'), @@ -3401,6 +3340,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat } +@common_exceptions_400 def re_validate_certificate(request, course_key, generated_certificate): """ Remove certificate invalidation from db and start certificate generation task for this student. @@ -3421,6 +3361,7 @@ def re_validate_certificate(request, course_key, generated_certificate): # We need to generate certificate only for a single student here student = certificate_invalidation.generated_certificate.user + lms.djangoapps.instructor_task.api.generate_certificates_for_students( request, course_key, student_set="specific_student", specific_student_id=student.id ) diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 3996af339a..c49a9be1d8 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -25,7 +25,26 @@ log = logging.getLogger(__name__) class AlreadyRunningError(Exception): """Exception indicating that a background task is already running""" - pass + + message = _('Requested task is already running') + + def __init__(self, message=None): + + if not message: + message = self.message + super(AlreadyRunningError, self).__init__(message) + + +class QueueConnectionError(Exception): + """ + Exception indicating that celery task was not created successfully. + """ + message = _('Error occured. Please try again later.') + + def __init__(self, message=None): + if not message: + message = self.message + super(QueueConnectionError, self).__init__(message) def _task_is_running(course_id, task_type, task_key): @@ -57,7 +76,8 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester): if _task_is_running(course_id, task_type, task_key): log.warning("Duplicate task found for task_type %s and task_key %s", task_type, task_key) - raise AlreadyRunningError("requested task is already running") + error_message = generate_already_running_error_message(task_type) + raise AlreadyRunningError(error_message) try: most_recent_id = InstructorTask.objects.latest('id').id @@ -75,6 +95,37 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester): return InstructorTask.create(course_id, task_type, task_key, task_input, requester) +def generate_already_running_error_message(task_type): + """ + Returns already running error message for given task type. + """ + + message = '' + report_types = { + 'grade_problems': _('problem grade'), + 'problem_responses_csv': _('problem responses'), + 'profile_info_csv': _('enrolled learner profile'), + 'may_enroll_info_csv': _('enrollment'), + 'detailed_enrollment_report': _('detailed enrollment'), + 'exec_summary_report': _('executive summary'), + 'course_survey_report': _('survey'), + 'proctored_exam_results_report': _('proctored exam results'), + 'export_ora2_data': _('ORA data'), + 'grade_course': _('grade'), + + } + + if report_types.get(task_type): + + message = _( + "The {report_type} report is being created. " + "To view the status of the report, see Pending Tasks below. " + "You will be able to download the report when it is complete." + ).format(report_type=report_types.get(task_type)) + + return message + + def _get_xmodule_instance_args(request, task_id): """ Calculate parameters needed for instantiating xmodule instances. @@ -190,6 +241,27 @@ def _update_instructor_task(instructor_task, task_result): instructor_task.save() +def _update_instructor_task_state(instructor_task, task_state, message=None): + """ + Update state and output of InstructorTask object. + """ + instructor_task.task_state = task_state + if message: + instructor_task.task_output = message + + instructor_task.save() + + +def _handle_instructor_task_failure(instructor_task, error): + """ + Do required operations if task creation was not complete. + """ + log.info("instructor task (%s) failed, result: %s", instructor_task.task_id, error.message) + _update_instructor_task_state(instructor_task, FAILURE, error.message) + + raise QueueConnectionError() + + def get_updated_instructor_task(task_id): """ Returns InstructorTask object corresponding to a given `task_id`. @@ -365,6 +437,10 @@ def submit_task(request, task_type, task_class, course_key, task_input, task_key task_id = instructor_task.task_id task_args = [instructor_task.id, _get_xmodule_instance_args(request, task_id)] - task_class.apply_async(task_args, task_id=task_id) + try: + task_class.apply_async(task_args, task_id=task_id) + + except Exception as error: + _handle_instructor_task_failure(instructor_task, error) return instructor_task diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 741de82f4c..e010455ff5 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -32,7 +32,7 @@ from lms.djangoapps.instructor_task.api import ( submit_reset_problem_attempts_for_all_students, submit_reset_problem_attempts_in_entrance_exam ) -from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError +from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask from lms.djangoapps.instructor_task.tasks import export_ora2_data from lms.djangoapps.instructor_task.tests.test_base import ( @@ -43,6 +43,7 @@ from lms.djangoapps.instructor_task.tests.test_base import ( TestReportMixin ) from xmodule.modulestore.exceptions import ItemNotFoundError +from celery.states import FAILURE class InstructorTaskReportTest(InstructorTaskTestCase): @@ -164,15 +165,31 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase): ) @ddt.unpack def test_submit_task(self, task_function, expected_task_type, params=None): + """ + Tests submission of instructor task. + """ if params is None: params = {} if params.get('student'): params['student'] = self.student - # tests submit, and then tests a second identical submission. problem_url_name = 'H1P1' self.define_option_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) + + # unsuccessful submission, exception raised while submitting. + with patch('lms.djangoapps.instructor_task.tasks_base.BaseInstructorTask.apply_async') as apply_async: + + error = Exception() + apply_async.side_effect = error + + with self.assertRaises(QueueConnectionError): + instructor_task = task_function(self.create_task_request(self.instructor), location, **params) + + most_recent_task = InstructorTask.objects.latest('id') + self.assertEquals(most_recent_task.task_state, FAILURE) + + # successful submission instructor_task = task_function(self.create_task_request(self.instructor), location, **params) self.assertEquals(instructor_task.task_type, expected_task_type) diff --git a/lms/static/js/instructor_dashboard/data_download.js b/lms/static/js/instructor_dashboard/data_download.js index bd4e62b4d6..32636a76a4 100644 --- a/lms/static/js/instructor_dashboard/data_download.js +++ b/lms/static/js/instructor_dashboard/data_download.js @@ -122,16 +122,18 @@ }); this.$proctored_exam_csv_btn.click(function() { var url = dataDownloadObj.$proctored_exam_csv_btn.data('endpoint'); + var errorMessage = gettext('Error generating proctored exam results. Please try again.'); return $.ajax({ type: 'POST', dataType: 'json', url: url, - error: function() { + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } dataDownloadObj.clear_display(); - dataDownloadObj.$reports_request_response_error.text( - gettext('Error generating proctored exam results. Please try again.') - ); - return $('.msg-error').css({ + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); }, @@ -146,16 +148,18 @@ }); this.$survey_results_csv_btn.click(function() { var url = dataDownloadObj.$survey_results_csv_btn.data('endpoint'); + var errorMessage = gettext('Error generating survey results. Please try again.'); return $.ajax({ type: 'POST', dataType: 'json', url: url, - error: function() { + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } dataDownloadObj.clear_display(); - dataDownloadObj.$reports_request_response_error.text( - gettext('Error generating survey results. Please try again.') - ); - return $('.msg-error').css({ + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); }, @@ -170,16 +174,18 @@ }); this.$list_studs_csv_btn.click(function() { var url = dataDownloadObj.$list_studs_csv_btn.data('endpoint') + '/csv'; + var errorMessage = gettext('Error generating student profile information. Please try again.'); dataDownloadObj.clear_display(); return $.ajax({ type: 'POST', dataType: 'json', url: url, - error: function() { - dataDownloadObj.$reports_request_response_error.text( - gettext('Error generating student profile information. Please try again.') - ); - return $('.msg-error').css({ + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); }, @@ -201,9 +207,13 @@ url: url, error: function() { dataDownloadObj.clear_display(); - return dataDownloadObj.$download_request_response_error.text( + dataDownloadObj.$download_request_response_error.text( gettext('Error getting student list.') ); + return dataDownloadObj.$download_request_response_error.css({ + display: 'block' + }); + }, success: function(data) { var $tablePlaceholder, columns, feature, gridData, options; @@ -251,7 +261,7 @@ dataDownloadObj.$reports_request_response_error.text( JSON.parse(error.responseText) ); - return $('.msg-error').css({ + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); }, @@ -265,16 +275,18 @@ }); this.$list_may_enroll_csv_btn.click(function() { var url = dataDownloadObj.$list_may_enroll_csv_btn.data('endpoint'); + var errorMessage = gettext('Error generating list of students who may enroll. Please try again.'); dataDownloadObj.clear_display(); return $.ajax({ type: 'POST', dataType: 'json', url: url, - error: function() { - dataDownloadObj.$reports_request_response_error.text( - gettext('Error generating list of students who may enroll. Please try again.') - ); - return $('.msg-error').css({ + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); }, @@ -294,9 +306,12 @@ url: url, error: function() { dataDownloadObj.clear_display(); - return dataDownloadObj.$download_request_response_error.text( + dataDownloadObj.$download_request_response_error.text( gettext('Error retrieving grading configuration.') ); + return dataDownloadObj.$download_request_response_error.css({ + display: 'block' + }); }, success: function(data) { dataDownloadObj.clear_display(); @@ -307,29 +322,27 @@ }); this.$async_report_btn.click(function(e) { var url = $(e.target).data('endpoint'); + var errorMessage = ''; dataDownloadObj.clear_display(); return $.ajax({ type: 'POST', dataType: 'json', url: url, - error: statusAjaxError(function() { - if (e.target.name === 'calculate-grades-csv') { - dataDownloadObj.$grades_request_response_error.text( - gettext('Error generating grades. Please try again.') - ); + error: function(error) { + if (error.responseText) { + errorMessage = JSON.parse(error.responseText); + } else if (e.target.name === 'calculate-grades-csv') { + errorMessage = gettext('Error generating grades. Please try again.'); } else if (e.target.name === 'problem-grade-report') { - dataDownloadObj.$grades_request_response_error.text( - gettext('Error generating problem grade report. Please try again.') - ); + errorMessage = gettext('Error generating problem grade report. Please try again.'); } else if (e.target.name === 'export-ora2-data') { - dataDownloadObj.$grades_request_response_error.text( - gettext('Error generating ORA data report. Please try again.') - ); + errorMessage = gettext('Error generating ORA data report. Please try again.'); } - return $('.msg-error').css({ + dataDownloadObj.$reports_request_response_error.text(errorMessage); + return dataDownloadObj.$reports_request_response_error.css({ display: 'block' }); - }), + }, success: function(data) { dataDownloadObj.$reports_request_response.text(data.status); return $('.msg-confirm').css({ diff --git a/lms/static/js/spec/staff_debug_actions_spec.js b/lms/static/js/spec/staff_debug_actions_spec.js index f6a45fc5c7..53dd08c0f9 100644 --- a/lms/static/js/spec/staff_debug_actions_spec.js +++ b/lms/static/js/spec/staff_debug_actions_spec.js @@ -96,7 +96,7 @@ define([ error_msg: 'Failed to reset attempts for user.' }; StaffDebug.doInstructorDashAction(action); - AjaxHelpers.respondWithError(requests); + AjaxHelpers.respondWithTextError(requests); expect($('#idash_msg').text()).toBe('Failed to reset attempts for user. '); $('#result_' + locationName).remove(); }); diff --git a/lms/static/js/staff_debug_actions.js b/lms/static/js/staff_debug_actions.js index caf4b67750..d242e7f49c 100644 --- a/lms/static/js/staff_debug_actions.js +++ b/lms/static/js/staff_debug_actions.js @@ -54,12 +54,12 @@ var StaffDebug = (function() { try { responseJSON = $.parseJSON(request.responseText); } catch (e) { - responseJSON = {error: gettext('Unknown Error Occurred.')}; + responseJSON = 'Unknown Error Occurred.'; } var text = _.template('{error_msg} {error}', {interpolate: /\{(.+?)\}/g})( { error_msg: action.error_msg, - error: responseJSON.error + error: gettext(responseJSON) } ); var html = _.template('
{text}
', {interpolate: /\{(.+?)\}/g})(