From 8660c9a7afe4095d4292748f14640f01ebaec11d Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Wed, 15 May 2013 02:10:19 -0400 Subject: [PATCH] Check descriptor to identify problems that don't support regrading. --- common/lib/xmodule/xmodule/capa_module.py | 4 +-- .../xmodule/xmodule/tests/test_capa_module.py | 5 ++- lms/djangoapps/courseware/task_queue.py | 35 +++++++++++++------ 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 306fb38d0e..d0a84e7bd5 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -823,7 +823,7 @@ class CapaModule(CapaFields, XModule): {'success' : 'correct' | 'incorrect' | AJAX alert msg string } Raises NotFoundError if called on a problem that has not yet been - answered, or if it's a problem that cannot be regraded. + answered, or NotImplementedError if it's a problem that cannot be regraded. Returns the error messages for exceptions occurring while performing the regrading, rather than throwing them. @@ -835,7 +835,7 @@ class CapaModule(CapaFields, XModule): if not self.lcp.supports_regrading(): event_info['failure'] = 'unsupported' self.system.track_function('problem_regrade_fail', event_info) - raise NotFoundError('Problem does not support regrading') + raise NotImplementedError("Problem's definition does not support regrading") if not self.done: event_info['failure'] = 'unanswered' diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 2a31b6478a..8dd1a37595 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -642,13 +642,12 @@ class CapaModuleTest(unittest.TestCase): module.regrade_problem() def test_regrade_problem_not_supported(self): - # Simulate that the problem is NOT done module = CapaFactory.create(done=True) # Try to regrade the problem, and get exception with patch('capa.capa_problem.LoncapaProblem.supports_regrading') as mock_supports_regrading: mock_supports_regrading.return_value = False - with self.assertRaises(xmodule.exceptions.NotFoundError): + with self.assertRaises(NotImplementedError): module.regrade_problem() def test_regrade_problem_error(self): @@ -668,7 +667,7 @@ class CapaModuleTest(unittest.TestCase): # Expect an AJAX alert message in 'success' expected_msg = 'Error: test error' - self.assertEqual(expected_msg, result['success']) + self.assertEqual(result['success'], expected_msg) # Expect that the number of attempts is NOT incremented self.assertEqual(module.attempts, 1) diff --git a/lms/djangoapps/courseware/task_queue.py b/lms/djangoapps/courseware/task_queue.py index 06522d57e5..d846375c27 100644 --- a/lms/djangoapps/courseware/task_queue.py +++ b/lms/djangoapps/courseware/task_queue.py @@ -297,6 +297,26 @@ def _get_task_completion_message(course_task_log_entry): ########### Add task-submission methods here: +def _check_arguments_for_regrading(course_id, problem_url): + """ + Do simple checks on the descriptor to confirm that it supports regrading. + + Confirms first that the problem_url is defined (since that's currently typed + in). An ItemNotFoundException is raised if the corresponding module + descriptor doesn't exist. NotImplementedError is returned if the + corresponding module doesn't support regrading calls. + """ + descriptor = modulestore().get_instance(course_id, problem_url) + supports_regrade = False + if hasattr(descriptor,'module_class'): + module_class = descriptor.module_class + if hasattr(module_class, 'regrade_problem'): + supports_regrade = True + + if not supports_regrade: + msg = "Specified module does not support regrading." + raise NotImplementedError(msg) + def submit_regrade_problem_for_student(request, course_id, problem_url, student): """ @@ -309,10 +329,8 @@ def submit_regrade_problem_for_student(request, course_id, problem_url, student) An exception is thrown if the problem doesn't exist, or if the particular problem is already being regraded for this student. """ - # check arguments: make sure that the problem_url is defined - # (since that's currently typed in). If the corresponding module descriptor doesn't exist, - # an exception will be raised. Let it pass up to the caller. - modulestore().get_instance(course_id, problem_url) + # check arguments: let exceptions return up to the caller. + _check_arguments_for_regrading(course_id, problem_url) task_name = 'regrade_problem' @@ -341,14 +359,11 @@ def submit_regrade_problem_for_all_students(request, course_id, problem_url): An exception is thrown if the problem doesn't exist, or if the particular problem is already being regraded. """ - # check arguments: make sure that the problem_url is defined - # (since that's currently typed in). If the corresponding module descriptor doesn't exist, - # an exception will be raised. Let it pass up to the caller. - modulestore().get_instance(course_id, problem_url) - - task_name = 'regrade_problem' + # check arguments: let exceptions return up to the caller. + _check_arguments_for_regrading(course_id, problem_url) # check to see if task is already running, and reserve it otherwise + task_name = 'regrade_problem' course_task_log = _reserve_task(course_id, task_name, problem_url, request.user) # Submit task: