fix(ngModel): don’t clear the model when an external validator failed

Calling `ctrl.$setValidity()` with a an error key that
does not belong to a validator in `ctrl.$validator` should
not result in setting the model to `undefined` on the next
input change. This bug was introduced in 1.3.0-beta.12.

Closes #8357
Fixes #8080

Author: tbosch

CHANGELOG.md

@@ -1871,14 +1871,6 @@ this limitation, use a regular expression object as the value for the expression
     //after
     $scope.exp = /abc/i;
 
-- **NgModel:** due to [f3cb2741161353f387d02725637ce4ba062a9bc0](https://github.com/angular/angular.js/commit/f3cb2741161353f387d02725637ce4ba062a9bc0),
-
-#### since 1.3.0-beta.11
-
-If the user enters a value and a parser or validator fails, the model will be set to `undefined`.
-This is the same behavior as in 1.2.x, but different to 1.3.0-beta.11, as there only invalid parsers
-would set the model to `undefined`, but invalid validators would not change the model.
-
 - **Scope:** due to [8c6a8171](https://github.com/angular/angular.js/commit/8c6a8171f9bdaa5cdabc0cc3f7d3ce10af7b434d),
   Scope#$id is now of time number rather than string. Since the
 id is primarily being used for debugging purposes this change should not affect

src/ng/directive/input.js

@@ -1906,9 +1906,11 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
 
     // check parser error
     if (!processParseErrors(parseValid)) {
+      validationDone(false);
       return;
     }
     if (!processSyncValidators()) {
+      validationDone(false);
       return;
     }
     processAsyncValidators();
@@ -1926,7 +1928,6 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
           forEach(ctrl.$asyncValidators, function(v, name) {
             setValidity(name, null);
           });
-          validationDone();
           return false;
         }
       }
@@ -1944,14 +1945,14 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
         forEach(ctrl.$asyncValidators, function(v, name) {
           setValidity(name, null);
         });
-        validationDone();
         return false;
       }
       return true;
     }
 
     function processAsyncValidators() {
       var validatorPromises = [];
+      var allValid = true;
       forEach(ctrl.$asyncValidators, function(validator, name) {
         var promise = validator(modelValue, viewValue);
         if (!isPromiseLike(promise)) {
@@ -1962,13 +1963,16 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
         validatorPromises.push(promise.then(function() {
           setValidity(name, true);
         }, function(error) {
+          allValid = false;
           setValidity(name, false);
         }));
       });
       if (!validatorPromises.length) {
-        validationDone();
+        validationDone(true);
       } else {
-        $q.all(validatorPromises).then(validationDone);
+        $q.all(validatorPromises).then(function() {
+          validationDone(allValid);
+        }, noop);
       }
     }
 
@@ -1978,10 +1982,10 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
       }
     }
 
-    function validationDone() {
+    function validationDone(allValid) {
       if (localValidationRunId === currentValidationRunId) {
 
-        doneCallback();
+        doneCallback(allValid);
       }
     }
   };
@@ -2042,9 +2046,13 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
       ctrl.$modelValue = modelValue;
       writeToModelIfNeeded();
     }
-    ctrl.$$runValidators(parserValid, modelValue, viewValue, function() {
+    ctrl.$$runValidators(parserValid, modelValue, viewValue, function(allValid) {
       if (!allowInvalid) {
-        ctrl.$modelValue = ctrl.$valid ? modelValue : undefined;
+        // Note: Don't check ctrl.$valid here, as we could have
+        // external validators (e.g. calculated on the server),
+        // that just call $setValidity and need the model value
+        // to calculate their validity.
+        ctrl.$modelValue = allValid ? modelValue : undefined;
         writeToModelIfNeeded();
       }
     });

test/ng/directive/inputSpec.js

@@ -214,6 +214,15 @@ describe('NgModelController', function() {
       expect(ctrl.$modelValue).toBeUndefined();
     });
 
+    it('should not reset the model when the view is invalid due to an external validator', function() {
+      ctrl.$setViewValue('aaaa');
+      expect(ctrl.$modelValue).toBe('aaaa');
+
+      ctrl.$setValidity('someExternalError', false);
+      ctrl.$setViewValue('bbbb');
+      expect(ctrl.$modelValue).toBe('bbbb');
+    });
+
     it('should not reset the view when the view is invalid', function() {
       // this test fails when the view changes the model and
       // then the model listener in ngModel picks up the change and
@@ -302,6 +311,13 @@ describe('NgModelController', function() {
       expect(ctrl.$error).toEqual({ high : true });
     });
 
+    it('should not remove external validators when a parser failed', function() {
+      ctrl.$parsers.push(function(v) { return undefined; });
+      ctrl.$setValidity('externalError', false);
+      ctrl.$setViewValue('someValue');
+      expect(ctrl.$error).toEqual({ externalError : true, parse: true });
+    });
+
     it('should remove all non-parse-related CSS classes from the form when a parser fails',
       inject(function($compile, $rootScope) {
 
@@ -711,7 +727,7 @@ describe('NgModelController', function() {
       expect(ctrl.$pending).toBeUndefined();
     }));
 
-    it('should clear and ignore all pending promises when the input values changes', inject(function($q) {
+    it('should clear and ignore all pending promises when the model value changes', inject(function($q) {
       ctrl.$validators.sync = function(value) {
         return true;
       };
@@ -775,6 +791,44 @@ describe('NgModelController', function() {
       expect(isObject(ctrl.$pending)).toBe(false);
     }));
 
+    it('should clear all errors from async validators if a parser fails', inject(function($q) {
+      var failParser = false;
+      ctrl.$parsers.push(function(value) {
+        return failParser ? undefined : value;
+      });
+
+      ctrl.$asyncValidators.async = function(value) {
+        return $q.reject();
+      };
+
+      ctrl.$setViewValue('x..y..z');
+      expect(ctrl.$error).toEqual({async: true});
+
+      failParser = true;
+
+      ctrl.$setViewValue('1..2..3');
+      expect(ctrl.$error).toEqual({parse: true});
+    }));
+
+    it('should clear all errors from async validators if a sync validator fails', inject(function($q) {
+      var failValidator = false;
+      ctrl.$validators.sync = function(value) {
+        return !failValidator;
+      };
+
+      ctrl.$asyncValidators.async = function(value) {
+        return $q.reject();
+      };
+
+      ctrl.$setViewValue('x..y..z');
+      expect(ctrl.$error).toEqual({async: true});
+
+      failValidator = true;
+
+      ctrl.$setViewValue('1..2..3');
+      expect(ctrl.$error).toEqual({sync: true});
+    }));
+
     it('should re-evaluate the form validity state once the asynchronous promise has been delivered',
       inject(function($compile, $rootScope, $q) {