From 74e23546af9dcad10d162e9491862b78f5310efe Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 31 Oct 2012 18:40:10 -0400 Subject: [PATCH 1/4] More inputtype refactor - add an Attribute class - input types just need to declare which attributes they want, and how to transform and validate them, and the base class will do all the rest. - change OptionInput to new format. --- common/lib/capa/capa/inputtypes.py | 156 +++++++++++++++++++++++------ 1 file changed, 124 insertions(+), 32 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index de29b5e664..f154569fe4 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -32,8 +32,7 @@ graded status as'status' # makes sense, but a bunch of problems have markup that assumes block. Bigger TODO: figure out a # general css and layout strategy for capa, document it, then implement it. - - +from collections import namedtuple import json import logging from lxml import etree @@ -50,6 +49,58 @@ log = logging.getLogger('mitx.' + __name__) registry = TagRegistry() +class Attribute(object): + """ + Allows specifying required and optional attributes for input types. + """ + + # want to allow default to be None, but also allow required objects + _sentinel = object() + + def __init__(self, name, default=_sentinel, transform=None, validate=None): + """ + Define an attribute + + name (str): then name of the attribute--should be alphanumeric (valid for an XML attribute) + + default (any type): If not specified, this attribute is required. If specified, use this as the default value + if the attribute is not specified. Note that this value will not be transformed or validated. + + transform (function str -> any type): If not None, will be called to transform the parsed value into an internal + representation. + + validate (function str-or-return-type-of-tranform -> unit or exception): If not None, called to validate the + (possibly transformed) value of the attribute. Should raise ValueError with a helpful message if + the value is invalid. + """ + self.name = name + self.default = default + self.validate = validate + self.transform = transform + + def parse_from_xml(self, element): + """ + Given an etree xml element that should have this attribute, do the obvious thing: + - look for it. raise ValueError if not found and required. + - transform and validate. pass through any exceptions from transform or validate. + """ + val = element.get(self.name) + if self.default == self._sentinel and val is None: + raise ValueError('Missing required attribute {0}.'.format(self.name)) + + if val is None: + # not required, so return default + return self.default + + if self.transform is not None: + val = self.transform(val) + + if self.validate is not None: + self.validate(val) + + return val + + class InputTypeBase(object): """ Abstract base class for input types. @@ -102,9 +153,12 @@ class InputTypeBase(object): self.status = state.get('status', 'unanswered') - # Call subclass "constructor" -- means they don't have to worry about calling - # super().__init__, and are isolated from changes to the input constructor interface. try: + # Pre-parse and propcess all the declared requirements. + self.process_requirements() + + # Call subclass "constructor" -- means they don't have to worry about calling + # super().__init__, and are isolated from changes to the input constructor interface. self.setup() except Exception as err: # Something went wrong: add xml to message, but keep the traceback @@ -112,6 +166,32 @@ class InputTypeBase(object): raise Exception, msg, sys.exc_info()[2] + @classmethod + def get_attributes(cls): + """ + Should return a list of Attribute objects (see docstring there for details). Subclasses should override. e.g. + + return super(MyClass, cls).attributes + [Attribute('unicorn', True), + Attribute('num_dragons', 12, transform=int), ...] + """ + return [] + + + def process_requirements(self): + """ + Subclasses can declare lists of required and optional attributes. This + function parses the input xml and pulls out those attributes. This + isolates most simple input types from needing to deal with xml parsing at all. + + Processes attributes, putting the results in the self.loaded_attributes dictionary. + """ + # Use a local dict so that if there are exceptions, we don't end up in a partially-initialized state. + d = {} + for a in self.get_attributes(): + d[a.name] = a.parse_from_xml(self.xml) + + self.loaded_attributes = d + def setup(self): """ InputTypes should override this to do any needed initialization. It is called after the @@ -122,14 +202,28 @@ class InputTypeBase(object): """ pass + def _get_render_context(self): """ - Abstract method. Subclasses should implement to return the dictionary - of keys needed to render their template. + Should return a dictionary of keys needed to render the template for the input type. (Separate from get_html to faciliate testing of logic separately from the rendering) + + The default implementation gets the following rendering context: basic things like value, id, + status, and msg, as well as everything in self.loaded_attributes. + + This means that input types that only parse attributes get everything they need, and don't need + to override this method. """ - raise NotImplementedError + context = { + 'id': self.id, + 'value': self.value, + 'status': self.status, + 'msg': self.msg, + } + context.update(self.loaded_attributes) + return context + def get_html(self): """ @@ -139,7 +233,10 @@ class InputTypeBase(object): raise NotImplementedError("no rendering template specified for class {0}" .format(self.__class__)) - html = self.system.render_template(self.template, self._get_render_context()) + context = self._default_render_context() + context.update(self._get_render_context()) + + html = self.system.render_template(self.template, context) return etree.XML(html) @@ -158,33 +255,28 @@ class OptionInput(InputTypeBase): template = "optioninput.html" tags = ['optioninput'] - def setup(self): - # Extract the options... - options = self.xml.get('options') - if not options: - raise ValueError("optioninput: Missing 'options' specification.") + @classmethod + def get_attributes(cls): + """ + Convert options to a convenient format. + """ - # parse the set of possible options - oset = shlex.shlex(options[1:-1]) - oset.quotes = "'" - oset.whitespace = "," - oset = [x[1:-1] for x in list(oset)] + def parse_options(options): + """Given options string, convert it into an ordered list of (option, option) tuples + (Why? I don't know--that's what the template uses at the moment) + """ + # parse the set of possible options + oset = shlex.shlex(options[1:-1]) + oset.quotes = "'" + oset.whitespace = "," + oset = [x[1:-1] for x in list(oset)] - # make ordered list with (key, value) same - self.osetdict = [(oset[x], oset[x]) for x in range(len(oset))] - # TODO: allow ordering to be randomized + # make ordered list with (key, value) same + return [(oset[x], oset[x]) for x in range(len(oset))] - def _get_render_context(self): - - context = { - 'id': self.id, - 'value': self.value, - 'status': self.status, - 'msg': self.msg, - 'options': self.osetdict, - 'inline': self.xml.get('inline',''), - } - return context + return super(OptionInput, cls).get_attributes() + [ + Attribute('options', transform=parse_options), + Attribute('inline', '')] registry.register(OptionInput) From 7fcf02a0cc91e12e3889844e233c9b4b0ff26b8b Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 31 Oct 2012 23:40:19 -0400 Subject: [PATCH 2/4] Further refactor - small Attribute and InputTypeBase interface changes to make things cleaner - move html quoting into templates (use ${blah | h} syntax) - converting input types to use new format. --- common/lib/capa/capa/inputtypes.py | 230 +++++++++--------- .../capa/capa/templates/filesubmission.html | 2 +- .../capa/capa/templates/javascriptinput.html | 2 +- common/lib/capa/capa/templates/textline.html | 2 +- common/lib/capa/capa/tests/test_inputtypes.py | 28 ++- 5 files changed, 144 insertions(+), 120 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index f154569fe4..bd3642220f 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -57,7 +57,7 @@ class Attribute(object): # want to allow default to be None, but also allow required objects _sentinel = object() - def __init__(self, name, default=_sentinel, transform=None, validate=None): + def __init__(self, name, default=_sentinel, transform=None, validate=None, render=True): """ Define an attribute @@ -72,11 +72,14 @@ class Attribute(object): validate (function str-or-return-type-of-tranform -> unit or exception): If not None, called to validate the (possibly transformed) value of the attribute. Should raise ValueError with a helpful message if the value is invalid. + + render (bool): if False, don't include this attribute in the template context. """ self.name = name self.default = default self.validate = validate self.transform = transform + self.render = render def parse_from_xml(self, element): """ @@ -171,8 +174,7 @@ class InputTypeBase(object): """ Should return a list of Attribute objects (see docstring there for details). Subclasses should override. e.g. - return super(MyClass, cls).attributes + [Attribute('unicorn', True), - Attribute('num_dragons', 12, transform=int), ...] + return [Attribute('unicorn', True), Attribute('num_dragons', 12, transform=int), ...] """ return [] @@ -183,14 +185,19 @@ class InputTypeBase(object): function parses the input xml and pulls out those attributes. This isolates most simple input types from needing to deal with xml parsing at all. - Processes attributes, putting the results in the self.loaded_attributes dictionary. + Processes attributes, putting the results in the self.loaded_attributes dictionary. Also creates a set + self.to_render, containing the names of attributes that should be included in the context by default. """ - # Use a local dict so that if there are exceptions, we don't end up in a partially-initialized state. - d = {} + # Use local dicts and sets so that if there are exceptions, we don't end up in a partially-initialized state. + loaded = {} + to_render = set() for a in self.get_attributes(): - d[a.name] = a.parse_from_xml(self.xml) + loaded[a.name] = a.parse_from_xml(self.xml) + if a.render: + to_render.add(a.name) - self.loaded_attributes = d + self.loaded_attributes = loaded + self.to_render = to_render def setup(self): """ @@ -209,11 +216,11 @@ class InputTypeBase(object): (Separate from get_html to faciliate testing of logic separately from the rendering) - The default implementation gets the following rendering context: basic things like value, id, - status, and msg, as well as everything in self.loaded_attributes. + The default implementation gets the following rendering context: basic things like value, id, status, and msg, + as well as everything in self.loaded_attributes, and everything returned by self._extra_context(). - This means that input types that only parse attributes get everything they need, and don't need - to override this method. + This means that input types that only parse attributes and pass them to the template get everything they need, + and don't need to override this method. """ context = { 'id': self.id, @@ -221,9 +228,17 @@ class InputTypeBase(object): 'status': self.status, 'msg': self.msg, } - context.update(self.loaded_attributes) + context.update((a, v) for (a, v) in self.loaded_attributes.iteritems() if a in self.to_render) + context.update(self._extra_context()) return context + def _extra_context(self): + """ + Subclasses can override this to return extra context that should be passed to their templates for rendering. + + This is useful when the input type requires computing new template variables from the parsed attributes. + """ + return {} def get_html(self): """ @@ -233,8 +248,7 @@ class InputTypeBase(object): raise NotImplementedError("no rendering template specified for class {0}" .format(self.__class__)) - context = self._default_render_context() - context.update(self._get_render_context()) + context = self._get_render_context() html = self.system.render_template(self.template, context) return etree.XML(html) @@ -255,28 +269,32 @@ class OptionInput(InputTypeBase): template = "optioninput.html" tags = ['optioninput'] + @staticmethod + def parse_options(options): + """ + Given options string, convert it into an ordered list of (option_id, option_description) tuples, where + id==description for now. TODO: make it possible to specify different id and descriptions. + """ + # parse the set of possible options + lexer = shlex.shlex(options[1:-1]) + lexer.quotes = "'" + # Allow options to be separated by whitespace as well as commas + lexer.whitespace = ", " + + # remove quotes + tokens = [x[1:-1] for x in list(lexer)] + + # make list of (option_id, option_description), with description=id + return [(t, t) for t in tokens] + + @classmethod def get_attributes(cls): """ Convert options to a convenient format. """ - - def parse_options(options): - """Given options string, convert it into an ordered list of (option, option) tuples - (Why? I don't know--that's what the template uses at the moment) - """ - # parse the set of possible options - oset = shlex.shlex(options[1:-1]) - oset.quotes = "'" - oset.whitespace = "," - oset = [x[1:-1] for x in list(oset)] - - # make ordered list with (key, value) same - return [(oset[x], oset[x]) for x in range(len(oset))] - - return super(OptionInput, cls).get_attributes() + [ - Attribute('options', transform=parse_options), - Attribute('inline', '')] + return [Attribute('options', transform=cls.parse_options), + Attribute('inline', '')] registry.register(OptionInput) @@ -315,26 +333,22 @@ class ChoiceGroup(InputTypeBase): # value. (VS: would be nice to make this less hackish). if self.tag == 'choicegroup': self.suffix = '' - self.element_type = "radio" + self.html_input_type = "radio" elif self.tag == 'radiogroup': - self.element_type = "radio" + self.html_input_type = "radio" self.suffix = '[]' elif self.tag == 'checkboxgroup': - self.element_type = "checkbox" + self.html_input_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, - 'status': self.status, - 'input_type': self.element_type, - 'choices': self.choices, - 'name_array_suffix': self.suffix} - return context + def _extra_context(self): + return {'input_type': self.html_input_type, + 'choices': self.choices, + 'name_array_suffix': self.suffix} def extract_choices(element): ''' @@ -384,33 +398,23 @@ class JavascriptInput(InputTypeBase): template = "javascriptinput.html" tags = ['javascriptinput'] + @classmethod + def get_attributes(cls): + """ + Register the attributes. + """ + return [Attribute('params', None), + Attribute('problem_state', None), + Attribute('display_class', None), + Attribute('display_file', None),] + + def setup(self): # Need to provide a value that JSON can parse if there is no # student-supplied value yet. if self.value == "": self.value = 'null' - self.params = self.xml.get('params') - self.problem_state = self.xml.get('problem_state') - self.display_class = self.xml.get('display_class') - self.display_file = self.xml.get('display_file') - - - def _get_render_context(self): - escapedict = {'"': '"'} - value = saxutils.escape(self.value, escapedict) - msg = saxutils.escape(self.msg, escapedict) - - context = {'id': self.id, - 'params': self.params, - 'display_file': self.display_file, - 'display_class': self.display_class, - 'problem_state': self.problem_state, - 'value': value, - 'evaluation': msg, - } - return context - registry.register(JavascriptInput) @@ -418,51 +422,53 @@ registry.register(JavascriptInput) class TextLine(InputTypeBase): """ - + A text line input. Can do math preview if "math"="1" is specified. """ template = "textline.html" tags = ['textline'] + + @classmethod + def get_attributes(cls): + """ + Register the attributes. + """ + return [ + Attribute('size', None), + + # if specified, then textline is hidden and input id is stored + # in div with name=self.hidden. (TODO: is this functionality used by anyone?) + Attribute('hidden', False), + Attribute('inline', False), + + # Attributes below used in setup(), not rendered directly. + Attribute('math', None, render=False), + # TODO: 'dojs' flag is temporary, for backwards compatibility with 8.02x + Attribute('dojs', None, render=False), + Attribute('preprocessorClassName', None, render=False), + Attribute('preprocessorSrc', None, render=False), + ] + + def setup(self): - self.size = self.xml.get('size') + self.do_math = bool(self.loaded_attributes['math'] or + self.loaded_attributes['dojs']) - # if specified, then textline is hidden and input id is stored - # in div with name=self.hidden. - self.hidden = self.xml.get('hidden', False) - - self.inline = self.xml.get('inline', False) - - # TODO: 'dojs' flag is temporary, for backwards compatibility with 8.02x - self.do_math = bool(self.xml.get('math') or self.xml.get('dojs')) # TODO: do math checking using ajax instead of using js, so # that we only have one math parser. self.preprocessor = None if self.do_math: # Preprocessor to insert between raw input and Mathjax - self.preprocessor = {'class_name': self.xml.get('preprocessorClassName',''), - 'script_src': self.xml.get('preprocessorSrc','')} - if '' in self.preprocessor.values(): + self.preprocessor = {'class_name': self.loaded_attributes['preprocessorClassName'], + 'script_src': self.loaded_attributes['preprocessorSrc']} + if None in self.preprocessor.values(): self.preprocessor = None - - def _get_render_context(self): - # Escape answers with quotes, so they don't crash the system! - escapedict = {'"': '"'} - value = saxutils.escape(self.value, escapedict) - - context = {'id': self.id, - 'value': value, - 'status': self.status, - 'size': self.size, - 'msg': self.msg, - 'hidden': self.hidden, - 'inline': self.inline, - 'do_math': self.do_math, - 'preprocessor': self.preprocessor, - } - return context + def _extra_context(self): + return {'do_math': self.do_math, + 'preprocessor': self.preprocessor,} registry.register(TextLine) @@ -480,13 +486,26 @@ class FileSubmission(InputTypeBase): submitted_msg = ("Your file(s) have been submitted; as soon as your submission is" " graded, this message will be replaced with the grader's feedback.") - def setup(self): - escapedict = {'"': '"'} - self.allowed_files = json.dumps(self.xml.get('allowed_files', '').split()) - self.allowed_files = saxutils.escape(self.allowed_files, escapedict) - self.required_files = json.dumps(self.xml.get('required_files', '').split()) - self.required_files = saxutils.escape(self.required_files, escapedict) + @staticmethod + def parse_files(files): + """ + Given a string like 'a.py b.py c.out', split on whitespace and return as a json list. + """ + return json.dumps(files.split()) + @classmethod + def get_attributes(cls): + """ + Convert the list of allowed files to a convenient format. + """ + return [Attribute('allowed_files', '[]', transform=cls.parse_files), + Attribute('required_files', '[]', transform=cls.parse_files),] + + def setup(self): + """ + Do some magic to handle queueing status (render as "queued" instead of "incomplete"), + pull queue_len from the msg field. (TODO: get rid of the queue_len hack). + """ # Check if problem has been queued self.queue_len = 0 # Flag indicating that the problem has been queued, 'msg' is length of queue @@ -495,15 +514,8 @@ class FileSubmission(InputTypeBase): self.queue_len = self.msg self.msg = FileSubmission.submitted_msg - def _get_render_context(self): - - context = {'id': self.id, - 'status': self.status, - 'msg': self.msg, - 'value': self.value, - 'queue_len': self.queue_len, - 'allowed_files': self.allowed_files, - 'required_files': self.required_files,} + def _extra_context(self): + return {'queue_len': self.queue_len,} return context registry.register(FileSubmission) diff --git a/common/lib/capa/capa/templates/filesubmission.html b/common/lib/capa/capa/templates/filesubmission.html index 2572b25f8a..930469dc0d 100644 --- a/common/lib/capa/capa/templates/filesubmission.html +++ b/common/lib/capa/capa/templates/filesubmission.html @@ -12,7 +12,7 @@ % endif

