Fix validation in many cases, and ensure that the notification bar pops up correctly.

cms/static/js/models/settings/course_grading_policy.js

@@ -71,7 +71,7 @@ CMS.Models.Settings.CourseGrader = Backbone.Model.extend({
     },
     validate : function(attrs) {
         var errors = {};
-        if (attrs['type']) {
+        if (_.has(attrs, 'type')) {
             if (_.isEmpty(attrs['type'])) {
                 errors.type = "The assignment type must have a name.";
             }
@@ -83,7 +83,7 @@ CMS.Models.Settings.CourseGrader = Backbone.Model.extend({
                 }
             }
         }
-        if (attrs['weight']) {
+        if (_.has(attrs, 'weight')) {
             if (!isFinite(attrs.weight) || /\D+/.test(attrs.weight)) {
                 errors.weight = "Please enter an integer between 0 and 100.";
             }
@@ -97,19 +97,19 @@ CMS.Models.Settings.CourseGrader = Backbone.Model.extend({
 //                  errors.weight = "The weights cannot add to more than 100.";
                 }
             }}
-        if (attrs['min_count']) {
+        if (_.has(attrs, 'min_count')) {
             if (!isFinite(attrs.min_count) || /\D+/.test(attrs.min_count)) {
                 errors.min_count = "Please enter an integer.";
             }
             else attrs.min_count = parseInt(attrs.min_count);
         }
-        if (attrs['drop_count']) {
+        if (_.has(attrs, 'drop_count')) {
             if (!isFinite(attrs.drop_count) || /\D+/.test(attrs.drop_count)) {
                 errors.drop_count = "Please enter an integer.";
             }
             else attrs.drop_count = parseInt(attrs.drop_count);
         }
-        if (attrs['min_count'] && attrs['drop_count'] && attrs.drop_count > attrs.min_count) {
+        if (_.has(attrs, 'min_count') && _.has(attrs, 'drop_count') && attrs.drop_count > attrs.min_count) {
             errors.drop_count = "Cannot drop more " + attrs.type + " than will assigned.";
         }
         if (!_.isEmpty(errors)) return errors;

cms/static/js/views/settings/advanced_view.js

@@ -56,6 +56,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         CodeMirror.fromTextArea(textarea, {
             mode: "application/json", lineNumbers: false, lineWrapping: false,
             onChange: function(instance, changeobj) {
+                instance.save()
                 // this event's being called even when there's no change :-(
                 if (instance.getValue() !== oldValue) {
                     var message = gettext("Your changes will not take effect until you save your progress. Take care with key and value formatting, as validation is not implemented.");
@@ -94,8 +95,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
                     }
                 }
                 if (JSONValue !== undefined) {
-                    self.clearValidationErrors();
-                    self.model.set(key, JSONValue, {validate: true});
+                    self.model.set(key, JSONValue);
                 }
             }
         });
@@ -115,7 +115,8 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
                 analytics.track('Saved Advanced Settings', {
                     'course': course_location_analytics
                 });
-            }
+            },
+            silent: true
         });
     },
     revertView: function() {

cms/static/js/views/settings/main_settings_view.js

@@ -89,7 +89,6 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
         var timefield = $(div).find("input:.time");
         var cachethis = this;
         var setfield = function () {
-            cachethis.clearValidationErrors();
             var date = datefield.datepicker('getDate');
             if (date) {
                 var time = timefield.timepicker("getSecondsFromMidnight");
@@ -98,14 +97,16 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
                 }
                 var newVal = new Date(date.getTime() + time * 1000);
                 if (!cacheModel.has(fieldName) || cacheModel.get(fieldName).getTime() !== newVal.getTime()) {
-                    cacheModel.set(fieldName, newVal, {validate: true});
+                    cachethis.clearValidationErrors();
+                    cachethis.setAndValidate(fieldName, newVal);
                 }
             }
             else {
                 // Clear date (note that this clears the time as well, as date and time are linked).
                 // Note also that the validation logic prevents us from clearing the start date
                 // (start date is required by the back end).
-                cacheModel.set(fieldName, null, {validate: true});
+                cachethis.clearValidationErrors();
+                cachethis.setAndValidate(fieldName, null);
             }
         };
 
@@ -142,14 +143,13 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
         default: // Everything else is handled by datepickers and CodeMirror.
             break;
         }
-        var self = this;
         this.showNotificationBar(this.save_message,
                                  _.bind(this.saveView, this),
                                  _.bind(this.revertView, this));
     },
 
     removeSyllabus: function() {
-        if (this.model.has('syllabus'))	this.model.set({'syllabus': null});
+        if (this.model.has('syllabus'))	this.setAndValidate('syllabus', null);
     },
 
     assetSyllabus : function() {
@@ -184,7 +184,7 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
                     cachethis.clearValidationErrors();
                     var newVal = mirror.getValue();
                     if (cachethis.model.get(field) != newVal) {
-                        cachethis.model.set(field, newVal);
+                        cachethis.setAndValidate(field, newVal);
                         cachethis.showNotificationBar(cachethis.save_message,
                                                       _.bind(cachethis.saveView, cachethis),
                                                       _.bind(cachethis.revertView, cachethis));
@@ -209,6 +209,16 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
                        });
             },
             reset: true});
