Merge pull request #499 from edx/will/authoring-student-training-schema-validation

Schema validation for student training examples

openassessment/xblock/schema.py

@@ -5,7 +5,6 @@ Schema for validating and sanitizing data received from the JavaScript client.
 import dateutil
 from pytz import utc
 from voluptuous import Schema, Required, All, Any, Range, In, Invalid
-from openassessment.xblock.xml import parse_examples_xml_str, UpdateFromXmlError
 
 
 def utf8_validator(value):
@@ -57,25 +56,6 @@ def datetime_validator(value):
         raise Invalid(u"Could not parse datetime from value \"{val}\"".format(val=value))
 
 
-def examples_xml_validator(value):
-    """Parse and validate student training examples XML.
-
-    Args:
-        value: The value to parse.
-
-    Returns:
-        list of training examples, serialized as dictionaries.
-
-    Raises:
-        Invalid
-
-    """
-    try:
-        return parse_examples_xml_str(value)
-    except UpdateFromXmlError:
-        raise Invalid(u"Could not parse examples from XML")
-
-
 # Schema definition for an update from the Studio JavaScript editor.
 EDITOR_UPDATE_SCHEMA = Schema({
     Required('prompt'): utf8_validator,
@@ -98,7 +78,17 @@ EDITOR_UPDATE_SCHEMA = Schema({
             Required('due', default=None): Any(datetime_validator, None),
             'must_grade': All(int, Range(min=0)),
             'must_be_graded_by': All(int, Range(min=0)),
-            'examples': All(utf8_validator, examples_xml_validator)
+            'examples': [
+                Schema({
+                    Required('answer'): utf8_validator,
+                    Required('options_selected'): [
+                        Schema({
+                            Required('criterion'): utf8_validator,
+                            Required('option'): utf8_validator
+                        })
+                    ]
+                })
+            ]
         })
     ],
     Required('feedbackprompt', default=u""): utf8_validator,

openassessment/xblock/test/data/parse_examples_xml.json

 {
     "student_training_one_example": {
         "xml": [
+            "<examples>",
             "<example>",
             "<answer>ẗëṡẗ äṅṡẅëṛ</answer>",
             "<select criterion=\"Test criterion\" option=\"Yes\" />",
-            "</example>"
+            "</example>",
+            "</examples>"
         ],
         "examples": [
             {
@@ -21,6 +23,7 @@
 
     "student_training_multiple_examples": {
         "xml": [
+            "<examples>",
             "<example>",
             "<answer>ẗëṡẗ äṅṡẅëṛ</answer>",
             "<select criterion=\"Test criterion\" option=\"Yes\" />",
@@ -30,7 +33,8 @@
             "<answer>äṅöẗḧëṛ ẗëṡẗ äṅṡẅëṛ</answer>",
             "<select criterion=\"Another test criterion\" option=\"Yes\" />",
             "<select criterion=\"Test criterion\" option=\"No\" />",
-            "</example>"
+            "</example>",
+            "</examples>"
         ],
         "examples": [
             {

openassessment/xblock/test/data/update_xblock.json

@@ -85,5 +85,61 @@
                 "due": null
             }
         ]
+    },
+
+    "student_training": {
+        "criteria": [
+            {
+                "order_num": 0,
+                "name": "тєѕт ¢яιтєяιση",
+                "prompt": "Test criterion prompt",
+                "options": [
+                    {
+                        "order_num": 0,
+                        "points": 0,
+                        "name": "Ṅö",
+                        "explanation": "Ṅö explanation"
+                    },
+                    {
+                        "order_num": 1,
+                        "points": 2,
+                        "name": "sǝʎ",
+                        "explanation": "sǝʎ explanation"
+                    }
+                ],
+                "feedback": "required"
+            }
+        ],
+        "prompt": "My new prompt.",
+        "feedback_prompt": "Feedback prompt",
+        "submission_due": "4014-02-27T09:46",
+        "submission_start": "4014-02-10T09:46",
+        "title": "My new title.",
+        "assessments": [
+            {
+                "name": "student-training",
+                "examples": [
+                    {
+                        "answer": "Ṫḧïṡ ïṡ äṅ äṅṡẅëṛ",
+                        "options_selected": [
+                            { "criterion": "тєѕт ¢яιтєяιση", "option": "Ṅö" }
+                        ]
+                    },
+                    {
+                        "answer": "This is another answer",
+                        "options_selected": [
+                            { "criterion": "тєѕт ¢яιтєяιση", "option": "sǝʎ" }
+                        ]
+                    }
+                ]
+            },
+            {
+                "name": "peer-assessment",
+                "must_grade": 5,
+                "must_be_graded_by": 3,
+                "start": null,
+                "due": "4014-03-10T00:00"
+            }
+        ]
     }
 }

openassessment/xblock/test/test_studio.py

@@ -4,7 +4,6 @@ View-level tests for Studio view of OpenAssessment XBlock.
 
 import json
 import datetime as dt
-import lxml.etree as etree
 import pytz
 from ddt import ddt, file_data
 from .base import scenario, XBlockHandlerTestCase
@@ -31,7 +30,13 @@ class StudioViewTest(XBlockHandlerTestCase):
     @file_data('data/invalid_update_xblock.json')
     @scenario('data/basic_scenario.xml')
     def test_update_context_invalid_request_data(self, xblock, data):
-        expected_error = data.pop('expected_error')
+        # All schema validation errors have the same error message, so use that as the default
+        # Remove the expected error from the dictionary so we don't get an unexpected key error.
+        if 'expected_error' in data:
+            expected_error = data.pop('expected_error')
+        else:
+            expected_error = 'error updating xblock configuration'
+
         xblock.published_date = None
         resp = self.request(xblock, 'update_editor_context', json.dumps(data), response_format='json')
         self.assertFalse(resp['success'])

openassessment/xblock/test/test_xml.py

@@ -12,8 +12,8 @@ from django.test import TestCase
 import ddt
 from openassessment.xblock.openassessmentblock import OpenAssessmentBlock
 from openassessment.xblock.xml import (
-    serialize_content, parse_from_xml_str, parse_rubric_xml_str,
-    parse_examples_xml_str, parse_assessments_xml_str,
+    serialize_content, parse_from_xml_str, parse_rubric_xml,
+    parse_examples_xml, parse_assessments_xml,
     serialize_rubric_to_xml_str, serialize_examples_to_xml_str,
     serialize_assessments_to_xml_str, UpdateFromXmlError
 )
@@ -335,7 +335,8 @@ class TestParseRubricFromXml(TestCase):
 
     @ddt.file_data("data/parse_rubric_xml.json")
     def test_parse_rubric_from_xml(self, data):
-        rubric = parse_rubric_xml_str("".join(data['xml']))
+        xml = etree.fromstring("".join(data['xml']))
+        rubric = parse_rubric_xml(xml)
 
         self.assertEqual(rubric['prompt'], data['prompt'])
         self.assertEqual(rubric['feedbackprompt'], data['feedbackprompt'])
@@ -347,8 +348,8 @@ class TestParseExamplesFromXml(TestCase):
 
     @ddt.file_data("data/parse_examples_xml.json")
     def test_parse_examples_from_xml(self, data):
-        examples = parse_examples_xml_str("".join(data['xml']))
-
+        xml = etree.fromstring("".join(data['xml']))
+        examples = parse_examples_xml(xml)
         self.assertEqual(examples, data['examples'])
 
 @ddt.ddt
@@ -356,8 +357,8 @@ class TestParseAssessmentsFromXml(TestCase):
 
     @ddt.file_data("data/parse_assessments_xml.json")
     def test_parse_assessments_from_xml(self, data):
-        assessments = parse_assessments_xml_str("".join(data['xml']))
-
+        xml = etree.fromstring("".join(data['xml']))
+        assessments = parse_assessments_xml(xml)
         self.assertEqual(assessments, data['assessments'])
 
 
@@ -401,4 +402,3 @@ class TestUpdateFromXml(TestCase):
     def test_parse_from_xml_error(self, data):
         with self.assertRaises(UpdateFromXmlError):
             parse_from_xml_str("".join(data['xml']))
-

openassessment/xblock/xml.py

@@ -750,70 +750,6 @@ def parse_from_xml_str(xml):
     return parse_from_xml(_unicode_to_xml(xml))
 
 
-def parse_rubric_xml_str(xml):
-    """
-    Create a dictionary representation of the OpenAssessment XBlock rubric from
-    the given XML string.
-
-    Args:
-        xml (unicode): The XML definition of the XBlock's rubric.
-
-    Returns:
-        A dictionary of all rubric configuration.
-
-    Raises:
-        UpdateFromXmlError: The XML definition is invalid.
-        InvalidRubricError: The rubric was not semantically valid.
-
-    """
-    return parse_rubric_xml(_unicode_to_xml(xml))
-
-
-def parse_assessments_xml_str(xml):
-    """
-    Create a dictionary representation of the OpenAssessment XBlock assessments
-    from the given XML string.
-
-    Args:
-        xml (unicode): The XML definition of the XBlock's assessments.
-
-    Returns:
-        A list of dictionaries representing the deserialized XBlock
-        configuration for each assessment module.
-
-    Raises:
-        UpdateFromXmlError: The XML definition is invalid.
-        InvalidAssessmentsError: The assessments are not semantically valid.
-
-    """
-    return parse_assessments_xml(_unicode_to_xml(xml))
-
-
-def parse_examples_xml_str(xml):
-    """
-    Create a list representation of the OpenAssessment XBlock assessment
-    examples from the given XML string.
-
-    Args:
-        xml (unicode): The XML definition of the Assessment module's examples.
-
-    Returns:
-        A list of dictionaries representing the deserialized XBlock
-        configuration for each assessment example.
-
-    Raises:
-        UpdateFromXmlError: The XML definition is invalid.
-
-    """
-
-    # This should work for both wrapped and unwrapped examples. Based on our final configuration (and tests)
-    # we should handle both cases gracefully.
-    if "<examples>" not in xml:
-        xml = u"<examples>" + xml + u"</examples>"
-
-    return parse_examples_xml(list(_unicode_to_xml(xml).findall('example')))
-
-
 def _unicode_to_xml(xml):
     """
     Converts unicode string to XML node.