Refactor integer field validation

Add field validation for peer assessment must grade and must be graded by.

openassessment/templates/openassessmentblock/edit/oa_edit_peer_assessment.html

@@ -20,14 +20,14 @@
                 <li class="field comp-setting-entry">
                     <div class="wrapper-comp-setting">
                         <label for="peer_assessment_must_grade" class="setting-label">{% trans "Must Grade" %}</label>
-                        <input id="peer_assessment_must_grade" class="input setting-input" type="number" value="{{ assessments.peer_assessment.must_grade }}">
+                        <input id="peer_assessment_must_grade" class="input setting-input" type="number" value="{{ assessments.peer_assessment.must_grade }}" min="0" max="99">
                     </div>
                     <p class="setting-help">{% trans "Each student must assess this number of peer responses in order to recieve a grade."%}</p>
                 </li>
                 <li class="field comp-setting-entry">
                     <div class="wrapper-comp-setting">
                         <label for="peer_assessment_graded_by" class="setting-label"> {% trans "Graded By" %}</label>
-                        <input id="peer_assessment_graded_by" class="input setting-input" type="number" value="{{ assessments.peer_assessment.must_be_graded_by }}">
+                        <input id="peer_assessment_graded_by" class="input setting-input" type="number" value="{{ assessments.peer_assessment.must_be_graded_by }}" min="0" max="99">
                     </div>
                     <p class="setting-help">{% trans "Each response must be assessed by at least this many students in order to tabulate a score."%}</p>
                 </li>

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

@@ -75,6 +75,34 @@ describe("OpenAssessment edit assessment views", function() {
                 "Peer assessment due is invalid"
             );
         });