+    },
+    setAndValidate: function(attr, value) {
+        // If we call model.set() with {validate: true}, model fields
+        // will not be set if validation fails. This puts the UI and
+        // the model in an inconsistent state, and causes us to not
+        // see the right validation errors the next time validate() is
+        // called on the model. So we set *without* validating, then
+        // call validate ourselves.
+        this.model.set(attr, value);
+        this.model.isValid();
     }
 });
 

cms/static/js/views/settings/settings_grading_view.js

@@ -38,6 +38,7 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
         }
         );
         this.listenTo(this.model, 'invalid', this.handleValidationError);
+        this.listenTo(this.model, 'change', this.showNotificationBar);
         this.model.get('graders').on('reset', this.render, this);
         this.model.get('graders').on('add', this.render, this);
         this.selectorToField = _.invert(this.fieldToSelectorMap);
@@ -53,14 +54,18 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
         // Undo the double invocation error. At some point, fix the double invocation
         $(gradelist).empty();
         var gradeCollection = this.model.get('graders');
-        // We need to bind the 'remove' event here (rather than in
+        // We need to bind these events here (rather than in
         // initialize), or else we can only press the delete button
         // once due to the graders collection changing when we cancel
         // our changes.
-        gradeCollection.on('remove', function() {
+        _.each(['change', 'remove', 'add'],
+               function (event) {
+                   gradeCollection.on(event, function() {
                        this.showNotificationBar();
                        this.render();
                    }, this);
+               },
+               this);
         gradeCollection.each(function(gradeModel) {
             $(gradelist).append(self.template({model : gradeModel }));
             var newEle = gradelist.children().last();
@@ -69,9 +74,6 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
             // Listen in order to rerender when the 'cancel' button is
             // pressed
             self.listenTo(newView, 'revert', _.bind(self.render, self));
-            self.listenTo(gradeModel, 'change', function() {
-                self.showNotificationBar();
-            });
         });
 
         // render the grade cutoffs
@@ -89,7 +91,6 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
     addAssignmentType : function(e) {
         e.preventDefault();
         this.model.get('graders').push({});
-        this.showNotificationBar();
     },
     fieldToSelectorMap : {
         'grace_period' : 'course-grading-graceperiod'
@@ -99,7 +100,6 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
         self.clearValidationErrors();
         var newVal = self.model.dateToGracePeriod($(event.currentTarget).timepicker('getTime'));
         self.model.set('grace_period', newVal, {validate: true});
-        self.showNotificationBar();
     },
     updateModel : function(event) {
         if (!this.selectorToField[event.currentTarget.id]) return;
@@ -112,7 +112,6 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
             this.setField(event);
         break;
         }
-        this.showNotificationBar();
     },
 
     // Grade sliders attributes and methods
@@ -238,7 +237,6 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
                 },
                 {}),
                 {validate: true});
-        this.showNotificationBar();
     },
 
     addNewGrade: function(e) {
@@ -333,7 +331,8 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
                 self.render();
                 self.renderCutoffBar();
             },
-            reset: true});
+            reset: true,
+            silent: true});
     },
     showNotificationBar: function() {
         // We always call showNotificationBar with the same args, just

cms/static/js/views/validating_view.js

@@ -45,7 +45,8 @@ CMS.Views.ValidatingView = Backbone.View.extend({
         this.clearValidationErrors();
         var field = this.selectorToField[event.currentTarget.id];
         var newVal = $(event.currentTarget).val();
-        this.model.set(field, newVal, {validate: true});
+        this.model.set(field, newVal);
+        this.model.isValid();
         return newVal;
     },
     // these should perhaps go into a superclass but lack of event hash inheritance demotivates me
@@ -68,9 +69,18 @@ CMS.Views.ValidatingView = Backbone.View.extend({
     },
 
     showNotificationBar: function(message, primaryClick, secondaryClick) {
+        // Show a notification with message. primaryClick is called on
+        // pressing the save button, and secondaryClick (if it's
+        // passed, which it may not be) will be called on
+        // cancel. Takes care of hiding the notification bar at the
+        // appropriate times.
         if(this.notificationBarShowing) {
             return;
         }
+        // If we've already saved something, hide the alert.
+        if(this.saved) {
+            this.saved.hide();
+        }
         var self = this;
         this.confirmation = new CMS.Views.Notification.Warning({
             title: gettext("You've made some changes"),
@@ -93,10 +103,6 @@ CMS.Views.ValidatingView = Backbone.View.extend({
                             secondaryClick();
                         }
                         self.model.clear({silent : true});
-                        /*self.model.fetch({
-                            success : function() { self.render(); },
-                            reset: true
-                        });*/
                         self.confirmation.hide();
                         self.notificationBarShowing = false;
                     }
@@ -114,13 +120,23 @@ CMS.Views.ValidatingView = Backbone.View.extend({
             closeIcon: false
         });
         this.saved.show();
+        $.smoothScroll({
+            offset: 0,
+            easing: 'swing',
+            speed: 1000
+        });
     },
 
     saveView: function() {
         var self = this;
-        this.model.save({},
-                        {success: function() {
+        this.model.save(
+            {},
+            {
+                success: function() {
                     self.showSavedBar();
-                        }});
+                },
+                silent: true
+            }
+        );
     }
 });