diff --git a/apps/openassessment/xblock/test/data/invalid_rubrics.json b/apps/openassessment/xblock/test/data/invalid_rubrics.json index cf747b1..d38cc76 100644 --- a/apps/openassessment/xblock/test/data/invalid_rubrics.json +++ b/apps/openassessment/xblock/test/data/invalid_rubrics.json @@ -47,6 +47,71 @@ "is_released": false }, + "duplicate_criteria_names": { + "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" + } + ] + }, + { + "order_num": 0, + "name": "Test criterion", + "prompt": "Test criterion prompt", + "options": [ + { + "order_num": 0, + "points": 1, + "name": "Yes", + "explanation": "Yes explanation" + } + ] + } + ] + }, + "current_rubric": null, + "is_released": false + }, + + "duplicate_option_names": { + "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" + }, + { + "order_num": 0, + "points": 1, + "name": "No", + "explanation": "Second no explanation" + } + ] + } + ] + }, + "current_rubric": null, + "is_released": false + }, + "change_points_after_release": { "rubric": { "prompt": "Test Prompt", diff --git a/apps/openassessment/xblock/test/data/valid_rubrics.json b/apps/openassessment/xblock/test/data/valid_rubrics.json index 308733e..b06d0b5 100644 --- a/apps/openassessment/xblock/test/data/valid_rubrics.json +++ b/apps/openassessment/xblock/test/data/valid_rubrics.json @@ -40,13 +40,13 @@ { "order_num": 0, "points": 0, - "name": "☃", + "name": "☃ 1", "explanation": "☃" }, { "order_num": 1, "points": 2, - "name": "☃", + "name": "☃ 2", "explanation": "☃" } ] @@ -734,4 +734,4 @@ }, "is_released": true } -} \ No newline at end of file +} diff --git a/apps/openassessment/xblock/validation.py b/apps/openassessment/xblock/validation.py index 14f6018..e2d70c2 100644 --- a/apps/openassessment/xblock/validation.py +++ b/apps/openassessment/xblock/validation.py @@ -1,6 +1,7 @@ """ Validate changes to an XBlock before it is updated. """ +from collections import Counter 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 @@ -27,6 +28,21 @@ def _match_by_name(items, others): return zip(sorted(items, key=key_func), sorted(others, key=key_func)) +def _duplicates(items): + """ + Given an iterable of items, return a set of duplicate items in the list. + + Args: + items (list): The list of items, which may contain duplicates. + + Returns: + set: The set of duplicate items in the list. + + """ + counts = Counter(items) + return set(x for x in items if counts[x] > 1) + + def validate_assessments(assessments, enforce_peer_then_self=False): """ Check that the assessment dict is semantically valid. @@ -95,6 +111,23 @@ def validate_rubric(rubric_dict, current_rubric, is_released): except InvalidRubric: return (False, u'Rubric definition is not valid') + # No duplicate criteria names + duplicates = _duplicates([criterion['name'] for criterion in rubric_dict['criteria']]) + if len(duplicates) > 0: + msg = u"Criteria duplicate name(s): {duplicates}".format( + duplicates=", ".join(duplicates) + ) + return (False, msg) + + # No duplicate option names within a criterion + for criterion in rubric_dict['criteria']: + duplicates = _duplicates([option['name'] for option in criterion['options']]) + if len(duplicates) > 0: + msg = u"Options in '{criterion}' have duplicate name(s): {duplicates}".format( + criterion=criterion['name'], duplicates=", ".join(duplicates) + ) + return (False, msg) + # 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: @@ -178,4 +211,4 @@ def validator(oa_block, strict_post_release=True): return (True, u'') - return _inner \ No newline at end of file + return _inner