Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Cleanup for length-validation, added tests

  • Loading branch information...
commit 9fe44aaa9a2dc2fa5478f941720610b3751515f9 1 parent 70b2052
mde authored
Showing with 125 additions and 31 deletions.
  1. +52 −29 lib/validators.js
  2. +73 −2 test/validators.js
81 lib/validators.js
View
@@ -76,47 +76,70 @@ var validators = {
length: function (name, val, params, rule, locale) {
var qual = rule.qualifier
, err
- , msg;
+ , msg
+ , intVal
+ , errMsg = 'validatesLength must be set to a integer ' +
+ 'or object with min/max integer properties.'
+ // Coerces a value to an integer, only if it's coercible
+ // Returns null otherwise
+ , validInteger = function (n) {
+ var intVal = Math.round(n);
+ if (intVal == n) {
+ return intVal;
+ }
+ else {
+ return null;
+ }
+ };
+
+ // If a specific length is wanted, there has to be a value
+ // in the first place
if (!val) {
- return rule.message || 'Field "' + name + '" is required.';
+ return i18n.getText('model.validatesPresent', {name: name}, locale);

It's fine to get rid of the hardcoded message, but I think we need to leave the rule.message so it takes precedence over the localized text.
return rule.message || i18n.getText('model.validatesPresent', {name: name}, locale);

Matthew Eernisse Owner
mde added a note

Good call. I kind of dithered on it -- it might be weird to get a message about lengths (even a customized one), when the problem is that the thing isn't being passed at all. But you're right, if the user is doing an override, we have to assume she knows what she's doing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}
- if(Math.round(qual)==qual){
- qual = Math.round(qual);
- }else if(typeof qual !== 'undefined' && typeof qual !== 'object'){
- throw new Error('qualifier at validatesLength needs to be a number or an object');
+
+ // Validate that there's a qualifier to check against
+ if (!qual) {
+ throw new Error(errMsg);
}
- if (typeof qual == 'number') {
+
+ // First check to see if the qualifier is itself a number
+ // If so, validate the precise length
+ if ((intVal = validInteger(qual))) {
+ qual = intVal;
if (val.length != qual) {
- //return rule.message || 'Field "' + name + '" must be ' + qual +
- // ' characters long.';
msg = rule.message || i18n.getText('model.validatesExactLength',
{name: name}, locale);
}
}
- else {
- if(Math.round(qual.min)==qual.min){
- qual.min = Math.round(qual.min);
- }else if(typeof qual.min !== 'undefined'){
- throw new Error('min property of qualifier at validatesLength needs to be a number');
+ // If the qualifier wasn't a number, check for min or max
+ // property to validate against
+ else if (qual.min || qual.max) {
+ // If there's either a min or max, make sure at least one
+ // of them is a usable number
+ if ((intVal = validInteger(qual.min))) {
+ if (val.length < intVal) {
+ msg = rule.message || i18n.getText('model.validatesMinLength',
+ {name: name, min: qual.min}, locale);
+ }
}
- if(Math.round(qual.max)==qual.max){
- qual.max = Math.round(qual.max);
- }else if(typeof qual.max !== 'undefined'){
- throw new Error('max property of qualifier at validatesLength needs to be a number');
+ else if ((intVal = validInteger(qual.max))) {
+ if (val.length > intVal) {
+ msg = rule.message || i18n.getText('model.validatesMaxLength',
+ {name: name, max: qual.max}, locale);
+ }
}
- if (typeof qual.min == 'number' && val.length < qual.min) {
- //return rule.message || 'Field "' + name + '" must be at least ' +
- // qual.min + ' characters long.';
- msg = rule.message || i18n.getText('model.validatesMinLength',
- {name: name, min: qual.min}, locale);
- }
- if (typeof qual.max == 'number' && val.length > qual.max) {
- //return rule.message || 'Field "' + name + '" may not be more than ' +
- // qual.max + ' characters long.';
- msg = rule.message || i18n.getText('model.validatesMaxLength',
- {name: name, max: qual.max}, locale);
+ // If neither min nor max provided a valid number, throw an error
+ if (!intVal) {
+ throw new Error(errMsg);
}
}
+ // The qualifier wasn't a number, and there's no min or max
+ // property work with -- throw
+ else {
+ throw new Error(errMsg);
+ }
+
return msg;
},
75 test/validators.js
View
@@ -1,4 +1,5 @@
var model = require('../lib')
+ , validators = model.validators
, assert = require('assert')
, tests;
@@ -6,7 +7,7 @@ var ModelWithValidations = function () {
this.defineProperties({
requiredPropertyAddedByDefineProperties: {type: 'string', required: true},
});
-
+
this.property('requiredPropertyAddedByProperty', 'string', { required: true});
};
@@ -17,10 +18,80 @@ tests = {
assert.ok(model.descriptionRegistry['ModelWithValidations'].properties.
requiredPropertyAddedByDefineProperties.validations.present);
}
+
, 'Required option adds a validatesPresent rule when using property': function () {
assert.ok(model.descriptionRegistry['ModelWithValidations'].properties.
requiredPropertyAddedByProperty.validations.present);
}
+
+, 'Validating exact length with incorrect length': function () {
+ var msg = validators.length('foo', '1111', null, {qualifier: 3});
+ assert.equal('[[model.validatesExactLength]]', msg);
+ }
+
+, 'Validating exact length with correct length': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: 3});
+ assert.ok(!msg);
+ }
+
+, 'Validating exact length with correct length and string qualifier': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: '3'});
+ assert.ok(!msg);
+ }
+
+, 'Validating exact length with correct length and string qualifier': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: '3'});
+ assert.ok(!msg);
+ }
+
+, 'Validating exact length with correct length and non-number string qualifier': function () {
+ assert.throws(function () {
+ var msg = validators.length('foo', '111', null, {qualifier: 'hello'});
+ });
+ }
+
+, 'Validating min length with incorrect length': function () {
+ var msg = validators.length('foo', '11', null, {qualifier: {min: 3}});
+ assert.equal('[[model.validatesMinLength]]', msg);
+ }
+
+, 'Validating min length with correct length': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: 3}});
+ assert.ok(!msg);
+ }
+
+, 'Validating min length with correct length and string qualifier': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: '3'}});
+ assert.ok(!msg);
+ }
+
+, 'Validating min length with correct length and non-number string qualifier': function () {
+ assert.throws(function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: 'hello'}});
+ });
+ }
+
+, 'Validating max length with incorrect length': function () {
+ var msg = validators.length('foo', '1111', null, {qualifier: {max: 3}});
+ assert.equal('[[model.validatesMaxLength]]', msg);
+ }
+
+, 'Validating max length with correct length': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: 3}});
+ assert.ok(!msg);
+ }
+
+, 'Validating max length with correct length and string qualifier': function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: '3'}});
+ assert.ok(!msg);
+ }
+
+, 'Validating max length with correct length and non-number string qualifier': function () {
+ assert.throws(function () {
+ var msg = validators.length('foo', '111', null, {qualifier: {min: 'hello'}});
+ });
+ }
+
};
-module.exports = tests;
+module.exports = tests;
Miguel Madero

It's fine to get rid of the hardcoded message, but I think we need to leave the rule.message so it takes precedence over the localized text.
return rule.message || i18n.getText('model.validatesPresent', {name: name}, locale);

Matthew Eernisse

Good call. I kind of dithered on it -- it might be weird to get a message about lengths (even a customized one), when the problem is that the thing isn't being passed at all. But you're right, if the user is doing an override, we have to assume she knows what she's doing.

Please sign in to comment.
Something went wrong with that request. Please try again.