From 009d6c2e019f9f8344bcb0673d5f3bbd15d98683 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 24 Oct 2012 14:23:02 -0400 Subject: [PATCH] ChoiceGroup refactor - Make it into a class. - Combine ChoiceGroup, RadioGroup, CheckboxGroup implementation. (All three tags still work--this just unifies the code) - add tests --- common/lib/capa/capa/inputtypes.py | 151 ++++++++---------- common/lib/capa/capa/tests/test_inputtypes.py | 93 ++++++++--- 2 files changed, 134 insertions(+), 110 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 55115e66d8..acff3abf6a 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -97,8 +97,8 @@ class InputTypeBase(object): have a render_template function. - xml : Element tree of this Input element - state : a dictionary with optional keys: - * 'value' - * 'id' + * 'value' -- the current value of this input (what the student entered last time) + * 'id' -- the id of this input, typically "{problem-location}_{response-num}_{input-num}" * 'status' (answered, unanswered, unsubmitted) * 'feedback' (dictionary containing keys for hints, errors, or other feedback from previous attempt. Specifically 'message', 'hint', 'hintmode'. If 'hintmode' @@ -234,49 +234,70 @@ register_input_class(OptionInput) # TODO: consolidate choicegroup, radiogroup, checkboxgroup after discussion of # desired semantics. -def choicegroup(element, value, status, render_template, msg=''): - ''' - Radio button inputs: multiple choice or true/false + +class ChoiceGroup(InputTypeBase): + """ + Radio button or checkbox inputs: multiple choice or true/false TODO: allow order of choices to be randomized, following lon-capa spec. Use "location" attribute, ie random, top, bottom. - ''' - eid = element.get('id') - if element.get('type') == "MultipleChoice": - element_type = "radio" - elif element.get('type') == "TrueFalse": - element_type = "checkbox" - else: - element_type = "radio" - choices = [] - for choice in element: - if not choice.tag == 'choice': - raise Exception("[courseware.capa.inputtypes.choicegroup] " - "Error: only tags should be immediate children " - "of a , found %s instead" % choice.tag) - ctext = "" - # TODO: what if choice[0] has math tags in it? - ctext += ''.join([etree.tostring(x) for x in choice]) - if choice.text is not None: - # TODO: fix order? - ctext += choice.text - choices.append((choice.get("name"), ctext)) - context = {'id': eid, - 'value': value, - 'state': status, - 'input_type': element_type, - 'choices': choices, - 'name_array_suffix': ''} - html = render_template("choicegroup.html", context) - return etree.XML(html) -_reg(choicegroup) + Example: + + + + This is foil One. + + + This is foil Two. + + + This is foil Three. + + + """ + template = "choicegroup.html" + tags = ['choicegroup', 'radiogroup', 'checkboxgroup'] + + def __init__(self, system, xml, state): + super(ChoiceGroup, self).__init__(system, xml, state) + + if self.tag == 'choicegroup': + self.suffix = '' + if self.xml.get('type') == "MultipleChoice": + self.element_type = "radio" + elif self.xml.get('type') == "TrueFalse": + # Huh? Why TrueFalse->checkbox? Each input can be true / false separately? + self.element_type = "checkbox" + else: + self.element_type = "radio" + + elif self.tag == 'radiogroup': + self.element_type = "radio" + self.suffix = '[]' + elif self.tag == 'checkboxgroup': + self.element_type = "checkbox" + self.suffix = '[]' + else: + raise Exception("ChoiceGroup: unexpected tag {0}".format(self.tag)) + + self.choices = extract_choices(self.xml) + + def _get_render_context(self): + context = {'id': self.id, + 'value': self.value, + 'state': self.status, + 'input_type': self.element_type, + 'choices': self.choices, + 'name_array_suffix': self.suffix} + return context -#----------------------------------------------------------------------------- def extract_choices(element): ''' - Extracts choices for a few input types, such as radiogroup and - checkboxgroup. + Extracts choices for a few input types, such as ChoiceGroup, RadioGroup and + CheckboxGroup. + + returns list of (choice_name, choice_text) tuples TODO: allow order of choices to be randomized, following lon-capa spec. Use "location" attribute, ie random, top, bottom. @@ -285,63 +306,25 @@ def extract_choices(element): choices = [] for choice in element: - if not choice.tag == 'choice': + if choice.tag != 'choice': raise Exception("[courseware.capa.inputtypes.extract_choices] \ Expected a tag; got %s instead" % choice.tag) choice_text = ''.join([etree.tostring(x) for x in choice]) + if choice.text is not None: + # TODO: fix order? + choice_text += choice.text choices.append((choice.get("name"), choice_text)) return choices + + +register_input_class(ChoiceGroup) -# TODO: consolidate choicegroup, radiogroup, checkboxgroup after discussion of -# desired semantics. -def radiogroup(element, value, status, render_template, msg=''): - ''' - Radio button inputs: (multiple choice) - ''' +#----------------------------------------------------------------------------- - eid = element.get('id') - - choices = extract_choices(element) - - context = {'id': eid, - 'value': value, - 'state': status, - 'input_type': 'radio', - 'choices': choices, - 'name_array_suffix': '[]'} - - html = render_template("choicegroup.html", context) - return etree.XML(html) - - -_reg(radiogroup) - -# TODO: consolidate choicegroup, radiogroup, checkboxgroup after discussion of -# desired semantics. -def checkboxgroup(element, value, status, render_template, msg=''): - ''' - Checkbox inputs: (select one or more choices) - ''' - - eid = element.get('id') - - choices = extract_choices(element) - - context = {'id': eid, - 'value': value, - 'state': status, - 'input_type': 'checkbox', - 'choices': choices, - 'name_array_suffix': '[]'} - - html = render_template("choicegroup.html", context) - return etree.XML(html) - -_reg(checkboxgroup) def javascriptinput(element, value, status, render_template, msg='null'): ''' diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index 79cd9b6c98..833cc396c2 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -35,7 +35,7 @@ class OptionInputTest(unittest.TestCase): state = {'value': 'Down', 'id': 'sky_input', 'status': 'answered'} - option_input = inputtypes.OptionInput(system, element, state) + option_input = inputtypes.get_class_for_tag('optioninput')(system, element, state) context = option_input._get_render_context() @@ -53,40 +53,81 @@ class ChoiceGroupTest(unittest.TestCase): Test choice groups. ''' def test_mult_choice(self): - xml_str = """ - - - This is foil One. - - - This is foil Two. - - - This is foil Three. - - - This is foil Four. - - - This is foil Five. - + xml_template = """ + + This is foil One. + This is foil Two. + This is foil Three. """ + + def check_type(type_str, expected_input_type): + print "checking for type_str='{0}'".format(type_str) + xml_str = xml_template.format(type_str) + + element = etree.fromstring(xml_str) + + state = {'value': 'foil3', + 'id': 'sky_input', + 'status': 'answered'} + + option_input = inputtypes.get_class_for_tag('choicegroup')(system, element, state) + + context = option_input._get_render_context() + + expected = {'id': 'sky_input', + 'value': 'foil3', + 'state': 'answered', + 'input_type': expected_input_type, + 'choices': [('foil1', 'This is foil One.'), + ('foil2', 'This is foil Two.'), + ('foil3', 'This is foil Three.'),], + 'name_array_suffix': '', # what is this for?? + } + + self.assertEqual(context, expected) + + check_type('', 'radio') + check_type('type=""', 'radio') + check_type('type="MultipleChoice"', 'radio') + check_type('type="TrueFalse"', 'checkbox') + # fallback. + check_type('type="StrangeUnknown"', 'radio') + + + def check_group(self, tag, expected_input_type, expected_suffix): + xml_str = """ + <{tag}> + This is foil One. + This is foil Two. + This is foil Three. + + """.format(tag=tag) + element = etree.fromstring(xml_str) - state = {'value': 'Down', + state = {'value': 'foil3', 'id': 'sky_input', 'status': 'answered'} - option_input = inputtypes.OptionInput(system, element, state) - context = option_input._get_render_context() + the_input = inputtypes.get_class_for_tag(tag)(system, element, state) - expected = {'value': 'Down', - 'options': [('Up', 'Up'), ('Down', 'Down')], + context = the_input._get_render_context() + + expected = {'id': 'sky_input', + 'value': 'foil3', 'state': 'answered', - 'msg': '', - 'inline': '', - 'id': 'sky_input'} + 'input_type': expected_input_type, + 'choices': [('foil1', 'This is foil One.'), + ('foil2', 'This is foil Two.'), + ('foil3', 'This is foil Three.'),], + 'name_array_suffix': expected_suffix, # what is this for?? + } self.assertEqual(context, expected) + def test_radiogroup(self): + self.check_group('radiogroup', 'radio', '[]') + + def test_checkboxgroup(self): + self.check_group('checkboxgroup', 'checkbox', '[]')