From 94fea8e822424786eb45ae641ccbacf024537107 Mon Sep 17 00:00:00 2001 From: Gabe Mulley Date: Tue, 25 Feb 2014 14:53:50 -0500 Subject: [PATCH] Add map to problem_check event with descriptions of student answers The map contains a human readable description of the answer if necessary. It is useful for problem like multiple choice, when the response of the student is replaced by a moniker. For example "choice_0" instead of the full text. Fixes: AN-587 --- common/lib/capa/capa/inputtypes.py | 17 +- common/lib/xmodule/xmodule/capa_base.py | 88 ++++++- .../xmodule/xmodule/tests/test_capa_module.py | 238 +++++++++++++++++- 3 files changed, 332 insertions(+), 11 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index f425b96fef..790e93679d 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -288,6 +288,14 @@ class InputTypeBase(object): html = self.capa_system.render_template(self.template, context) return etree.XML(html) + def get_user_visible_answer(self, internal_answer): + """ + Given the internal representation of the answer provided by the user, return the representation of the answer + as the user saw it. Subclasses should override this method if and only if the internal represenation of the + answer is different from the answer that is displayed to the user. + """ + return internal_answer + #----------------------------------------------------------------------------- @@ -385,6 +393,7 @@ class ChoiceGroup(InputTypeBase): raise Exception("ChoiceGroup: unexpected tag {0}".format(self.tag)) self.choices = self.extract_choices(self.xml) + self._choices_map = dict(self.choices) # pylint: disable=attribute-defined-outside-init @classmethod def get_attributes(cls): @@ -419,6 +428,12 @@ class ChoiceGroup(InputTypeBase): choices.append((choice.get("name"), stringify_children(choice))) return choices + def get_user_visible_answer(self, internal_answer): + if isinstance(internal_answer, basestring): + return self._choices_map[internal_answer] + + return [self._choices_map[i] for i in internal_answer] + #----------------------------------------------------------------------------- @@ -1021,7 +1036,7 @@ class ChemicalEquationInput(InputTypeBase): """ Can set size of text field. """ - return [Attribute('size', '20'), + return [Attribute('size', '20'), Attribute('label', ''),] def _extra_context(self): diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py index 9f43632a74..edc0449af6 100644 --- a/common/lib/xmodule/xmodule/capa_base.py +++ b/common/lib/xmodule/xmodule/capa_base.py @@ -866,7 +866,8 @@ class CapaMixin(CapaFields): event_info['problem_id'] = self.location.url() answers = self.make_dict_of_responses(data) - event_info['answers'] = convert_files_to_filenames(answers) + answers_without_files = convert_files_to_filenames(answers) + event_info['answers'] = answers_without_files _ = self.runtime.service(self, "i18n").ugettext @@ -944,6 +945,7 @@ class CapaMixin(CapaFields): event_info['correct_map'] = correct_map.get_dict() event_info['success'] = success event_info['attempts'] = self.attempts + event_info['submission'] = self.get_submission_metadata_safe(answers_without_files, correct_map) self.runtime.track_function('problem_check', event_info) if hasattr(self.runtime, 'psychometrics_handler'): # update PsychometricsData using callback @@ -957,6 +959,90 @@ class CapaMixin(CapaFields): 'contents': html, } + def get_submission_metadata_safe(self, answers, correct_map): + """ + Ensures that no exceptions are thrown while generating input metadata summaries. Returns the + summary if it is successfully created, otherwise an empty dictionary. + """ + try: + return self.get_submission_metadata(answers, correct_map) + except Exception: # pylint: disable=broad-except + # NOTE: The above process requires deep inspection of capa structures that may break for some + # uncommon problem types. Ensure that it does not prevent answer submission in those + # cases. Any occurrences of errors in this block should be investigated and resolved. + log.exception('Unable to gather submission metadata, it will not be included in the event.') + + return {} + + def get_submission_metadata(self, answers, correct_map): + """ + Return a map of inputs to their corresponding summarized metadata. + + Returns: + A map whose keys are a unique identifier for the input (in this case a capa input_id) and + whose values are: + + question (str): Is the prompt that was presented to the student. It corresponds to the + label of the input. + answer (mixed): Is the answer the student provided. This may be a rich structure, + however it must be json serializable. + response_type (str): The XML tag of the capa response type. + input_type (str): The XML tag of the capa input type. + correct (bool): Whether or not the provided answer is correct. Will be an empty + string if correctness could not be determined. + variant (str): In some cases the same question can have several different variants. + This string should uniquely identify the variant of the question that was answered. + In the capa context this corresponds to the `seed`. + + This function attempts to be very conservative and make very few assumptions about the structure + of the problem. If problem related metadata cannot be located it should be replaced with empty + strings ''. + """ + + input_metadata = {} + for input_id, internal_answer in answers.iteritems(): + answer_input = self.lcp.inputs.get(input_id) + + if answer_input is None: + log.warning('Input id %s is not mapped to an input type.', input_id) + + answer_response = None + for response, responder in self.lcp.responders.iteritems(): + for other_input_id in self.lcp.responder_answers[response]: + if other_input_id == input_id: + answer_response = responder + + if answer_response is None: + log.warning('Answer responder could not be found for input_id %s.', input_id) + + user_visible_answer = internal_answer + if hasattr(answer_input, 'get_user_visible_answer'): + user_visible_answer = answer_input.get_user_visible_answer(internal_answer) + + # If this problem has rerandomize enabled, then it will generate N variants of the + # question, one per unique seed value. In this case we would like to know which + # variant was selected. Ideally it would be nice to have the exact question that + # was presented to the user, with values interpolated etc, but that can be done + # later if necessary. + variant = '' + if self.rerandomize != 'never': + variant = self.seed + + is_correct = correct_map.is_correct(input_id) + if is_correct is None: + is_correct = '' + + input_metadata[input_id] = { + 'question': getattr(answer_input, 'loaded_attributes', {}).get('label', ''), + 'answer': user_visible_answer, + 'response_type': getattr(getattr(answer_response, 'xml', None), 'tag', ''), + 'input_type': getattr(answer_input, 'tag', ''), + 'correct': is_correct, + 'variant': variant, + } + + return input_metadata + def rescore_problem(self): """ Checks whether the existing answers to a problem are correct. diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index 8c103e0ed5..8f5094ccd1 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -58,21 +58,22 @@ class CapaFactory(object): return cls.num @classmethod - def input_key(cls, input_num=2): + def input_key(cls, response_num=2, input_num=1): """ Return the input key to use when passing GET parameters """ - return ("input_" + cls.answer_key(input_num)) + return ("input_" + cls.answer_key(response_num, input_num)) @classmethod - def answer_key(cls, input_num=2): + def answer_key(cls, response_num=2, input_num=1): """ Return the key stored in the capa problem answer dict """ return ( - "%s_%d_1" % ( + "%s_%d_%d" % ( "-".join(['i4x', 'edX', 'capa_test', 'problem', 'SampleProblem%d' % cls.num]), - input_num, + response_num, + input_num ) ) @@ -544,8 +545,8 @@ class CapaModuleTest(unittest.TestCase): # Create a request dictionary for check_problem. get_request_dict = { - CapaFactoryWithFiles.input_key(input_num=2): fileobjs, - CapaFactoryWithFiles.input_key(input_num=3): 'None', + CapaFactoryWithFiles.input_key(response_num=2): fileobjs, + CapaFactoryWithFiles.input_key(response_num=3): 'None', } module.check_problem(get_request_dict) @@ -594,8 +595,8 @@ class CapaModuleTest(unittest.TestCase): # Create a webob Request with the files uploaded. post_data = [] for fname, fileobj in zip(fnames, fileobjs): - post_data.append((CapaFactoryWithFiles.input_key(input_num=2), (fname, fileobj))) - post_data.append((CapaFactoryWithFiles.input_key(input_num=3), 'None')) + post_data.append((CapaFactoryWithFiles.input_key(response_num=2), (fname, fileobj))) + post_data.append((CapaFactoryWithFiles.input_key(response_num=3), 'None')) request = webob.Request.blank("/some/fake/url", POST=post_data, content_type='multipart/form-data') module.handle('xmodule_handler', request, 'problem_check') @@ -1402,3 +1403,222 @@ class ComplexEncoderTest(unittest.TestCase): expected_str = '1-1*j' json_str = json.dumps(complex_num, cls=ComplexEncoder) self.assertEqual(expected_str, json_str[1:-1]) # ignore quotes + + +class TestProblemCheckTracking(unittest.TestCase): + """ + Ensure correct tracking information is included in events emitted during problem checks. + """ + + def setUp(self): + self.maxDiff = None + + def test_choice_answer_text(self): + factory = self.capa_factory_for_problem_xml("""\ + +