+
+        it("validates the must grade field", function() {
+            // Invalid value (not a number)
+            view.mustGradeNum("123abc");
+            expect(view.validate()).toBe(false);
+            expect(view.validationErrors()).toContain("Peer assessment must grade is invalid");
+
+            view.clearValidationErrors();
+
+            // Valid value
+            view.mustGradeNum("34");
+            expect(view.validate()).toBe(true);
+            expect(view.validationErrors()).toEqual([]);
+        });
+
+        it("validates the must be graded by field", function() {
+            // Invalid value (not a number)
+            view.mustBeGradedByNum("123abc");
+            expect(view.validate()).toBe(false);
+            expect(view.validationErrors()).toContain("Peer assessment must be graded by is invalid");
+
+            view.clearValidationErrors();
+
+            // Valid value
+            view.mustBeGradedByNum("34");
+            expect(view.validate()).toBe(true);
+            expect(view.validationErrors()).toEqual([]);
+        });
     });
 
     describe("OpenAssessment.EditSelfAssessmentView", function() {

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

@@ -38,7 +38,10 @@ Returns:
 OpenAssessment.RubricOption = function(element, notifier) {
     this.element = element;
     this.notifier = notifier;
-    this.MAX_POINTS = 1000;
+    this.pointsField = new OpenAssessment.IntField(
+        $(".openassessment_criterion_option_points", this.element),
+        { min: 0, max: 999 }
+    );
     $(this.element).focusout($.proxy(this.updateHandler, this));
 };
 
@@ -99,8 +102,8 @@ OpenAssessment.RubricOption.prototype = {
 
     **/
     points: function(points) {
-        var sel = $('.openassessment_criterion_option_points', this.element);
-        return OpenAssessment.Fields.intField(sel, points);
+        if (points !== undefined) { this.pointsField.set(points); }
+        return this.pointsField.get();
     },
 
     /**
@@ -196,14 +199,7 @@ OpenAssessment.RubricOption.prototype = {
 
     **/
     validate: function() {
-        var pointString = $(".openassessment_criterion_option_points", this.element).val();
-        var matches = pointString.trim().match(/^\d{1,3}$/g);
-        var isValid = (matches !== null);
-        if (!isValid) {
-            $(".openassessment_criterion_option_points", this.element)
-                .addClass("openassessment_highlighted_field");
-        }
-        return isValid;
+        return this.pointsField.validate();
     },
 
     /**
@@ -215,8 +211,7 @@ OpenAssessment.RubricOption.prototype = {
 
     **/
     validationErrors: function() {
-        var sel = $(".openassessment_criterion_option_points", this.element);
-        var hasError = sel.hasClass("openassessment_highlighted_field");
+        var hasError = (this.pointsField.validationErrors().length > 0);
         return hasError ? ["Option points are invalid"] : [];
     },
 
@@ -224,8 +219,7 @@ OpenAssessment.RubricOption.prototype = {
     Clear all validation errors from the UI.
     **/
     clearValidationErrors: function() {
-        $(".openassessment_criterion_option_points", this.element)
-            .removeClass("openassessment_highlighted_field");
+        this.pointsField.clearValidationErrors();
     }
 };
 

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

@@ -11,6 +11,14 @@ Returns:
 OpenAssessment.EditPeerAssessmentView = function(element) {
     this.element = element;
     this.name = "peer-assessment";
+    this.mustGradeField = new OpenAssessment.IntField(
+        $("#peer_assessment_must_grade", this.element),
+        { min: 0, max: 99 }
+    );
+    this.mustBeGradedByField = new OpenAssessment.IntField(
+        $("#peer_assessment_graded_by", this.element),
+        { min: 0, max: 99 }
+    );
 
     // Configure the toggle checkbox to enable/disable this assessment
     new OpenAssessment.ToggleControl(
@@ -83,8 +91,8 @@ OpenAssessment.EditPeerAssessmentView.prototype = {
         int
     **/
     mustGradeNum: function(num) {
-        var sel = $("#peer_assessment_must_grade", this.element);
-        return OpenAssessment.Fields.intField(sel, num);
+        if (num !== undefined) { this.mustGradeField.set(num); }
+        return this.mustGradeField.get();
     },
 
     /**
@@ -97,8 +105,8 @@ OpenAssessment.EditPeerAssessmentView.prototype = {
         int
     **/
     mustBeGradedByNum: function(num) {
-        var sel = $("#peer_assessment_graded_by", this.element);
-        return OpenAssessment.Fields.intField(sel, num);
+        if (num !== undefined) { this.mustBeGradedByField.set(num); }
+        return this.mustBeGradedByField.get();
     },
 
     /**
@@ -149,7 +157,9 @@ OpenAssessment.EditPeerAssessmentView.prototype = {
     validate: function() {
         var startValid = this.startDatetimeControl.validate();
         var dueValid = this.dueDatetimeControl.validate();
-        return startValid && dueValid;
+        var mustGradeValid = this.mustGradeField.validate();
+        var mustBeGradedByValid = this.mustBeGradedByField.validate();
+        return startValid && dueValid && mustGradeValid && mustBeGradedByValid;
     },
 
    /**
@@ -168,6 +178,12 @@ OpenAssessment.EditPeerAssessmentView.prototype = {
         if (this.dueDatetimeControl.validationErrors().length > 0) {
             errors.push("Peer assessment due is invalid");
         }
+        if (this.mustGradeField.validationErrors().length > 0) {
+            errors.push("Peer assessment must grade is invalid");
+        }
+        if(this.mustBeGradedByField.validationErrors().length > 0) {
+            errors.push("Peer assessment must be graded by is invalid");
+        }
         return errors;
     },
 
@@ -177,6 +193,8 @@ OpenAssessment.EditPeerAssessmentView.prototype = {
     clearValidationErrors: function() {
         this.startDatetimeControl.clearValidationErrors();
         this.dueDatetimeControl.clearValidationErrors();
+        this.mustGradeField.clearValidationErrors();
+        this.mustBeGradedByField.clearValidationErrors();
     },
 };
 

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

@@ -3,23 +3,108 @@ Utilities for reading / writing fields.
 **/
 OpenAssessment.Fields = {
     stringField: function(sel, value) {
-        if (typeof(value) !== "undefined") { sel.val(value); }
+        if (value !== undefined) { sel.val(value); }
         return sel.val();
     },
 
-    intField: function(sel, value) {
-        if (typeof(value) !== "undefined") { sel.val(value); }
-        return parseInt(sel.val(), 10);
-    },
-
     booleanField: function(sel, value) {
-        if (typeof(value) !== "undefined") { sel.prop("checked", value); }
+        if (value !== undefined) { sel.prop("checked", value); }
         return sel.prop("checked");
     },
 };
 
 
 /**
+Integer input.
+
+Args:
+    inputSel (JQuery selector or DOM element): The input field.
+
+Keyword args:
+    min (int): The minimum value allowed in the input.
+    max (int): The maximum value allowed in the input.
+
+**/
+OpenAssessment.IntField = function(inputSel, restrictions) {
+    this.max = restrictions.max;
+    this.min = restrictions.min;
+    this.input = $(inputSel);
+};
+
+OpenAssessment.IntField.prototype = {
+
+    /**
+    Retrieve the integer value from the input.
+    Decimal values will be truncated, and non-numeric
+    values will become NaN.
+
+    Returns:
+        integer or NaN
+    **/
+    get: function() {
+        return parseInt(this.input.val().trim(), 10);
+    },
+
+    /**
+    Set the input value.
+
+    Args:
+        val (int or string)
+
+    **/
+    set: function(val) {
+        this.input.val(val);
+    },
+
+    /**
+    Mark validation errors if the field does not satisfy the restrictions.
+    Fractional values are not considered valid integers.
+
+    This will trim whitespace from the field, so "   34  " would be considered
+    a valid input.
+
+    Returns:
+        Boolean indicating whether the field's value is valid.
+
+    **/
+    validate: function() {
+        var value = this.get();
+        var isValid = !isNaN(value) && value >= this.min && value <= this.max;
+
+        // Decimal values not allowed
+        if (this.input.val().indexOf(".") !== -1) {
+            isValid = false;
+        }
+
+        if (!isValid) {
+            this.input.addClass("openassessment_highlighted_field");
+        }
+        return isValid;
+    },
+
+    /**
+    Clear any validation errors from the UI.
+    **/
+    clearValidationErrors: function() {
+        this.input.removeClass("openassessment_highlighted_field");
+    },
+
+    /**
+    Return a list of validation errors currently displayed
+    in the UI.  Mainly useful for testing.
+
+    Returns:
+        list of strings
+
+    **/
+    validationErrors: function() {
+        var hasError = this.input.hasClass("openassessment_highlighted_field");
+        return hasError ? ["Int field is invalid"] : [];
+    },
+};
+
+
+/**
 Show and hide elements based on a checkbox.
 
 Args: