Skip to content

Commit 662458d

Browse files
committed
Fix validateNumericality, nullCheck & add tests
validateNumericality didn't test if attributes value is a number only if it's type is number. Further nullCheck had a wrong testing order. It first checked if value is null, later if blank. Also null check only used two equals, not three. We don't use blank() anymore, testing if variable is undefined should be fine too. Added tests covering validateNumericality.
1 parent ec204df commit 662458d

File tree

2 files changed

+76
-8
lines changed

2 files changed

+76
-8
lines changed

lib/validations.js

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ function validateLength(attr, conf, err, options) {
287287
function validateNumericality(attr, conf, err, options) {
288288
if (nullCheck.call(this, attr, conf, err)) return;
289289

290-
if (typeof this[attr] !== 'number') {
290+
if (typeof this[attr] !== 'number' || isNaN(this[attr])) {
291291
return err('number');
292292
}
293293
if (conf.int && this[attr] !== Math.round(this[attr])) {
@@ -633,16 +633,26 @@ var defaultMessages = {
633633
uniqueness: 'is not unique',
634634
};
635635

636+
/**
637+
* Checks if attribute is undefined or null. Calls err function with 'blank' or 'null'.
638+
* See defaultMessages. You can affect this behaviour with conf.allowBlank and conf.allowNull.
639+
* @param {String} attr Property name of attribute
640+
* @param {Object} conf conf object for validator
641+
* @param {Function} err
642+
* @return {Boolean} returns true if attribute is null or blank
643+
*/
636644
function nullCheck(attr, conf, err) {
637-
if (this[attr] == null) {
638-
if (!conf.allowNull) {
639-
err('null');
645+
// First determine if attribute is defined
646+
if (typeof this[attr] === 'undefined') {
647+
if (!conf.allowBlank) {
648+
err('blank');
640649
}
641650
return true;
642651
} else {
643-
if (blank(this[attr])) {
644-
if (!conf.allowBlank) {
645-
err('blank');
652+
// Now check if attribute is null
653+
if (this[attr] === null) {
654+
if (!conf.allowNull) {
655+
err('null');
646656
}
647657
return true;
648658
}

test/validations.test.js

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,65 @@ describe('validations', function() {
558558
});
559559

560560
describe('numericality', function() {
561-
it('should validate numericality');
561+
it('passes when given numeric values', function() {
562+
User.validatesNumericalityOf('age');
563+
var user = new User({age: 10});
564+
user.isValid().should.be.true();
565+
});
566+
567+
it('fails when given non-numeric values', function() {
568+
User.validatesNumericalityOf('age');
569+
var user = new User({age: 'notanumber'});
570+
user.isValid().should.be.false();
571+
user.errors.should.eql({age: ['is not a number']});
572+
});
573+
574+
it('fails when given undefined values', function() {
575+
User.validatesNumericalityOf('age');
576+
var u = new User({});
577+
u.isValid().should.be.false();
578+
u.errors.should.eql({age: ['is blank']});
579+
});
580+
581+
it('skips undefined values when allowBlank option is true', function() {
582+
User.validatesNumericalityOf('age', {allowBlank: true});
583+
var user = new User({});
584+
user.isValid().should.be.true();
585+
});
586+
587+
it('fails when given non-numeric values when allowBlank option is true', function() {
588+
User.validatesNumericalityOf('age', {allowBlank: true});
589+
var user = new User({age: 'test'});
590+
user.isValid().should.be.false();
591+
user.errors.should.eql({age: ['is not a number']});
592+
593+
});
594+
595+
it('fails when given null values', function() {
596+
User.validatesNumericalityOf('age');
597+
var user = new User({age: null});
598+
user.isValid().should.be.false();
599+
user.errors.should.eql({age: ['is null']});
600+
});
601+
602+
it('passes when given null values when allowNull option is true', function() {
603+
User.validatesNumericalityOf('age', {allowNull: true});
604+
var user = new User({age: null});
605+
user.isValid().should.be.true();
606+
});
607+
608+
it('passes when given float values', function() {
609+
User.validatesNumericalityOf('age');
610+
var user = new User({age: 13.37});
611+
user.isValid().should.be.true();
612+
});
613+
614+
it('fails when given non-integer values when int option is true', function() {
615+
User.validatesNumericalityOf('age', {int: true});
616+
var user = new User({age: 13.37});
617+
user.isValid().should.be.false();
618+
user.errors.should.eql({age: ['is not an integer']});
619+
});
562620
});
563621

564622
describe('inclusion', function() {

0 commit comments

Comments
 (0)