${status}

- +
${msg|n}
diff --git a/common/lib/capa/capa/templates/javascriptinput.html b/common/lib/capa/capa/templates/javascriptinput.html index 8b4c8f7115..b4d007e4d8 100644 --- a/common/lib/capa/capa/templates/javascriptinput.html +++ b/common/lib/capa/capa/templates/javascriptinput.html @@ -2,7 +2,7 @@
+ data-submission="${value|h}" data-evaluation="${msg|h}">
diff --git a/common/lib/capa/capa/templates/textline.html b/common/lib/capa/capa/templates/textline.html index 97c512fc00..fbb5467b67 100644 --- a/common/lib/capa/capa/templates/textline.html +++ b/common/lib/capa/capa/templates/textline.html @@ -20,7 +20,7 @@
% endif - This is foil One.'), ('foil2', 'This is foil Two.'), @@ -119,12 +133,13 @@ class JavascriptInputTest(unittest.TestCase): context = the_input._get_render_context() expected = {'id': 'prob_1_2', + 'status': 'unanswered', + 'msg': '', + 'value': '3', 'params': params, 'display_file': display_file, 'display_class': display_class, - 'problem_state': problem_state, - 'value': '3', - 'evaluation': '',} + 'problem_state': problem_state,} self.assertEqual(context, expected) @@ -204,9 +219,6 @@ class FileSubmissionTest(unittest.TestCase): element = etree.fromstring(xml_str) - escapedict = {'"': '"'} - esc = lambda s: saxutils.escape(s, escapedict) - state = {'value': 'BumbleBee.py', 'status': 'incomplete', 'feedback' : {'message': '3'}, } @@ -220,8 +232,8 @@ class FileSubmissionTest(unittest.TestCase): 'msg': input_class.submitted_msg, 'value': 'BumbleBee.py', 'queue_len': '3', - 'allowed_files': esc('["runme.py", "nooooo.rb", "ohai.java"]'), - 'required_files': esc('["cookies.py"]')} + 'allowed_files': '["runme.py", "nooooo.rb", "ohai.java"]', + 'required_files': '["cookies.py"]'} self.assertEqual(context, expected) From b78fb8dff665cb15de2c378440d305d217c79456 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Thu, 1 Nov 2012 00:05:48 -0400 Subject: [PATCH 3/4] Refactored the rest of the input types --- common/lib/capa/capa/inputtypes.py | 218 ++++++++---------- .../capa/capa/templates/crystallography.html | 2 +- .../lib/capa/capa/templates/vsepr_input.html | 2 +- common/lib/capa/capa/tests/test_inputtypes.py | 18 +- 4 files changed, 110 insertions(+), 130 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index bd3642220f..2ff926479a 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -21,12 +21,14 @@ Each input type takes the xml tree as 'element', the previous answer as 'value', graded status as'status' """ -# TODO: there is a lot of repetitive "grab these elements from xml attributes, with these defaults, -# put them in the context" code. Refactor so class just specifies required and optional attrs (with -# defaults for latter), and InputTypeBase does the right thing. +# TODO: make hints do something + +# TODO: make all inputtypes actually render msg + +# TODO: remove unused fields (e.g. 'hidden' in a few places) + +# TODO: add validators so that content folks get better error messages. -# TODO: Quoting and unquoting is handled in a pretty ad-hoc way. Also something that could be done -# properly once in InputTypeBase. # Possible todo: make inline the default for textlines and other "one-line" inputs. It probably # makes sense, but a bunch of problems have markup that assumes block. Bigger TODO: figure out a @@ -39,7 +41,6 @@ from lxml import etree import re import shlex # for splitting quoted strings import sys -import xml.sax.saxutils as saxutils from registry import TagRegistry @@ -535,13 +536,30 @@ class CodeInput(InputTypeBase): # non-codemirror editor. ] + # pulled out for testing + submitted_msg = ("Your file(s) have been submitted; as soon as your submission is" + " graded, this message will be replaced with the grader's feedback.") + + @classmethod + def get_attributes(cls): + """ + Convert options to a convenient format. + """ + return [Attribute('rows', '30'), + Attribute('cols', '80'), + Attribute('hidden', ''), + + # For CodeMirror + Attribute('mode', 'python'), + Attribute('linenumbers', 'true'), + # Template expects tabsize to be an int it can do math with + Attribute('tabsize', 4, transform=int), + ] def setup(self): - self.rows = self.xml.get('rows') or '30' - self.cols = self.xml.get('cols') or '80' - # if specified, then textline is hidden and id is stored in div of name given by hidden - self.hidden = self.xml.get('hidden', '') - + """ + Implement special logic: handle queueing state, and default input. + """ # if no student input yet, then use the default input given by the problem if not self.value: self.value = self.xml.text @@ -552,28 +570,11 @@ class CodeInput(InputTypeBase): if self.status == 'incomplete': self.status = 'queued' self.queue_len = self.msg - self.msg = 'Submitted to grader.' + self.msg = self.submitted_msg - # For CodeMirror - self.mode = self.xml.get('mode', 'python') - self.linenumbers = self.xml.get('linenumbers', 'true') - self.tabsize = int(self.xml.get('tabsize', '4')) - - def _get_render_context(self): - - context = {'id': self.id, - 'value': self.value, - 'status': self.status, - 'msg': self.msg, - 'mode': self.mode, - 'linenumbers': self.linenumbers, - 'rows': self.rows, - 'cols': self.cols, - 'hidden': self.hidden, - 'tabsize': self.tabsize, - 'queue_len': self.queue_len, - } - return context + def _extra_context(self): + """Defined queue_len, add it """ + return {'queue_len': self.queue_len,} registry.register(CodeInput) @@ -586,26 +587,19 @@ class Schematic(InputTypeBase): template = "schematicinput.html" tags = ['schematic'] - def setup(self): - self.height = self.xml.get('height') - self.width = self.xml.get('width') - self.parts = self.xml.get('parts') - self.analyses = self.xml.get('analyses') - self.initial_value = self.xml.get('initial_value') - self.submit_analyses = self.xml.get('submit_analyses') + @classmethod + def get_attributes(cls): + """ + Convert options to a convenient format. + """ + return [ + Attribute('height', None), + Attribute('width', None), + Attribute('parts', None), + Attribute('analyses', None), + Attribute('initial_value', None), + Attribute('submit_analyses', None),] - - def _get_render_context(self): - - context = {'id': self.id, - 'value': self.value, - 'initial_value': self.initial_value, - 'status': self.status, - 'width': self.width, - 'height': self.height, - 'parts': self.parts, - 'analyses': self.analyses, - 'submit_analyses': self.submit_analyses,} return context registry.register(Schematic) @@ -626,12 +620,20 @@ class ImageInput(InputTypeBase): template = "imageinput.html" tags = ['imageinput'] - def setup(self): - self.src = self.xml.get('src') - self.height = self.xml.get('height') - self.width = self.xml.get('width') + @classmethod + def get_attributes(cls): + """ + Note: src, height, and width are all required. + """ + return [Attribute('src'), + Attribute('height'), + Attribute('width'),] - # if value is of the form [x,y] then parse it and send along coordinates of previous answer + + def setup(self): + """ + if value is of the form [x,y] then parse it and send along coordinates of previous answer + """ m = re.match('\[([0-9]+),([0-9]+)]', self.value.strip().replace(' ', '')) if m: # Note: we subtract 15 to compensate for the size of the dot on the screen. @@ -641,19 +643,10 @@ class ImageInput(InputTypeBase): (self.gx, self.gy) = (0, 0) - def _get_render_context(self): + def _extra_context(self): - context = {'id': self.id, - 'value': self.value, - 'height': self.height, - 'width': self.width, - 'src': self.src, - 'gx': self.gx, - 'gy': self.gy, - 'status': self.status, - 'msg': self.msg, - } - return context + return {'gx': self.gx, + 'gy': self.gy} registry.register(ImageInput) @@ -669,30 +662,18 @@ class Crystallography(InputTypeBase): template = "crystallography.html" tags = ['crystallography'] + @classmethod + def get_attributes(cls): + """ + Note: height, width are required. + """ + return [Attribute('size', None), + Attribute('height'), + Attribute('width'), - def setup(self): - self.height = self.xml.get('height') - self.width = self.xml.get('width') - self.size = self.xml.get('size') - - # if specified, then textline is hidden and id is stored in div of name given by hidden - self.hidden = self.xml.get('hidden', '') - - # Escape answers with quotes, so they don't crash the system! - escapedict = {'"': '"'} - self.value = saxutils.escape(self.value, escapedict) - - def _get_render_context(self): - context = {'id': self.id, - 'value': self.value, - 'status': self.status, - 'size': self.size, - 'msg': self.msg, - 'hidden': self.hidden, - 'width': self.width, - 'height': self.height, - } - return context + # can probably be removed (textline should prob be always-hidden) + Attribute('hidden', ''), + ] registry.register(Crystallography) @@ -707,29 +688,16 @@ class VseprInput(InputTypeBase): template = 'vsepr_input.html' tags = ['vsepr_input'] - def setup(self): - self.height = self.xml.get('height') - self.width = self.xml.get('width') - - # Escape answers with quotes, so they don't crash the system! - escapedict = {'"': '"'} - self.value = saxutils.escape(self.value, escapedict) - - self.molecules = self.xml.get('molecules') - self.geometries = self.xml.get('geometries') - - def _get_render_context(self): - - context = {'id': self.id, - 'value': self.value, - 'status': self.status, - 'msg': self.msg, - 'width': self.width, - 'height': self.height, - 'molecules': self.molecules, - 'geometries': self.geometries, - } - return context + @classmethod + def get_attributes(cls): + """ + Note: height, width are required. + """ + return [Attribute('height'), + Attribute('width'), + Attribute('molecules'), + Attribute('geometries'), + ] registry.register(VseprInput) @@ -750,17 +718,17 @@ class ChemicalEquationInput(InputTypeBase): template = "chemicalequationinput.html" tags = ['chemicalequationinput'] - def setup(self): - self.size = self.xml.get('size', '20') + @classmethod + def get_attributes(cls): + """ + Can set size of text field. + """ + return [Attribute('size', '20'),] - def _get_render_context(self): - context = { - 'id': self.id, - 'value': self.value, - 'status': self.status, - 'size': self.size, - 'previewer': '/static/js/capa/chemical_equation_preview.js', - } - return context + def _extra_context(self): + """ + TODO (vshnayder): Get rid of this once we have a standard way of requiring js to be loaded. + """ + return {'previewer': '/static/js/capa/chemical_equation_preview.js',} registry.register(ChemicalEquationInput) diff --git a/common/lib/capa/capa/templates/crystallography.html b/common/lib/capa/capa/templates/crystallography.html index f46e2f753a..2370f59dd2 100644 --- a/common/lib/capa/capa/templates/crystallography.html +++ b/common/lib/capa/capa/templates/crystallography.html @@ -19,7 +19,7 @@
% endif - % endif - diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index 5fbe73503f..826d304717 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -2,9 +2,18 @@ Tests of input types. TODO: +- refactor: so much repetive code (have factory methods that build xml elements directly, etc) + +- test error cases + +- check rendering -- e.g. msg should appear in the rendered output. If possible, test that + templates are escaping things properly. + + - test unicode in values, parameters, etc. - test various html escapes - test funny xml chars -- should never get xml parse error if things are escaped properly. + """ from lxml import etree @@ -267,14 +276,15 @@ class CodeInputTest(unittest.TestCase): 'status': 'incomplete', 'feedback' : {'message': '3'}, } - the_input = lookup_tag('codeinput')(test_system, element, state) + input_class = lookup_tag('codeinput') + the_input = input_class(test_system, element, state) context = the_input._get_render_context() expected = {'id': 'prob_1_2', 'value': 'print "good evening"', 'status': 'queued', - 'msg': 'Submitted to grader.', + 'msg': input_class.submitted_msg, 'mode': mode, 'linenumbers': linenumbers, 'rows': rows, @@ -323,8 +333,9 @@ class SchematicTest(unittest.TestCase): expected = {'id': 'prob_1_2', 'value': value, - 'initial_value': initial_value, 'status': 'unsubmitted', + 'msg': '', + 'initial_value': initial_value, 'width': width, 'height': height, 'parts': parts, @@ -488,6 +499,7 @@ class ChemicalEquationTest(unittest.TestCase): expected = {'id': 'prob_1_2', 'value': 'H2OYeah', 'status': 'unanswered', + 'msg': '', 'size': size, 'previewer': '/static/js/capa/chemical_equation_preview.js', } From 79174259cbbbc1f1756daf27d9048f5b408cfec9 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Fri, 2 Nov 2012 14:00:51 -0400 Subject: [PATCH 4/4] Address Ike's comments. - change a few comments / strings - move extract_choices into the ChoiceGroup class --- common/lib/capa/capa/inputtypes.py | 54 ++++++++++++++++-------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 2ff926479a..0b2250f98d 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -265,6 +265,8 @@ class OptionInput(InputTypeBase): Example: The location of the sky + + # TODO: allow ordering to be randomized """ template = "optioninput.html" @@ -288,7 +290,6 @@ class OptionInput(InputTypeBase): # make list of (option_id, option_description), with description=id return [(t, t) for t in tokens] - @classmethod def get_attributes(cls): """ @@ -344,39 +345,40 @@ class ChoiceGroup(InputTypeBase): else: raise Exception("ChoiceGroup: unexpected tag {0}".format(self.tag)) - self.choices = extract_choices(self.xml) + self.choices = self.extract_choices(self.xml) def _extra_context(self): return {'input_type': self.html_input_type, 'choices': self.choices, 'name_array_suffix': self.suffix} -def extract_choices(element): - ''' - Extracts choices for a few input types, such as ChoiceGroup, RadioGroup and - CheckboxGroup. + @staticmethod + def extract_choices(element): + ''' + Extracts choices for a few input types, such as ChoiceGroup, RadioGroup and + CheckboxGroup. - returns list of (choice_name, choice_text) tuples + 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. - ''' + TODO: allow order of choices to be randomized, following lon-capa spec. Use + "location" attribute, ie random, top, bottom. + ''' - choices = [] + choices = [] - for choice in element: - if choice.tag != 'choice': - raise Exception( - "[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 + for choice in element: + if choice.tag != 'choice': + raise Exception( + "[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)) + choices.append((choice.get("name"), choice_text)) - return choices + return choices registry.register(ChoiceGroup) @@ -424,6 +426,9 @@ registry.register(JavascriptInput) class TextLine(InputTypeBase): """ A text line input. Can do math preview if "math"="1" is specified. + + If the hidden attribute is specified, the textline is hidden and the input id is stored in a div with name equal + to the value of the hidden attribute. This is used e.g. for embedding simulations turned into questions. """ template = "textline.html" @@ -438,8 +443,7 @@ class TextLine(InputTypeBase): return [ Attribute('size', None), - # if specified, then textline is hidden and input id is stored - # in div with name=self.hidden. (TODO: is this functionality used by anyone?) + Attribute('hidden', False), Attribute('inline', False), @@ -537,7 +541,7 @@ class CodeInput(InputTypeBase): ] # pulled out for testing - submitted_msg = ("Your file(s) have been submitted; as soon as your submission is" + submitted_msg = ("Submitted. As soon as your submission is" " graded, this message will be replaced with the grader's feedback.") @classmethod