From dfa435012b75b187bd8ed7e510ce1518a5a70048 Mon Sep 17 00:00:00 2001 From: Adam Palay Date: Wed, 3 Jul 2013 18:01:11 -0400 Subject: [PATCH] notify students for NotFoundErrors from capa_module, improve error logging --- common/lib/xmodule/xmodule/capa_module.py | 19 +++- lms/djangoapps/courseware/module_render.py | 11 ++- .../tests/test_submitting_problems.py | 96 +++++++++++++++++-- 3 files changed, 109 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index ef3e1999c8..51c1a396c3 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -152,6 +152,7 @@ class CapaFields(object): scope=Scope.settings ) + class CapaModule(CapaFields, XModule): """ An XModule implementing LonCapa format problems, implemented by way of @@ -552,6 +553,16 @@ class CapaModule(CapaFields, XModule): 'ungraded_response': self.handle_ungraded_response } + generic_error_message = ( + "We're sorry, there was an error with processing your request. " + "Please try reloading your page and trying again." + ) + + not_found_error_message = ( + "The state of this problem has changed since you loaded this page. " + "Please refresh your page." + ) + if dispatch not in handlers: return 'Error' @@ -559,9 +570,14 @@ class CapaModule(CapaFields, XModule): try: result = handlers[dispatch](data) + + except NotFoundError as err: + _, _, traceback_obj = sys.exc_info() + raise ProcessingError, (not_found_error_message, err), traceback_obj + except Exception as err: _, _, traceback_obj = sys.exc_info() - raise ProcessingError(err.message, traceback_obj) + raise ProcessingError, (generic_error_message, err), traceback_obj after = self.get_progress() @@ -1125,7 +1141,6 @@ class CapaDescriptor(CapaFields, RawDescriptor): problem.rerandomize = "always" return problem - @property def non_editable_metadata_fields(self): non_editable_fields = super(CapaDescriptor, self).non_editable_metadata_fields diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index db7ba1641e..c343701a94 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -10,7 +10,7 @@ from django.core.cache import cache from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import Http404 -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpResponse from django.views.decorators.csrf import csrf_exempt import pyparsing @@ -38,6 +38,7 @@ from courseware.masquerade import setup_masquerade from courseware.model_data import LmsKeyValueStore, LmsUsage, ModelDataCache from courseware.models import StudentModule from util.sandboxing import can_execute_unsafe_code +from util.json_request import JsonResponse log = logging.getLogger(__name__) @@ -509,11 +510,11 @@ def modx_dispatch(request, dispatch, location, course_id): log.exception("Module indicating to user that request doesn't exist") raise Http404 - # For XModule-specific errors, we respond with 400 - except ProcessingError: - log.warning("Module encountered an error while prcessing AJAX call", + # For XModule-specific errors, we log the error and respond with an error message + except ProcessingError as err: + log.warning("Module encountered an error while processing AJAX call", exc_info=True) - return HttpResponseBadRequest() + return JsonResponse(object={'success': err.args[0]}, status=200) # If any other error occurred, re-raise it to trigger a 500 response except: diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 0ed37cdd5c..9081a910c9 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -107,6 +107,15 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): resp = self.client.post(modx_url) return resp + def show_question_answer(self, problem_url_name): + """ + Shows the answer to the current student. + """ + problem_location = self.problem_location(problem_url_name) + modx_url = self.modx_url(problem_location, 'problem_show') + resp = self.client.post(modx_url) + return resp + def add_dropdown_to_section(self, section_location, name, num_inputs=2): """ Create and return a dropdown problem. @@ -131,7 +140,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): parent_location=section_location, category='problem', data=prob_xml, - metadata={'randomize': 'always'}, + metadata={'rerandomize': 'always'}, display_name=name ) @@ -139,7 +148,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): self.refresh_course() return problem - def add_graded_section_to_course(self, name, section_format='Homework'): + def add_graded_section_to_course(self, name, section_format='Homework', late=False, reset=False, showanswer=False): """ Creates a graded homework section within a chapter and returns the section. """ @@ -151,12 +160,44 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase): category='chapter' ) - section = ItemFactory.create( - parent_location=self.chapter.location, - display_name=name, - category='sequential', - metadata={'graded': True, 'format': section_format} - ) + if late: + section = ItemFactory.create( + parent_location=self.chapter.location, + display_name=name, + category='sequential', + metadata={'graded': True, 'format': section_format, 'due': '2013-05-20T23:30'} + ) + elif reset: + section = ItemFactory.create( + parent_location=self.chapter.location, + display_name=name, + category='sequential', + rerandomize='always', + metadata={ + 'graded': True, + 'format': section_format, + } + ) + + elif showanswer: + section = ItemFactory.create( + parent_location=self.chapter.location, + display_name=name, + category='sequential', + showanswer='never', + metadata={ + 'graded': True, + 'format': section_format, + } + ) + + else: + section = ItemFactory.create( + parent_location=self.chapter.location, + display_name=name, + category='sequential', + metadata={'graded': True, 'format': section_format} + ) # now that we've added the problem and section to the course # we fetch the course from the database so the object we are @@ -257,7 +298,7 @@ class TestCourseGrader(TestSubmittingProblems): hw_section = next(section for section in sections_list if section.get('url_name') == hw_url_name) return [s.earned for s in hw_section['scores']] - def basic_setup(self): + def basic_setup(self, late=False, reset=False, showanswer=False): """ Set up a simple course for testing basic grading functionality. """ @@ -278,7 +319,7 @@ class TestCourseGrader(TestSubmittingProblems): self.add_grading_policy(grading_policy) # set up a simple course with four problems - self.homework = self.add_graded_section_to_course('homework') + self.homework = self.add_graded_section_to_course('homework', late=late, reset=reset, showanswer=showanswer) self.add_dropdown_to_section(self.homework.location, 'p1', 1) self.add_dropdown_to_section(self.homework.location, 'p2', 1) self.add_dropdown_to_section(self.homework.location, 'p3', 1) @@ -346,6 +387,41 @@ class TestCourseGrader(TestSubmittingProblems): self.add_dropdown_to_section(self.homework3.location, self.hw3_names[0], 1) self.add_dropdown_to_section(self.homework3.location, self.hw3_names[1], 1) + def test_submission_late(self): + """Test problem for due date in the past""" + self.basic_setup(late=True) + resp = self.submit_question_answer('p1', {'2_1': 'Correct'}) + self.assertEqual(resp.status_code, 200) + err_msg = ( + "The state of this problem has changed since you loaded this page. " + "Please refresh your page." + ) + self.assertEqual(json.loads(resp.content).get("success"), err_msg) + + def test_submission_reset(self): + """Test problem ProcessingErrors due to resets""" + self.basic_setup(reset=True) + resp = self.submit_question_answer('p1', {'2_1': 'Correct'}) + # submit a second time to draw NotFoundError + resp = self.submit_question_answer('p1', {'2_1': 'Correct'}) + self.assertEqual(resp.status_code, 200) + err_msg = ( + "The state of this problem has changed since you loaded this page. " + "Please refresh your page." + ) + self.assertEqual(json.loads(resp.content).get("success"), err_msg) + + def test_submission_show_answer(self): + """Test problem for ProcessingErrors due to showing answer""" + self.basic_setup(showanswer=True) + resp = self.show_question_answer('p1') + self.assertEqual(resp.status_code, 200) + err_msg = ( + "The state of this problem has changed since you loaded this page. " + "Please refresh your page." + ) + self.assertEqual(json.loads(resp.content).get("success"), err_msg) + def test_none_grade(self): """ Check grade is 0 to begin with.