From 2b1e9ebc4ed0a183b3788ec4fb4d8a4fb4496ea9 Mon Sep 17 00:00:00 2001 From: Shahar Talmi Date: Sat, 14 Jun 2014 16:04:37 +0300 Subject: [PATCH] fix(ngModel): test & update correct model when running $validate If `$validate` is invoked when the model is already invalid, `$validate` should pass `$$invalidModelValue` to the validators, not `$modelValue`. More over, if the model has become valid, the previously invalid model should be assigned to `$modelValue`. Lastly, in case the model has become invalid, the previously valid model should be assigned to `$$invalidModelValue`. Closes #7836 --- src/ng/directive/input.js | 39 ++++++++------ test/ng/directive/inputSpec.js | 94 ++++++++++++++++++++++++++++------ 2 files changed, 102 insertions(+), 31 deletions(-) diff --git a/src/ng/directive/input.js b/src/ng/directive/input.js index 9ee879dc1890..acf5da543162 100644 --- a/src/ng/directive/input.js +++ b/src/ng/directive/input.js @@ -1770,13 +1770,24 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ * Runs each of the registered validations set on the $validators object. */ this.$validate = function() { - this.$$runValidators(ctrl.$modelValue, ctrl.$viewValue); + // ignore $validate before model initialized + if (ctrl.$modelValue !== ctrl.$modelValue) { + return; + } + + var prev = ctrl.$modelValue; + ctrl.$$runValidators(ctrl.$$invalidModelValue || ctrl.$modelValue, ctrl.$viewValue); + if (prev !== ctrl.$modelValue) { + ctrl.$$writeModelToScope(); + } }; this.$$runValidators = function(modelValue, viewValue) { forEach(ctrl.$validators, function(fn, name) { ctrl.$setValidity(name, fn(modelValue, viewValue)); }); + ctrl.$modelValue = ctrl.$valid ? modelValue : undefined; + ctrl.$$invalidModelValue = ctrl.$valid ? undefined : modelValue; }; /** @@ -1815,22 +1826,22 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ if (ctrl.$modelValue !== modelValue && (isUndefined(ctrl.$$invalidModelValue) || ctrl.$$invalidModelValue != modelValue)) { - ctrl.$$runValidators(modelValue, viewValue); - ctrl.$modelValue = ctrl.$valid ? modelValue : undefined; - ctrl.$$invalidModelValue = ctrl.$valid ? undefined : modelValue; - - ngModelSet($scope, ctrl.$modelValue); - forEach(ctrl.$viewChangeListeners, function(listener) { - try { - listener(); - } catch(e) { - $exceptionHandler(e); - } - }); + ctrl.$$writeModelToScope(); } }; + this.$$writeModelToScope = function() { + ngModelSet($scope, ctrl.$modelValue); + forEach(ctrl.$viewChangeListeners, function(listener) { + try { + listener(); + } catch(e) { + $exceptionHandler(e); + } + }); + }; + /** * @ngdoc method * @name ngModel.NgModelController#$setViewValue @@ -1909,8 +1920,6 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$ } ctrl.$$runValidators(modelValue, viewValue); - ctrl.$modelValue = ctrl.$valid ? modelValue : undefined; - ctrl.$$invalidModelValue = ctrl.$valid ? undefined : modelValue; if (ctrl.$viewValue !== viewValue) { ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue; diff --git a/test/ng/directive/inputSpec.js b/test/ng/directive/inputSpec.js index c4acdc05a679..329079ffb622 100644 --- a/test/ng/directive/inputSpec.js +++ b/test/ng/directive/inputSpec.js @@ -294,27 +294,13 @@ describe('NgModelController', function() { }; ctrl.$modelValue = 'test'; + ctrl.$$invalidModelValue = undefined; ctrl.$validate(); expect(ctrl.$valid).toBe(false); ctrl.$modelValue = 'TEST'; - ctrl.$validate(); - - expect(ctrl.$valid).toBe(true); - }); - - it('should perform validations when $validate() is called', function() { - ctrl.$validators.uppercase = function(value) { - return (/^[A-Z]+$/).test(value); - }; - - ctrl.$modelValue = 'test'; - ctrl.$validate(); - - expect(ctrl.$valid).toBe(false); - - ctrl.$modelValue = 'TEST'; + ctrl.$$invalidModelValue = undefined; ctrl.$validate(); expect(ctrl.$valid).toBe(true); @@ -403,6 +389,7 @@ describe('NgModelController', function() { }; }; + ctrl.$modelValue = undefined; ctrl.$validators.a = curry(true); ctrl.$validators.b = curry(true); ctrl.$validators.c = curry(false); @@ -423,6 +410,7 @@ describe('NgModelController', function() { }; }; + ctrl.$modelValue = undefined; ctrl.$validators.unique = curry(false); ctrl.$validators.tooLong = curry(false); ctrl.$validators.notNumeric = curry(true); @@ -1489,6 +1477,80 @@ describe('input', function() { expect(inputElm).toBeValid(); expect(scope.form.input.$error.maxlength).not.toBe(true); }); + + it('should assign the correct model after an observed validator became valid', function() { + compileInput(''); + + scope.$apply(function() { + scope.max = 1; + }); + changeInputValueTo('12345'); + expect(scope.value).toBeUndefined(); + + scope.$apply(function() { + scope.max = 6; + }); + expect(scope.value).toBe('12345'); + }); + + it('should assign the correct model after an observed validator became invalid', function() { + compileInput(''); + + scope.$apply(function() { + scope.max = 6; + }); + changeInputValueTo('12345'); + expect(scope.value).toBe('12345'); + + scope.$apply(function() { + scope.max = 1; + }); + expect(scope.value).toBeUndefined(); + }); + + it('should leave the value as invalid if observed maxlength changed, but is still invalid', function() { + compileInput(''); + scope.$apply(function() { + scope.max = 1; + }); + + changeInputValueTo('12345'); + expect(inputElm).toBeInvalid(); + expect(scope.form.input.$error.maxlength).toBe(true); + expect(scope.value).toBeUndefined(); + + scope.$apply(function() { + scope.max = 3; + }); + + expect(inputElm).toBeInvalid(); + expect(scope.form.input.$error.maxlength).toBe(true); + expect(scope.value).toBeUndefined(); + }); + + it('should not notify if observed maxlength changed, but is still invalid', function() { + compileInput(''); + + scope.$apply(function() { + scope.max = 1; + }); + changeInputValueTo('12345'); + + scope.ngChangeSpy = jasmine.createSpy(); + scope.$apply(function() { + scope.max = 3; + }); + + expect(scope.ngChangeSpy).not.toHaveBeenCalled(); + }); + + it('should leave the model untouched when validating before model initialization', function() { + scope.value = '12345'; + compileInput(''); + expect(scope.value).toBe('12345'); + }); + });