diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index 6676eeeb33..cf7af2e55b 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py @@ -71,10 +71,15 @@ class OpenEndedModule(openendedchild.OpenEndedChild): self._parse(oeparam, self.child_prompt, self.child_rubric, system) + # If there are multiple tasks (like self-assessment followed by ai), once + # the the status of the first task is set to DONE, setup_next_task() will + # create the OpenEndedChild with parameter child_created=True so that the + # submission can be sent to the grader. Keep trying each time this module + # is loaded until it succeeds. if self.child_created is True and self.child_state == self.ASSESSING: - self.child_created = False - self.send_to_grader(self.latest_answer(), system) - self.child_created = False + success, message = self.send_to_grader(self.latest_answer(), system) + if success: + self.child_created = False def _parse(self, oeparam, prompt, rubric, system): ''' @@ -196,20 +201,25 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'student_info': json.dumps(student_info), } - (error, msg) = qinterface.send_to_queue( + error, error_message = qinterface.send_to_queue( header=xheader, body=json.dumps(contents) ) # Convert error to a success value success = True + message = "Successfully saved your feedback." if error: success = False - - self.child_state = self.DONE + message = "Unable to save your feedback. Please try again later." + log.error("Unable to send feedback to grader. location: {0}, error_message: {1}".format( + self.location_string, error_message + )) + else: + self.child_state = self.DONE # This is a student_facing_message - return {'success': success, 'msg': "Successfully submitted your feedback."} + return {'success': success, 'msg': message} def send_to_grader(self, submission, system): """ @@ -257,7 +267,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): }) # Submit request. When successful, 'msg' is the prior length of the queue - qinterface.send_to_queue( + error, error_message = qinterface.send_to_queue( header=xheader, body=json.dumps(contents) ) @@ -267,7 +277,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild): 'key': queuekey, 'time': qtime, } - return True + + success = True + message = "Successfully saved your submission." + if error: + success = False + message = 'Unable to submit your submission to grader. Please try again later.' + log.error("Unable to submit to grader. location: {0}, error_message: {1}".format( + self.location_string, error_message + )) + + return (success, message) def _update_score(self, score_msg, queuekey, system): """ @@ -671,17 +691,26 @@ class OpenEndedModule(openendedchild.OpenEndedChild): if self.child_state != self.INITIAL: return self.out_of_sync_error(data) + message = "Successfully saved your submission." + # add new history element with answer and empty score and hint. success, error_message, data = self.append_file_link_to_student_answer(data) - if success: + if not success: + message = error_message + else: data['student_answer'] = OpenEndedModule.sanitize_html(data['student_answer']) - self.new_history_entry(data['student_answer']) - self.send_to_grader(data['student_answer'], system) - self.change_state(self.ASSESSING) + success, error_message = self.send_to_grader(data['student_answer'], system) + if not success: + message = error_message + # Store the answer instead + self.store_answer(data, system) + else: + self.new_history_entry(data['student_answer']) + self.change_state(self.ASSESSING) return { 'success': success, - 'error': error_message, + 'error': message, 'student_response': data['student_answer'].replace("\n", "
") } diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index 3c17f52d4a..aa4b8b5d99 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -283,7 +283,7 @@ class OpenEndedChild(object): 'child_state': self.child_state, 'max_score': self._max_score, 'child_attempts': self.child_attempts, - 'child_created': False, + 'child_created': self.child_created, 'stored_answer': self.stored_answer, } return json.dumps(state) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 857942864b..f08af88a87 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py @@ -254,7 +254,7 @@ class OpenEndedModuleTest(unittest.TestCase): self.test_system.open_ended_grading_interface = None self.test_system.location = self.location self.mock_xqueue = MagicMock() - self.mock_xqueue.send_to_queue.return_value = (None, "Message") + self.mock_xqueue.send_to_queue.return_value = (0, "Queued") def constructed_callback(dispatch="score_update"): return dispatch @@ -265,50 +265,87 @@ class OpenEndedModuleTest(unittest.TestCase): self.openendedmodule = OpenEndedModule(self.test_system, self.location, self.definition, self.descriptor, self.static_data, self.metadata) - # Disabled 1/27/14 due to flakiness in master - # Should not be comparing the submission time to the current time! - @unittest.skip def test_message_post(self): - get = {'feedback': 'feedback text', - 'submission_id': '1', - 'grader_id': '1', - 'score': 3} - qtime = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat) - student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, - 'submission_time': qtime} - contents = { - 'feedback': get['feedback'], - 'submission_id': int(get['submission_id']), - 'grader_id': int(get['grader_id']), - 'score': get['score'], - 'student_info': json.dumps(student_info) - } + """Test message_post() sends feedback to xqueue.""" - result = self.openendedmodule.message_post(get, self.test_system) + submission_time = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat) + + feedback_post = { + 'feedback': 'feedback text', + 'submission_id': '1', + 'grader_id': '1', + 'score': 3 + } + result = self.openendedmodule.message_post(feedback_post, self.test_system) self.assertTrue(result['success']) + # make sure it's actually sending something we want to the queue - self.mock_xqueue.send_to_queue.assert_called_with(body=json.dumps(contents), header=ANY) + mock_send_to_queue_body_arg = json.loads(self.mock_xqueue.send_to_queue.call_args[1]['body']) + self.assertEqual(mock_send_to_queue_body_arg['feedback'], feedback_post['feedback']) + self.assertEqual(mock_send_to_queue_body_arg['submission_id'], int(feedback_post['submission_id'])) + self.assertEqual(mock_send_to_queue_body_arg['grader_id'], int(feedback_post['grader_id'])) + self.assertEqual(mock_send_to_queue_body_arg['score'], feedback_post['score']) + body_arg_student_info = json.loads(mock_send_to_queue_body_arg['student_info']) + self.assertEqual(body_arg_student_info['anonymous_student_id'], self.test_system.anonymous_student_id) + self.assertGreaterEqual(body_arg_student_info['submission_time'], submission_time) state = json.loads(self.openendedmodule.get_instance_state()) - self.assertIsNotNone(state['child_state'], OpenEndedModule.DONE) + self.assertEqual(state['child_state'], OpenEndedModule.DONE) + + def test_message_post_fail(self): + """Test message_post() if unable to send feedback to xqueue.""" + + self.mock_xqueue.send_to_queue.return_value = (1, "Not Queued") + + feedback_post = { + 'feedback': 'feedback text', + 'submission_id': '1', + 'grader_id': '1', + 'score': 3 + } + result = self.openendedmodule.message_post(feedback_post, self.test_system) + self.assertFalse(result['success']) + + state = json.loads(self.openendedmodule.get_instance_state()) + self.assertNotEqual(state['child_state'], OpenEndedModule.DONE) - # Disabled 1/27/14 due to flakiness in master - # Should not be comparing the submission time to the current time! - @unittest.skip def test_send_to_grader(self): - submission = "This is a student submission" - qtime = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat) - student_info = {'anonymous_student_id': self.test_system.anonymous_student_id, - 'submission_time': qtime} - contents = self.openendedmodule.payload.copy() - contents.update({ - 'student_info': json.dumps(student_info), - 'student_response': submission, - 'max_score': self.max_score - }) - result = self.openendedmodule.send_to_grader(submission, self.test_system) + student_response = "This is a student submission" + submission_time = datetime.strftime(datetime.now(UTC), xqueue_interface.dateformat) + + result, __ = self.openendedmodule.send_to_grader(student_response, self.test_system) self.assertTrue(result) - self.mock_xqueue.send_to_queue.assert_called_with(body=json.dumps(contents), header=ANY) + + mock_send_to_queue_body_arg = json.loads(self.mock_xqueue.send_to_queue.call_args[1]['body']) + self.assertEqual(mock_send_to_queue_body_arg['student_response'], student_response) + self.assertEqual(mock_send_to_queue_body_arg['max_score'], self.max_score) + body_arg_student_info = json.loads(mock_send_to_queue_body_arg['student_info']) + self.assertEqual(body_arg_student_info['anonymous_student_id'], self.test_system.anonymous_student_id) + self.assertGreaterEqual(body_arg_student_info['submission_time'], submission_time) + + def test_send_to_grader_fail(self): + """Test send_to_grader() if unable to send submission to xqueue.""" + + student_response = "This is a student submission" + self.mock_xqueue.send_to_queue.return_value = (1, "Not Queued") + result, __ = self.openendedmodule.send_to_grader(student_response, self.test_system) + self.assertFalse(result) + + def test_save_answer_fail(self): + """Test save_answer() if unable to send submission to grader.""" + + submission = "This is a student submission" + self.openendedmodule.send_to_grader = Mock(return_value=(False, "Failed")) + response = self.openendedmodule.save_answer( + {"student_answer": submission}, + get_test_system() + ) + self.assertFalse(response['success']) + self.assertNotEqual(self.openendedmodule.latest_answer(), submission) + self.assertEqual(self.openendedmodule.stored_answer, submission) + state = json.loads(self.openendedmodule.get_instance_state()) + self.assertEqual(state['child_state'], OpenEndedModule.INITIAL) + self.assertEqual(state['stored_answer'], submission) def update_score_single(self): self.openendedmodule.new_history_entry("New Entry") @@ -384,7 +421,7 @@ class OpenEndedModuleTest(unittest.TestCase): self.assertEqual(test_module.get_display_answer(), saved_response) # Mock out the send_to_grader function so it doesn't try to connect to the xqueue. - test_module.send_to_grader = Mock(return_value=True) + test_module.send_to_grader = Mock(return_value=(True, "Success")) # Submit a student response to the question. test_module.handle_ajax( "save_answer", @@ -815,7 +852,6 @@ class CombinedOpenEndedModuleConsistencyTest(unittest.TestCase): ''' - static_data = { 'max_attempts': 20, 'prompt': prompt, @@ -934,11 +970,16 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): hint = "blah" def get_module_system(self, descriptor): + + def construct_callback(dispatch="score_update"): + return dispatch + test_system = get_test_system() test_system.open_ended_grading_interface = None test_system.xqueue['interface'] = Mock( - send_to_queue=Mock(side_effect=[1, "queued"]) + send_to_queue=Mock(return_value=(0, "Queued")) ) + test_system.xqueue['construct_callback'] = construct_callback return test_system @@ -1009,6 +1050,96 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): self._handle_ajax("reset", {}) self.assertEqual(self._module().current_task_number, 0) + def test_open_ended_flow_with_xqueue_failure(self): + """ + Test a two step problem where the student first goes through the self assessment step, and then the + open ended step with the xqueue failing in the first step. + """ + assessment = [1, 1] + + # Simulate a student saving an answer + self._handle_ajax("save_answer", {"student_answer": self.answer}) + status = self._handle_ajax("get_status", {}) + self.assertIsInstance(status, basestring) + + # Mock a student submitting an assessment + assessment_dict = MultiDict({'assessment': sum(assessment)}) + assessment_dict.extend(('score_list[]', val) for val in assessment) + + mock_xqueue_interface = Mock( + send_to_queue=Mock(return_value=(1, "Not Queued")) + ) + + # Call handle_ajax on the module with xqueue down + module = self._module() + with patch.dict(module.xmodule_runtime.xqueue, {'interface': mock_xqueue_interface}): + module.handle_ajax("save_assessment", assessment_dict) + self.assertEqual(module.current_task_number, 1) + self.assertTrue((module.child_module.get_task_number(1).child_created)) + module.save() + + # Check that next time the OpenEndedModule is loaded it calls send_to_grader + with patch.object(OpenEndedModule, 'send_to_grader') as mock_send_to_grader: + mock_send_to_grader.return_value = (False, "Not Queued") + module = self._module().child_module.get_score() + self.assertTrue(mock_send_to_grader.called) + self.assertTrue((self._module().child_module.get_task_number(1).child_created)) + + # Loading it this time should send submission to xqueue correctly + self.assertFalse((self._module().child_module.get_task_number(1).child_created)) + self.assertEqual(self._module().current_task_number, 1) + self.assertEqual(self._module().state, OpenEndedChild.ASSESSING) + + task_one_json = json.loads(self._module().task_states[0]) + self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) + + # Move to the next step in the problem + self._handle_ajax("next_problem", {}) + self.assertEqual(self._module().current_task_number, 1) + self._module().render('student_view') + + # Try to get the rubric from the module + self._handle_ajax("get_combined_rubric", {}) + + self.assertEqual(self._module().state, OpenEndedChild.ASSESSING) + + # Make a fake reply from the queue + queue_reply = { + 'queuekey': "", + 'xqueue_body': json.dumps({ + 'score': 0, + 'feedback': json.dumps({"spelling": "Spelling: Ok.", "grammar": "Grammar: Ok.", + "markup-text": " all of us can think of a book that we hope none of our children or any other children have taken off the shelf . but if i have the right to remove that book from the shelf that work i abhor then you also have exactly the same right and so does everyone else . and then we have no books left on the shelf for any of us . katherine paterson , author write a persuasive essay to a newspaper reflecting your vies on censorship in libraries . do you believe that certain materials , such as books , music , movies , magazines , etc . , should be removed from the shelves if they are found offensive ? support your position with convincing arguments from your own experience , observations , and or reading . "}), + 'grader_type': "ML", + 'success': True, + 'grader_id': 1, + 'submission_id': 1, + 'rubric_xml': "Writing Applications0 Language Conventions 0", + 'rubric_scores_complete': True, + }) + } + + self._handle_ajax("check_for_score", {}) + + # Update the module with the fake queue reply + self._handle_ajax("score_update", queue_reply) + + module = self._module() + self.assertFalse(module.ready_to_reset) + self.assertEqual(module.current_task_number, 1) + + # Get html and other data client will request + module.render('student_view') + + self._handle_ajax("skip_post_assessment", {}) + + # Get all results + self._handle_ajax("get_combined_rubric", {}) + + # reset the problem + self._handle_ajax("reset", {}) + self.assertEqual(self._module().state, "initial") + def test_open_ended_flow_correct(self): """ Test a two step problem where the student first goes through the self assessment step, and then the @@ -1032,17 +1163,9 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore): self.assertEqual(json.loads(task_one_json['child_history'][0]['post_assessment']), assessment) # Move to the next step in the problem - try: - self._handle_ajax("next_problem", {}) - except GradingServiceError: - # This error is okay. We don't have a grading service to connect to! - pass + self._handle_ajax("next_problem", {}) self.assertEqual(self._module().current_task_number, 1) - try: - self._module().render('student_view') - except GradingServiceError: - # This error is okay. We don't have a grading service to connect to! - pass + self._module().render('student_view') # Try to get the rubric from the module self._handle_ajax("get_combined_rubric", {}) @@ -1098,7 +1221,7 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore): test_system = get_test_system() test_system.open_ended_grading_interface = None test_system.xqueue['interface'] = Mock( - send_to_queue=Mock(side_effect=[1, "queued"]) + send_to_queue=Mock(return_value=(0, "Queued")) ) return test_system @@ -1172,7 +1295,7 @@ class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore): test_system.open_ended_grading_interface = None test_system.s3_interface = test_util_open_ended.S3_INTERFACE test_system.xqueue['interface'] = Mock( - send_to_queue=Mock(side_effect=[1, "queued"]) + send_to_queue=Mock(return_value=(0, "Queued")) ) return test_system