From 973dd33d633c883b25d2a330394463a3b3ac53d4 Mon Sep 17 00:00:00 2001
From: Clark Wang <clark.wangs@gmail.com>
Date: Wed, 19 Nov 2014 21:50:08 +0800
Subject: [PATCH] fix skipping async validator will always fail if condition is
 un-fulfilled

If the validator configured with `{async:true}` option and `if/unless`
condition, validator should be skipped when the condition is un-fulfilled,
so the validator should be pass.

But currently, when skipping the validator, it calls `done(true)` which
accepts a `fail` flag as a param, this will fail the entire validation.

Signed-off-by: Clark Wang <clark.wangs@gmail.com>
---
 lib/validations.js       |  2 +-
 test/validations.test.js | 94 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/lib/validations.js b/lib/validations.js
index 3d2e0849..ee371457 100644
--- a/lib/validations.js
+++ b/lib/validations.js
@@ -505,7 +505,7 @@ function validationFailed(inst, attr, conf, cb) {
   // that can be specified in conf
   if (skipValidation(inst, conf, 'if') 
     || skipValidation(inst, conf, 'unless')) {
-    if (cb) cb(true);
+    if (cb) cb(false);
     return false;
   }
 
diff --git a/test/validations.test.js b/test/validations.test.js
index 4c83e682..11833e10 100644
--- a/test/validations.test.js
+++ b/test/validations.test.js
@@ -52,16 +52,106 @@ describe('validations', function () {
 
     describe('skipping', function () {
 
-      it('should allow to skip using if: attribute', function () {
+      it('should NOT skip when `if` is fulfilled', function () {
         User.validatesPresenceOf('pendingPeriod', {if: 'createdByAdmin'});
         var user = new User;
         user.createdByAdmin = true;
         user.isValid().should.be.false;
         user.errors.pendingPeriod.should.eql(['can\'t be blank']);
-        user.pendingPeriod = 1
+        user.pendingPeriod = 1;
         user.isValid().should.be.true;
       });
 
+      it('should skip when `if` is NOT fulfilled', function () {
+        User.validatesPresenceOf('pendingPeriod', {if: 'createdByAdmin'});
+        var user = new User;
+        user.createdByAdmin = false;
+        user.isValid().should.be.true;
+        user.errors.should.be.false;
+        user.pendingPeriod = 1;
+        user.isValid().should.be.true;
+      });
+
+      it('should NOT skip when `unless` is fulfilled', function () {
+        User.validatesPresenceOf('pendingPeriod', {unless: 'createdByAdmin'});
+        var user = new User;
+        user.createdByAdmin = false;
+        user.isValid().should.be.false;
+        user.errors.pendingPeriod.should.eql(['can\'t be blank']);
+        user.pendingPeriod = 1;
+        user.isValid().should.be.true;
+      });
+
+      it('should skip when `unless` is NOT fulfilled', function () {
+        User.validatesPresenceOf('pendingPeriod', {unless: 'createdByAdmin'});
+        var user = new User;
+        user.createdByAdmin = true;
+        user.isValid().should.be.true;
+        user.errors.should.be.false;
+        user.pendingPeriod = 1;
+        user.isValid().should.be.true;
+      });
+
+    });
+
+    describe('skipping in async validation', function () {
+
+      it('should skip when `if` is NOT fulfilled', function (done) {
+        User.validateAsync('pendingPeriod', function (err, done) {
+          if (!this.pendingPeriod) err();
+          done();
+        }, {if: 'createdByAdmin', code: 'presence', message: 'can\'t be blank'});
+        var user = new User;
+        user.createdByAdmin = false;
+        user.isValid(function (valid) {
+          valid.should.be.true;
+          user.errors.should.be.false;
+          done();
+        });
+      });
+
+      it('should NOT skip when `if` is fulfilled', function (done) {
+        User.validateAsync('pendingPeriod', function (err, done) {
+          if (!this.pendingPeriod) err();
+          done();
+        }, {if: 'createdByAdmin', code: 'presence', message: 'can\'t be blank'});
+        var user = new User;
+        user.createdByAdmin = true;
+        user.isValid(function (valid) {
+          valid.should.be.false;
+          user.errors.pendingPeriod.should.eql(['can\'t be blank']);
+          done();
+        });
+      });
+
+      it('should skip when `unless` is NOT fulfilled', function (done) {
+        User.validateAsync('pendingPeriod', function (err, done) {
+          if (!this.pendingPeriod) err();
+          done();
+        }, {unless: 'createdByAdmin', code: 'presence', message: 'can\'t be blank'});
+        var user = new User;
+        user.createdByAdmin = true;
+        user.isValid(function (valid) {
+          valid.should.be.true;
+          user.errors.should.be.false;
+          done();
+        });
+      });
+
+      it('should NOT skip when `unless` is fulfilled', function (done) {
+        User.validateAsync('pendingPeriod', function (err, done) {
+          if (!this.pendingPeriod) err();
+          done();
+        }, {unless: 'createdByAdmin', code: 'presence', message: 'can\'t be blank'});
+        var user = new User;
+        user.createdByAdmin = false;
+        user.isValid(function (valid) {
+          valid.should.be.false;
+          user.errors.pendingPeriod.should.eql(['can\'t be blank']);
+          done();
+        });
+      });
+
     });
 
     describe('lifecycle', function () {