Merge pull request #293 from edx/clintonb/product-validation

Added validation for all nested products on the course model

ecommerce/static/js/collections/product_collection.js

 define([
-        'backbone'
+        'backbone',
+        'utils/utils'
     ],
-    function (Backbone) {
+    function (Backbone,
+              Utils) {
         'use strict';
 
-        return Backbone.Collection.extend({});
+        return Backbone.Collection.extend({
+            /**
+             * Validates the collection by iterating over the nested models.
+             *
+             * @return {Boolean} Boolean indicating if this Collection is valid.
+             */
+            isValid: function () {
+                return Utils.areModelsValid(this.models);
+            }
+        });
     }
 );

ecommerce/static/js/common.js

 require([
         'jquery',
         'backbone',
+        'backbone.validation',
         'bootstrap',
         'bootstrap_accessibility',
         'underscore'
@@ -12,5 +13,14 @@ require([
             // Activate all pre-rendered tooltips.
             $('[data-toggle="tooltip"]').tooltip();
         });
+
+        // NOTE (CCB): Even if validation fails, force the model to be updated. This will ensure calls
+        // to model.isValid(true) return false when we validate models before saving. Without forceUpdate,
+        // our models would always be valid, and we'd have to add additional code to check the form fields
+        // before saving.
+        Backbone.Validation.configure({
+            forceUpdate: true,
+            labelFormatter: 'label'
+        });
     }
 );

ecommerce/static/js/config.js

@@ -20,6 +20,7 @@ require.config({
         'requirejs': 'bower_components/requirejs/require',
         'routers': 'js/routers',
         'templates': 'templates',
+        'test': 'js/test',
         'text': 'bower_components/text/text',
         'underscore': 'bower_components/underscore/underscore',
         'underscore.string': 'bower_components/underscore.string/dist/underscore.string',

ecommerce/static/js/models/course_model.js

@@ -30,10 +30,6 @@ define([
               Utils) {
         'use strict';
 
-        Backbone.Validation.configure({
-            labelFormatter: 'label'
-        });
-
         _.extend(Backbone.Model.prototype, Backbone.Validation.mixin);
 
         _.extend(Backbone.Validation.patterns, {
@@ -83,6 +79,11 @@ define([
                     if (invalid) {
                         return gettext('The verification deadline must occur AFTER the upgrade deadline.');
                     }
+                },
+                products: function (value) {
+                    if (!value.isValid()) {
+                        return gettext('Product validation failed.');
+                    }
                 }
             },
 

ecommerce/static/js/test/spec-utils.js

+define([
+        'backbone'
+    ],
+    function (Backbone) {
+        'use strict';
+
+        return {
+            /**
+             * Returns a Backbone model that can be used to test validation.
+             *
+             * @param {Boolean} valid - Indicates if the model should pass or fail validation.
+             */
+            getModelForValidation: function (valid) {
+                return Backbone.Model.extend({
+                    isValid: function () {
+                        return valid;
+                    },
+
+                    validate: function () {
+                        if (valid) {
+                            return {};
+                        }
+
+                        return {error: 'Test model: validate always fails.'};
+                    }
+                });
+            }
+        };
+    }
+);

ecommerce/static/js/test/specs/collections/product_collection_spec.js

+define([
+        'collections/product_collection',
+        'test/spec-utils'
+    ],
+    function (ProductCollection,
+              SpecUtils) {
+        'use strict';
+        var collection;
+
+        describe('Product collection', function () {
+            beforeEach(function () {
+                collection = new ProductCollection();
+            });
+
+            describe('isValid', function () {
+                it('should return true if the collection is empty', function () {
+                    collection.reset();
+                    expect(collection.isValid()).toEqual(true);
+                });
+
+                it('should return true if all models are valid', function () {
+                    var ModelClass = SpecUtils.getModelForValidation(true);
+                    collection.reset([new ModelClass(), new ModelClass()]);
+
+                    expect(collection.isValid()).toEqual(true);
+                });
+
+                it('should return false if any of the models is NOT valid', function () {
+                    var ModelClass = SpecUtils.getModelForValidation(false);
+                    collection.reset([new ModelClass(), new ModelClass()]);
+
+                    expect(collection.isValid()).toEqual(false);
+
+                    // A mixture of validation statuses should always return false.
+                    collection.add(new (SpecUtils.getModelForValidation(true))());
+                    expect(collection.isValid()).toEqual(false);
+                });
+            });
+        });
+    }
+);

ecommerce/static/js/test/specs/models/course_model_spec.js

@@ -94,16 +94,23 @@ define([
                 ]
             };
 
-        beforeEach(function () {
-            model = Course.findOrCreate(data, {parse: true});
-        });
-
         describe('Course model', function () {
+            beforeEach(function () {
+                model = Course.findOrCreate(data, {parse: true});
+
+                // Remove the parent products
+                model.removeParentProducts();
+            });
+
             describe('removeParentProducts', function () {
                 it('should remove all parent products from the products collection', function () {
-                    var products = model.get('products');
+                    var products;
+
+                    // Re-initialize the model since the beforeEach removes the parents
+                    model = Course.findOrCreate(data, {parse: true});
 
                     // Sanity check to ensure the products were properly parsed
+                    products = model.get('products');
                     expect(products.length).toEqual(3);
 
                     // Remove the parent products
@@ -167,6 +174,7 @@ define([
                     spyOn($, 'ajax');
                     $.cookie('ecommerce_csrftoken', cookie);
 
+                    expect(model.validate()).toBeFalsy();
                     model.save();
 
                     // $.ajax should have been called
@@ -239,6 +247,19 @@ define([
                     expect(model.isValid(true)).toBeFalsy();
                 });
             });
+
+            describe('products validation', function () {
+                it('should return an error message if any product is invalid', function () {
+                    var msg = 'Product validation failed.',
+                        products = model.get('products');
+
+                    // Add an invalid product
+                    products.push(new ProfessionalSeat({price: null}));
+
+                    expect(model.validate().products).toEqual(msg);
+                    expect(model.isValid(true)).toBeFalsy();
+                });
+            });
         });
     }
 );

ecommerce/static/js/test/specs/utils/utils_spec.js

 define([
+        'backbone',
+        'test/spec-utils',
         'utils/utils'
     ],
-    function (Utils) {
+    function (Backbone,
+              SpecUtils,
+              Utils) {
         'use strict';
 
         describe('Utils', function () {
@@ -30,6 +34,26 @@ define([
                     expect(Utils.restoreTimezone(dt)).toEqual(dt + '+00:00');
                 });
             });
+
+            describe('areModelsValid', function () {
+                it('should return true if all models are valid', function () {
+                    var ModelClass = SpecUtils.getModelForValidation(true),
+                        models = [new ModelClass(), new ModelClass()];
+
+                    expect(Utils.areModelsValid(models)).toEqual(true);
+                });
+
+                it('should return false if any of the models is NOT valid', function () {
+                    var ModelClass = SpecUtils.getModelForValidation(false),
+                        models = [new ModelClass(), new ModelClass()];
+
+                    expect(Utils.areModelsValid(models)).toEqual(false);
+
+                    // A mixture of validation statuses should always return false.
+                    models.push(new (SpecUtils.getModelForValidation(true))());
+                    expect(Utils.areModelsValid(models)).toEqual(false);
+                });
+            });
         });
     }
 );

ecommerce/static/js/utils/utils.js

@@ -67,6 +67,21 @@ define([
                     datetime = moment.utc(datetime + 'Z').format();
                 }
                 return datetime;
+            },
+
+            /**
+             * Indicates if all models in the array are valid.
+             *
+             * Calls isValid() on every model in the array.
+             *
+             * @param {Backbone.Model[]} models
+             * @returns {Boolean} indicates if ALL models are valid.
+             */
+            areModelsValid: function (models) {
+                return _.every(models, function (model) {
+                    return model.isValid(true);
+                });
             }
         };
-    });
+    }
+);

ecommerce/static/js/views/course_form_view.js

@@ -144,7 +144,7 @@ define([
                 }
 
                 // Enable validation
-                Backbone.Validation.bind(this, {forceUpdate: !this.editing});
+                Backbone.Validation.bind(this);
             },
 
             remove: function () {
@@ -389,6 +389,12 @@ define([
 
                         if (response.responseJSON && response.responseJSON.error) {
                             message = response.responseJSON.error;
+
+                            // Log the error to the console for debugging purposes
+                            console.error(message);
+                        } else {
+                            // Log the error to the console for debugging purposes
+                            console.error(response.responseText);
                         }
 
                         self.clearAlerts();