z0mt3c / node-restify-validation

Validation for REST Services built with node-restify in node.js
MIT License
91 stars 49 forks source link

Default validation error message is undefined #41

Closed alexsk closed 8 years ago

alexsk commented 8 years ago

File lib/validation.js line 140:

var message = validator.msg || validatorCodes.messages[validator.name] || validatorCodes.messages._default;

but validatorCodes.messages._default is not defined in lib/validatorCodes.js

gchauvet commented 8 years ago

Can you provide a UT that match this case ?

alexsk commented 8 years ago

There is a defined validatorCodes.codes._default, but undefined validatorCodes.messages._default. And it's not clear for me about message field existence. It should be either optional (as we have if (message)) than it's better to drop default value from the definition:

var message = validator.msg || validatorCodes.messages[validator.name];

or it may be mandatory with some default message text, like Invalid value defined as validatorCodes.messages._default. I'd prefer the last case.

Unit test that may cover this case should contain validator without specified message via msg field and doesn't have default text in validatorCodes.js. It may be either custom validator or something from node-validator library. Let's pick up isISBN for example:

it('node-validator error messages', function() {
  var validationReq = { params: {
    ip:    '123',
    isbn:  '123',
    isbn2: '123'
  } };
  var validationModel = { resources: {
    ip:    { isRequired: true, isIP: true },
    isbn:  { isRequired: true, isISBN: true, msg: 'Invalid ISBN' },
    isbn2: { isRequired: true, isISBN: true },
  } };
  var options = { errorsAsArray: true };

  var errors = index.validation.process(validationModel, validationReq, options);
  errors.length.should.equal(3);
  should.exist(errors[0].message);
  should.equal(errors[0].message, 'Invalid IP');   // OK

  should.exist(errors[1].message);
  should.equal(errors[1].message, 'Invalid ISBN'); // OK: msg specified

  should.exist(errors[2].message);                 // FAILS: no default message
  should.equal(errors[2].message, 'Invalid value');
});
gchauvet commented 8 years ago

Done. I will create a release version ASAP.

alexsk commented 8 years ago

Thank you for a quick assistance and a new version.