From 3b3a48b609b7791857f4e56d99c3e46e3dedd4cb Mon Sep 17 00:00:00 2001
From: Oli Weighell <oliweighell.work@gmail.com>
Date: Thu, 16 Jun 2016 09:37:50 +0100
Subject: [PATCH] Fix to prevent an options validator being added on every step
 submission

This is an initial fix for issue #42. For 'options' fields the form controller adds a validator to check that the chosen option is one of the defined set of options for the field. Rather than adding this validator on each form submission, this change adds it once on start up.

- Moves the equality validator on 'options' outside of the function return
- Unit test added for the updated code, and others cleaned up slightly.
---
 lib/validation/index.js | 36 +++++++++++++++++++-----------------
 test/spec/spec.form.js  | 24 +++++++++++++++++++++---
 2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/lib/validation/index.js b/lib/validation/index.js
index d6a6bad..b848fef 100644
--- a/lib/validation/index.js
+++ b/lib/validation/index.js
@@ -30,6 +30,24 @@ function applyValidator(validator, value, key) {
 }
 
 function validator(fields) {
+
+    _.each(fields, function (field, key) {
+        fields[key].validate = fields[key].validate || [];
+
+        if (typeof fields[key].validate === 'string') {
+            fields[key].validate = [fields[key].validate];
+        }
+
+        if (fields[key].options) {
+            fields[key].validate.push({
+                type: 'equal',
+                arguments: _.map(fields[key].options, function (o) {
+                    return typeof o === 'string' ? o : o.value;
+                })
+            });
+        }
+    });
+
     return function (key, value, values, emptyValue) {
         emptyValue = emptyValue === undefined ? '' : emptyValue;
 
@@ -50,25 +68,9 @@ function validator(fields) {
         }
 
         if (fields[key]) {
-            fields[key].validate = fields[key].validate || [];
             if (shouldValidate()) {
-                var f = fields[key].validate;
-
-                if (typeof f === 'string') {
-                    f = [f];
-                }
-
-                if (fields[key].options) {
-                    f.push({
-                        type: 'equal',
-                        arguments: _.map(fields[key].options, function (o) {
-                            return typeof o === 'string' ? o : o.value;
-                        })
-                    });
-                }
-
                 debug('Applying validation on field %s with %s', key, value);
-                return _.reduce(f, function (err, validator) {
+                return _.reduce(fields[key].validate, function (err, validator) {
                     return err || applyValidator(validator, value, key);
                 }, null);
             } else {
diff --git a/test/spec/spec.form.js b/test/spec/spec.form.js
index 8d91a18..4d3b0c3 100644
--- a/test/spec/spec.form.js
+++ b/test/spec/spec.form.js
@@ -21,13 +21,20 @@ describe('Form Controller', function () {
         form.should.be.an.instanceOf(EventEmitter);
     });
 
-    it('throws if both template is undefined', function () {
+    it('throws if template is undefined', function () {
         var fn = function () {
             return new Form({});
         };
         fn.should.throw();
     });
 
+    it('throws if options are undefined', function () {
+        var fn = function () {
+            return new Form();
+        };
+        fn.should.throw();
+    });
+
     it('has `get` and `post` methods', function () {
         var form = new Form({ template: 'index' });
         form.get.should.be.a('function');
@@ -303,7 +310,7 @@ describe('Form Controller', function () {
             cb.should.have.been.calledWithExactly(new Error('Undefined validator:unknown'));
         });
 
-        it('ignores an unkown formatter', function () {
+        it('ignores an unknown formatter', function () {
             var form = new Form({
                 template: 'index',
                 fields: {
@@ -332,7 +339,7 @@ describe('Form Controller', function () {
             form.setErrors.should.have.been.calledWithExactly(null, req, res);
         });
 
-        it('call callback with error if _process fails', function () {
+        it('calls callback with error if _process fails', function () {
             var cb = sinon.stub();
             sinon.stub(form, '_process').yields('error');
             form.post(req, res, cb);
@@ -383,6 +390,17 @@ describe('Form Controller', function () {
             validators.equal.should.have.been.calledWith('number', 'one', 'two', 'three');
         });
 
+        it('does not keep adding equality validators if one already exists', function () {
+            req.body = {
+                options: 'number'
+            };
+            form.post(req, res, cb);
+            validators.equal.should.have.been.calledOnce;
+            form.post(req, res, cb);
+            validators.equal.should.have.been.calledTwice;
+            form.options.fields['options'].validate.length.should.equal(1);
+        });
+
         it('calls out to form.validate', function () {
             form.post(req, res, cb);
             form.validate.should.have.been.calledWith(req, res);