What color is the open ocean on a sunny day?

+ + + +

Which piece of furniture is built for sitting?

+ + + + a table + + + a desk + + + a chair + + + a bookshelf + + + +

Which of the following are musical instruments?

+ + + a piano + a tree + a guitar + a window + + + + """) + module = factory.create() + + answer_input_dict = { + factory.input_key(2): 'blue', + factory.input_key(3): 'choice_0', + factory.input_key(4): ['choice_0', 'choice_1'], + } + + event = self.get_event_for_answers(module, answer_input_dict) + + self.assertEquals(event['submission'], { + factory.answer_key(2): { + 'question': 'What color is the open ocean on a sunny day?', + 'answer': 'blue', + 'response_type': 'optionresponse', + 'input_type': 'optioninput', + 'correct': True, + 'variant': '', + }, + factory.answer_key(3): { + 'question': '', + 'answer': u'a table', + 'response_type': 'multiplechoiceresponse', + 'input_type': 'choicegroup', + 'correct': False, + 'variant': '', + }, + factory.answer_key(4): { + 'question': 'Which of the following are musical instruments?', + 'answer': [u'a piano', u'a tree'], + 'response_type': 'choiceresponse', + 'input_type': 'checkboxgroup', + 'correct': False, + 'variant': '', + }, + }) + + def capa_factory_for_problem_xml(self, xml): + class CustomCapaFactory(CapaFactory): + """ + A factory for creating a Capa problem with arbitrary xml. + """ + sample_problem_xml = textwrap.dedent(xml) + + return CustomCapaFactory + + def get_event_for_answers(self, module, answer_input_dict): + with patch.object(module.runtime, 'track_function') as mock_track_function: + module.check_problem(answer_input_dict) + + self.assertEquals(len(mock_track_function.mock_calls), 1) + mock_call = mock_track_function.mock_calls[0] + event = mock_call[1][1] + + return event + + def test_numerical_textline(self): + factory = CapaFactory + module = factory.create() + + answer_input_dict = { + factory.input_key(2): '3.14' + } + + event = self.get_event_for_answers(module, answer_input_dict) + self.assertEquals(event['submission'], { + factory.answer_key(2): { + 'question': '', + 'answer': '3.14', + 'response_type': 'numericalresponse', + 'input_type': 'textline', + 'correct': True, + 'variant': '', + } + }) + + def test_multiple_inputs(self): + factory = self.capa_factory_for_problem_xml("""\ + +

