Merge pull request #232 from edx/will/no-duplicate-criteria

Validation check for duplicate criteria and options

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",

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
+}

apps/openassessment/xblock/validation.py

 """
 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