Add client-side validation for student training examples with no options selected.

Simplify server-side validation message for training examples
Refactor training example JavaScript

openassessment/xblock/static/js/spec/studio/oa_edit_assessments.js

@@ -165,6 +165,10 @@ describe("OpenAssessment edit assessment views", function() {
         var view = null;
 
         beforeEach(function() {
+            // We need to load the student-training specific editing view
+            // so that the student training example template is properly initialized.
+            loadFixtures('oa_edit_student_training.html');
+
             var element = $("#oa_student_training_editor").get(0);
             view = new OpenAssessment.EditStudentTrainingView(element);
         });
@@ -173,28 +177,65 @@ describe("OpenAssessment edit assessment views", function() {
         it("loads a description", function () {
             // This assumes a particular structure of the DOM,
             // which is set by the HTML fixture.
-            var examples = view.exampleContainer.getItemValues();
-            expect(examples.length).toEqual(0);
+            expect(view.description()).toEqual({
+                examples: [
+                    {
+                        answer: 'Test answer',
+                        options_selected: [
+                            {
+                                criterion: 'criterion_with_two_options',
+                                option: 'option_1'
+                            }
+                        ]
+                    }
+                ]
+            });
         });
+
         it("modifies a description", function () {
             view.exampleContainer.add();
-            var examples = view.exampleContainer.getItemValues();
-            expect(examples.length).toEqual(1);
-        });
-        it("returns the correct format", function () {
-            view.exampleContainer.add();
-            var examples = view.exampleContainer.getItemValues();
-            expect(examples).toEqual(
-                [
+            expect(view.description()).toEqual({
+                examples: [
+                    {
+                        answer: 'Test answer',
+                        options_selected: [
+                            {
+                                criterion: 'criterion_with_two_options',
+                                option: 'option_1'
+                            }
+                        ]
+                    },
                     {
-                        answer: "",
-                        options_selected: []
+                        answer: '',
+                        options_selected: [
+                            {
+                                criterion: 'criterion_with_two_options',
+                                option: ''
+                            }
+                        ]
                     }
                 ]
-            );
+            });
         });
 
         it("shows an alert when disabled", function() { testAlertOnDisable(view); });
+
+        it("validates selected options", function() {
+            // On page load, the examples should be valid
+            expect(view.validate()).toBe(true);
+            expect(view.validationErrors()).toEqual([]);
+
+            // Add a new training example (default no option selected)
+            view.exampleContainer.add();
+
+            // Now there should be a validation error
+            expect(view.validate()).toBe(false);
+            expect(view.validationErrors()).toContain("Student training example is invalid.");
+
+            // Clear validation errors
+            view.clearValidationErrors();
+            expect(view.validationErrors()).toEqual([]);
+        });
     });
 
     describe("OpenAssessment.EditExampleBasedAssessmentView", function() {

openassessment/xblock/static/js/src/studio/oa_container_item.js

@@ -34,7 +34,7 @@ OpenAssessment.ItemUtilities = {
         var points = $(element).data('points');
         var label = $(element).data('label');
         // We don't want the lack of a label to make it look like - 1 points.
-        if (label == ""){
+        if (label === ""){
             label = gettext('Unnamed Option');
         }
         var singularString = label + " - " + points + " point";
@@ -484,12 +484,17 @@ OpenAssessment.RubricCriterion.prototype = {
  **/
 OpenAssessment.TrainingExample = function(element){
     this.element = element;
-    // Goes through and instantiates the option description in the training example for each option.
-    $(".openassessment_training_example_criterion_option", this.element) .each( function () {
-        $('option', this).each(function(){
-            OpenAssessment.ItemUtilities.refreshOptionString($(this));
-        });
-    });
+    this.criteria = $(".openassessment_training_example_criterion_option", this.element);
+    this.answer = $('.openassessment_training_example_essay', this.element).first();
+
+    // Initialize the option label in the training example for each option.
+    this.criteria.each(
+        function () {
+            $('option', this).each(function(){
+                OpenAssessment.ItemUtilities.refreshOptionString($(this));
+            });
+        }
+    );
 };
 
 OpenAssessment.TrainingExample.prototype = {
@@ -500,16 +505,17 @@ OpenAssessment.TrainingExample.prototype = {
 
         // Iterates through all of the options selected by the training example, and adds them
         // to a list.
-        var optionsSelected = [];
-        $(".openassessment_training_example_criterion_option", this.element) .each( function () {
-            optionsSelected.push({
-                criterion: $(this).data('criterion'),
-                option: $(this).prop('value')
-            });
-        });
+        var optionsSelected = this.criteria.map(
+            function () {
+                return {
+                    criterion: $(this).data('criterion'),
+                    option: $(this).prop('value')
+                };
+            }
+        ).get();
 
         return {
-            answer: $('.openassessment_training_example_essay', this.element).first().prop('value'),
+            answer: this.answer.prop('value'),
             options_selected: optionsSelected
         };
     },
@@ -519,7 +525,61 @@ OpenAssessment.TrainingExample.prototype = {
     removeHandler: function() {},
     updateHandler: function() {},
 
-    validate: function() { return true; },
-    validationErrors: function() { return []; },
-    clearValidationErrors: function() {}
+    /**
+    Mark validation errors.
+
+    Returns:
+        Boolean indicating whether the criterion is valid.
+
+    **/
+    validate: function() {
+        var isValid = true;
+
+        this.criteria.each(
+            function() {
+                var isOptionValid = ($(this).prop('value') !== "");
+                isValid = isOptionValid && isValid;
+
+                if (!isOptionValid) {
+                    $(this).addClass("openassessment_highlighted_field");
+                }
+            }
+        );
+
+        return isValid;
+    },
+
+    /**
+    Return a list of validation errors visible in the UI.
+    Mainly useful for testing.
+
+    Returns:
+        list of string
+
+    **/
+    validationErrors: function() {
+        var errors = [];
+        this.criteria.each(
+            function() {
+                var hasError = $(this).hasClass("openassessment_highlighted_field");
+                if (hasError) {
+                    errors.push("Student training example is invalid.");
+                }
+            }
+        );
+        return errors;
+    },
+
+    /**
+    Retrieve all elements representing items in this container.
+
+    Returns:
+        array of container item objects
+
+    **/
+    clearValidationErrors: function() {
+        this.criteria.each(
+            function() { $(this).removeClass("openassessment_highlighted_field"); }
+        );
+    }
 };
\ No newline at end of file

openassessment/xblock/static/js/src/studio/oa_edit_assessment.js

@@ -466,18 +466,56 @@ OpenAssessment.EditStudentTrainingView.prototype = {
     },
 
     /**
-     Gets the ID of the assessment
+    Gets the ID of the assessment
 
-     Returns:
-     string (CSS ID of the Element object)
-     **/
+    Returns:
+    string (CSS ID of the Element object)
+    **/
     getID: function() {
         return $(this.element).attr('id');
     },
 
-    validate: function() { return true; },
-    validationErrors: function() { return []; },
-    clearValidationErrors: function() {},
+    /**
+    Mark validation errors.
+
+    Returns:
+        Boolean indicating whether the view is valid.
+
+    **/
+    validate: function() {
+        var isValid = true;
+
+        $.each(this.exampleContainer.getAllItems(), function() {
+            isValid = this.validate() && isValid;
+        });
+
+        return isValid;
+    },
+
+    /**
+    Return a list of validation errors visible in the UI.
+    Mainly useful for testing.
+
+    Returns:
+        list of string
+
+    **/
+    validationErrors: function() {
+        var errors = [];
+        $.each(this.exampleContainer.getAllItems(), function() {
+            errors = errors.concat(this.validationErrors());
+        });
+        return errors;
+    },
+
+    /**
+    Clear all validation errors from the UI.
+    **/
+    clearValidationErrors: function() {
+        $.each(this.exampleContainer.getAllItems(), function() {
+            this.clearValidationErrors();
+        });
+    },
 };
 
 /**

openassessment/xblock/test/data/invalid_update_xblock.json

@@ -955,7 +955,7 @@
             }
         ],
         "editor_assessments_order": ["student-training", "peer-assessment", "self-assessment"],
-        "expected_error": "example 1 has an extra option"
+        "expected_error": "student training example option does not match the rubric"
     },
 
     "missing_editor_assessments_order": {

openassessment/xblock/test/test_validation.py

@@ -303,7 +303,7 @@ class ValidationIntegrationTest(TestCase):
         # Expect a validation error
         is_valid, msg = self.validator(self.RUBRIC, mutated_assessments)
         self.assertFalse(is_valid)
-        self.assertEqual(msg, u'Example 1 has an extra option for "Invalid criterion!"; Example 1 is missing an option for "vocabulary"')
+        self.assertEqual(msg, u'Student training example option does not match the rubric.')
 
     def test_student_training_examples_invalid_option(self):
         # Mutate the assessment training examples so the option names don't match the rubric
@@ -313,7 +313,7 @@ class ValidationIntegrationTest(TestCase):
         # Expect a validation error
         is_valid, msg = self.validator(self.RUBRIC, mutated_assessments)
         self.assertFalse(is_valid)
-        self.assertEqual(msg, u'Example 1 has an invalid option for "vocabulary": "Invalid option!"')
+        self.assertEqual(msg, u'Student training example option does not match the rubric.')
 
     def test_example_based_assessment_duplicate_point_values(self):
         # Mutate the rubric so that two options have the same point value

openassessment/xblock/validation.py

@@ -294,7 +294,7 @@ def validate_assessment_examples(rubric_dict, assessments, _):
             # examples against the rubric.
             errors = validate_training_examples(rubric_dict, examples)
             if errors:
-                return False, "; ".join(errors)
+                return False, _(u"Student training example option does not match the rubric.")
 
     return True, u''