Choose the correct color

+ +

What color is the sky?

+ +

What color are pine needles?

+ + + + """) + module = factory.create() + + answer_input_dict = { + factory.input_key(2, 1): 'blue', + factory.input_key(2, 2): 'yellow', + } + + event = self.get_event_for_answers(module, answer_input_dict) + self.assertEquals(event['submission'], { + factory.answer_key(2, 1): { + 'question': '', + 'answer': 'blue', + 'response_type': 'optionresponse', + 'input_type': 'optioninput', + 'correct': True, + 'variant': '', + }, + factory.answer_key(2, 2): { + 'question': '', + 'answer': 'yellow', + 'response_type': 'optionresponse', + 'input_type': 'optioninput', + 'correct': False, + 'variant': '', + }, + }) + + def test_rerandomized_inputs(self): + factory = CapaFactory + module = factory.create(rerandomize='always') + + answer_input_dict = { + factory.input_key(2): '3.14' + } + + event = self.get_event_for_answers(module, answer_input_dict) + self.assertEquals(event['submission'], { + factory.answer_key(2): { + 'question': '', + 'answer': '3.14', + 'response_type': 'numericalresponse', + 'input_type': 'textline', + 'correct': True, + 'variant': module.seed, + } + }) + + def test_file_inputs(self): + fnames = ["prog1.py", "prog2.py", "prog3.py"] + fpaths = [os.path.join(DATA_DIR, "capa", fname) for fname in fnames] + fileobjs = [open(fpath) for fpath in fpaths] + for fileobj in fileobjs: + self.addCleanup(fileobj.close) + + factory = CapaFactoryWithFiles + module = factory.create() + + # Mock the XQueueInterface. + xqueue_interface = XQueueInterface("http://example.com/xqueue", Mock()) + xqueue_interface._http_post = Mock(return_value=(0, "ok")) # pylint: disable=protected-access + module.system.xqueue['interface'] = xqueue_interface + + answer_input_dict = { + CapaFactoryWithFiles.input_key(response_num=2): fileobjs, + CapaFactoryWithFiles.input_key(response_num=3): 'None', + } + + event = self.get_event_for_answers(module, answer_input_dict) + self.assertEquals(event['submission'], { + factory.answer_key(2): { + 'question': '', + 'answer': fpaths, + 'response_type': 'coderesponse', + 'input_type': 'filesubmission', + 'correct': False, + 'variant': '', + }, + factory.answer_key(3): { + 'answer': 'None', + 'correct': True, + 'question': '', + 'response_type': 'customresponse', + 'input_type': 'textline', + 'variant': '' + } + })