Skip to content

Commit

Permalink
Fix to prevent an options validator being added on every step submission
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Oli Weighell committed Jun 16, 2016
1 parent d60cde7 commit 3b3a48b
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 20 deletions.
36 changes: 19 additions & 17 deletions lib/validation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down
24 changes: 21 additions & 3 deletions test/spec/spec.form.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 3b3a48b

Please sign in to comment.