Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion common/lib/capa/capa/inputtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,14 @@ def get_html(self):
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


#-----------------------------------------------------------------------------

Expand Down Expand Up @@ -385,6 +393,7 @@ def setup(self):
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):
Expand Down Expand Up @@ -419,6 +428,12 @@ def extract_choices(element):
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]


#-----------------------------------------------------------------------------

Expand Down Expand Up @@ -1021,7 +1036,7 @@ def get_attributes(cls):
"""
Can set size of text field.
"""
return [Attribute('size', '20'),
return [Attribute('size', '20'),
Attribute('label', ''),]

def _extra_context(self):
Expand Down
88 changes: 87 additions & 1 deletion common/lib/xmodule/xmodule/capa_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,8 @@ def check_problem(self, data):
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

Expand Down Expand Up @@ -944,6 +945,7 @@ def check_problem(self, data):
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
Expand All @@ -957,6 +959,90 @@ def check_problem(self, data):
'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.')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mulby from the insights from hut 8 today. Do you think it is worth to actually emit an event with can error "comment" or values marked as error?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An interesting idea, given the tight release deadline, I think we should skip it for now, but we should definitely consider it in the future.


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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not None if it could not be determined?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the empty string is less ambiguous given that it is serialized to JSON.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_user_visible_answer is on the base class. Shouldn't it always exist?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, since the base class implementation is identity by default, couldn't you skip the user_visible_answer = internal_answer line?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory get_user_visible_answer should always exists. However, we ran into some test cases that were not inheriting correctly. We didn't explore deep enough to understand why. Do you think it is worth it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems pretty strange to me, and I'd love to know how we got an input type that isn't derived from InputTypeBase.


# 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.
Expand Down
Loading