Skip to content

E
edx-ora2
Commit e12f27d3 by Will Daly
Options

Merge pull request #119 from edx/will/tim-86 

Post-release rubric validation
parents ecc9dde1 81896224
Showing with 177 additions and 136 deletions
apps/openassessment/assessment/test/data/invalid_rubrics.json deleted 100644 → 0
{
"zero_criteria": {
"rubric": {
"prompt": "Test Prompt",
"criteria": []
}
},
"zero_options": {
"rubric": {
"prompt": "Test Prompt",
"criteria": [
{
"order_num": 0,
"name": "Test criterion",
"prompt": "Test criterion prompt",
"options": []
}
]
}
},
"negative_points": {
"rubric": {
"prompt": "Test Prompt",
"criteria": [
{
"order_num": 0,
"name": "Test criterion",
"prompt": "Test criterion prompt",
"options": [
{
"order_num": 0,
"points": -1,
"name": "No",
"explanation": "No explanation"
}
]
}
]
}
}
}
apps/openassessment/assessment/test/data/valid_rubrics.json deleted 100644 → 0
{
"simple": {
"rubric": {
"prompt": "Test Prompt",
"criteria": [
{
"order_num": 0,
"name": "Test criterion",
"prompt": "Test criterion prompt",
"options": [
{
"order_num": 0,
"points": 0,
"name": "No",
"explanation": "No explanation"
},
{
"order_num": 1,
"points": 2,
"name": "Yes",
"explanation": "Yes explanation"
}
]
}
]
}
},
"unicode": {
"rubric": {
"prompt": "☃",
"criteria": [
{
"order_num": 0,
"name": "☃",
"prompt": "☃",
"options": [
{
"order_num": 0,
"points": 0,
"name": "☃",
"explanation": "☃"
},
{
"order_num": 1,
"points": 2,
"name": "☃",
"explanation": "☃"
}
]
}
]
}
}
}
apps/openassessment/xblock/openassessmentblock.py
......@@ -346,7 +346,7 @@ class OpenAssessmentBlock(
block.runtime.add_node_as_child(block, child, id_generator)
block = runtime.construct_xblock_from_class(cls, keys)
return update_from_xml(block, node, validator=validator(block.start, block.due))
return update_from_xml(block, node, validator=validator(block, strict_post_release=False))
def render_assessment(self, path, context_dict=None):
"""Render an Assessment Module's HTML
......@@ -461,6 +461,25 @@ class OpenAssessmentBlock(
else:
return True, None
def is_released(self, step=None):
"""
Check if a question has been released.
Kwargs:
step (str): The step in the workflow to check.
None: check whether the problem as a whole is open.
"submission": check whether the submission section is open.
"peer-assessment": check whether the peer-assessment section is open.
"self-assessment": check whether the self-assessment section is open.
Returns:
bool
"""
# By default, assume that we're published, in case the runtime doesn't support publish date.
is_published = getattr(self, 'published_date', True) is not None
is_open, reason = self.is_open(step=step)
return is_published and (is_open or reason == 'due')
def update_workflow_status(self, submission_uuid):
assessment_ui_model = self.get_assessment_module('peer-assessment')
requirements = {
......
apps/openassessment/xblock/static/js/src/oa_edit.js
......@@ -74,6 +74,9 @@ OpenAssessment.StudioUI.prototype = {
// Notify the client-side runtime that we finished saving
// so it can hide the "Saving..." notification.
ui.runtime.notify('save', {state: 'end'});
// Reload the XML definition in the editor
ui.load();
}).fail(function(msg) {
ui.showError(msg);
});
......
apps/openassessment/xblock/studio_mixin.py
......@@ -54,7 +54,7 @@ class StudioMixin(object):
"""
if 'xml' in data:
try:
update_from_xml_str(self, data['xml'], validator=validator(self.start, self.due))
update_from_xml_str(self, data['xml'], validator=validator(self))
except ValidationError as ex:
return {'success': False, 'msg': _('Validation error: {error}').format(error=ex.message)}
......
apps/openassessment/xblock/test/test_openassessment.py
......@@ -93,25 +93,29 @@ class TestDates(XBlockHandlerTestCase):
self.assert_is_open(
xblock,
dt.datetime(2014, 2, 28, 23, 59, 59),
None, False, "start"
None, False, "start",
released=False
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 1, 1, 1, 1),
None, True, None
None, True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 4, 23, 59, 59),
None, True, None
None, True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 5, 1, 1, 1),
None, False, "due"
None, False, "due",
released=True
)
@scenario('data/dates_scenario.xml')
......@@ -123,25 +127,29 @@ class TestDates(XBlockHandlerTestCase):
self.assert_is_open(
xblock,
dt.datetime(2014, 2, 28, 23, 59, 59).replace(tzinfo=pytz.utc),
"submission", False, "start"
"submission", False, "start",
released=False
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
"submission", True, None
"submission", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
"submission", True, None
"submission", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2014, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
"submission", False, "due"
"submission", False, "due",
released=True
)
@scenario('data/dates_scenario.xml')
......@@ -153,25 +161,29 @@ class TestDates(XBlockHandlerTestCase):
self.assert_is_open(
xblock,
dt.datetime(2015, 1, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
"peer-assessment", False, "start"
"peer-assessment", False, "start",
released=False
)
self.assert_is_open(
xblock,
dt.datetime(2015, 1, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
"peer-assessment", True, None
"peer-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2015, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
"peer-assessment", True, None
"peer-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2015, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
"peer-assessment", False, "due"
"peer-assessment", False, "due",
released=True
)
@scenario('data/dates_scenario.xml')
......@@ -183,25 +195,29 @@ class TestDates(XBlockHandlerTestCase):
self.assert_is_open(
xblock,
dt.datetime(2016, 1, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
"self-assessment", False, "start"
"self-assessment", False, "start",
released=False
)
self.assert_is_open(
xblock,
dt.datetime(2016, 1, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
"self-assessment", True, None
"self-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2016, 3, 31, 23, 59, 59).replace(tzinfo=pytz.utc),
"self-assessment", True, None
"self-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2016, 4, 1, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
"self-assessment", False, "due"
"self-assessment", False, "due",
released=True
)
@scenario('data/resolve_dates_scenario.xml')
......@@ -215,28 +231,53 @@ class TestDates(XBlockHandlerTestCase):
self.assert_is_open(
xblock,
dt.datetime(2014, 2, 28, 23, 59, 59).replace(tzinfo=pytz.utc),
"peer-assessment", False, "start"
"peer-assessment", False, "start",
released=False
)
self.assert_is_open(
xblock,
dt.datetime(2014, 3, 1, 1, 1, 1).replace(tzinfo=pytz.utc),
"peer-assessment", True, None
"peer-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2016, 5, 1, 23, 59, 59).replace(tzinfo=pytz.utc),
"peer-assessment", True, None
"peer-assessment", True, None,
released=True
)
self.assert_is_open(
xblock,
dt.datetime(2016, 5, 2, 1, 1, 1).replace(tzinfo=pytz.utc),
"peer-assessment", False, "due"
"peer-assessment", False, "due",
released=True
)
def assert_is_open(self, xblock, now, step, expected_is_open, expected_reason):
@scenario('data/basic_scenario.xml')
def test_is_released_unpublished(self, xblock):
# Simulate the runtime published_date mixin field
# The scenario doesn't provide a start date, so `is_released()`
# should be controlled only by the published date.
xblock.published_date = None
self.assertFalse(xblock.is_released())
@scenario('data/basic_scenario.xml')
def test_is_released_published(self, xblock):
# Simulate the runtime published_date mixin field
# The scenario doesn't provide a start date, so `is_released()`
# should be controlled only by the published date.
xblock.published_date = dt.datetime(2013, 1, 1).replace(tzinfo=pytz.utc)
self.assertTrue(xblock.is_released())
@scenario('data/basic_scenario.xml')
def test_is_released_no_published_date_field(self, xblock):
# If the runtime doesn't provide a published_date field, assume we've been published
self.assertTrue(xblock.is_released())
def assert_is_open(self, xblock, now, step, expected_is_open, expected_reason, released=None):
"""
Assert whether the XBlock step is open/closed.
......@@ -247,6 +288,9 @@ class TestDates(XBlockHandlerTestCase):
expected_is_open (bool): Do we expect the step to be open or closed?
expecetd_reason (str): Either "start", "due", or None.
Kwargs:
released (bool): If set, check whether the XBlock has been released.
Raises:
AssertionError
"""
......@@ -260,4 +304,7 @@ class TestDates(XBlockHandlerTestCase):
is_open, reason = xblock.is_open(step=step)
self.assertEqual(is_open, expected_is_open)
self.assertEqual(reason, expected_reason)
\ No newline at end of file
self.assertEqual(reason, expected_reason)
if released is not None:
self.assertEqual(xblock.is_released(step=step), released)
apps/openassessment/xblock/test/test_studio.py
......@@ -3,8 +3,10 @@ View-level tests for Studio view of OpenAssessment XBlock.
"""
import json
import datetime as dt
import lxml.etree as etree
import mock
import pytz
from ddt import ddt, data
from .base import scenario, XBlockHandlerTestCase
......@@ -44,6 +46,10 @@ class StudioViewTest(XBlockHandlerTestCase):
@scenario('data/basic_scenario.xml')
def test_update_xml(self, xblock):
# Set the XBlock's release date to the future,
# so we are not restricted in what we can edit
xblock.start = dt.datetime(3000, 1, 1).replace(tzinfo=pytz.utc).isoformat()
request = json.dumps({'xml': self.load_fixture_str('data/updated_block.xml')})
# Verify the response is successfully
......@@ -60,6 +66,17 @@ class StudioViewTest(XBlockHandlerTestCase):
self.assertEqual(xblock.rubric_criteria[0]['prompt'], 'Test criterion prompt')
@scenario('data/basic_scenario.xml')
def test_update_xml_post_release(self, xblock):
# XBlock start date defaults to already open,
# so we should get an error when trying to update anything that change the number of points
request = json.dumps({'xml': self.load_fixture_str('data/updated_block.xml')})
# Verify the response is successfully
resp = self.request(xblock, 'update_xml', request, response_format='json')
self.assertFalse(resp['success'])
@scenario('data/basic_scenario.xml')
def test_update_xml_invalid_request_data(self, xblock):
resp = self.request(xblock, 'update_xml', json.dumps({}), response_format='json')
self.assertFalse(resp['success'])
......
apps/openassessment/xblock/test/test_validation.py
......@@ -41,18 +41,19 @@ class AssessmentValidationTest(TestCase):
self.assertGreater(len(msg), 0)
@ddt.ddt
class RubricValidationTest(TestCase):
@ddt.file_data('data/valid_rubrics.json')
def test_valid_assessment(self, data):
success, msg = validate_rubric(data['rubric'])
success, msg = validate_rubric(data['rubric'], data['current_rubric'], data['is_released'])
self.assertTrue(success)
self.assertEqual(msg, u'')
@ddt.file_data('data/invalid_rubrics.json')
def test_invalid_assessment(self, data):
success, msg = validate_rubric(data['rubric'])
success, msg = validate_rubric(data['rubric'], data['current_rubric'], data['is_released'])
self.assertFalse(success)
self.assertGreater(len(msg), 0)
......
apps/openassessment/xblock/validation.py
"""
Validate changes to an XBlock before it is updated.
"""
from django.utils.translation import ugettext as _
from openassessment.assessment.serializers import rubric_from_dict, InvalidRubric
from openassessment.xblock.resolve_dates import resolve_dates, DateValidationError, InvalidDateFormat
def _match_by_name(items, others):
"""
Given two lists of dictionaries, each containing "name" keys,
return a set of tuples, where the items in the tuple are dictionaries
with the same "name" keys.
Args:
items (list of dict): Items to match, each of which must contain a "name" key.
others (list of dict): Items to match, each of which must contain a "name" key.
Returns:
list of tuples, each containing two dictionaries
Raises:
IndexError: A dictionary does no contain a 'name' key.
"""
# Sort each dictionary by its "name" key, then zip them and return
key_func = lambda x: x['name']
return zip(sorted(items, key=key_func), sorted(others, key=key_func))
def validate_assessments(assessments, enforce_peer_then_self=False):
"""
Check that the assessment dict is semantically valid.
......@@ -54,12 +76,14 @@ def validate_assessments(assessments, enforce_peer_then_self=False):
return (True, u'')
def validate_rubric(rubric_dict):
def validate_rubric(rubric_dict, current_rubric, is_released):
"""
Check that the rubric is semantically valid.
Args:
rubric_dict (dict): Serialized Rubric model
rubric_dict (dict): Serialized Rubric model representing the updated state of the rubric.
current_rubric (dict): Serialized Rubric model representing the current state of the rubric.
is_released (bool): True if and only if the problem has been released.
Returns:
tuple (is_valid, msg) where
......@@ -70,8 +94,26 @@ def validate_rubric(rubric_dict):
rubric_from_dict(rubric_dict)
except InvalidRubric:
return (False, u'Rubric definition is not valid')
else:
return (True, u'')
# After a problem is released, authors are allowed to change text,
# but nothing that would change the point value of a rubric.
if is_released:
# Number of criteria must be the same
if len(rubric_dict['criteria']) != len(current_rubric['criteria']):
return (False, u'Number of criteria cannot be changed after a problem is released.')
# Number of options for each criterion must be the same
for new_criterion, old_criterion in _match_by_name(rubric_dict['criteria'], current_rubric['criteria']):
if len(new_criterion['options']) != len(old_criterion['options']):
return (False, u'Number of options cannot be changed after a problem is released.')
else:
for new_option, old_option in _match_by_name(new_criterion['options'], old_criterion['options']):
if new_option['points'] != old_option['points']:
return (False, u'Point values cannot be changed after a problem is released.')
return (True, u'')
def validate_dates(start, end, date_ranges):
......@@ -96,30 +138,41 @@ def validate_dates(start, end, date_ranges):
return (True, u'')
def validator(start, due):
def validator(oa_block, strict_post_release=True):
"""
Return a validator function configured with the problem's start and end dates.
Return a validator function configured for the XBlock.
This will validate assessments, rubrics, and dates.
Args:
start (str): ISO-formatted date string indicating when the problem opens.
end (str): ISO-formatted date string indicating when the problem closes.
oa_block (OpenAssessmentBlock): The XBlock being updated.
Kwargs:
strict_post_release (bool): If true, restrict what authors can update once
a problem has been released.
Returns:
callable, of a form that can be passed to `update_from_xml`.
"""
def _inner(rubric_dict, submission_dict, assessments):
success, msg = validate_assessments(assessments, enforce_peer_then_self=True)
if not success:
return (False, msg)
success, msg = validate_rubric(rubric_dict)
current_rubric = {
'prompt': oa_block.prompt,
'criteria': oa_block.rubric_criteria
}
success, msg = validate_rubric(
rubric_dict, current_rubric,
strict_post_release and oa_block.is_released()
)
if not success:
return (False, msg)
submission_dates = [(start, submission_dict['due'])]
submission_dates = [(oa_block.start, submission_dict['due'])]
assessment_dates = [(asmnt['start'], asmnt['due']) for asmnt in assessments]
success, msg = validate_dates(start, due, submission_dates + assessment_dates)
success, msg = validate_dates(oa_block.start, oa_block.due, submission_dates + assessment_dates)
if not success:
return (False, msg)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment