Skip to content

Commit 056a317

Browse files
petebacondarwinpetebacondarwin
committed
fix(ngModel): fix issues when parserName is same as validator key
For $validate(), it is necessary to store the parseError state in the controller. Otherwise, if the parser name equals a validator key, $validate() will assume a parse error occured if the validator is invalid. Also, setting the validity for the parser now happens after setting validity for the validator key. Otherwise, the parse key is set, and then immediately afterwards the validator key is unset (because parse errors remove all other validations). Fixes angular#10698 Closes angular#10850 Closes angular#11046
1 parent 27fcca9 commit 056a317

2 files changed

Lines changed: 104 additions & 15 deletions

File tree

‎src/ng/directive/ngModel.js‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
244244
ngModelGet = parsedNgModel,
245245
ngModelSet = parsedNgModelAssign,
246246
pendingDebounce = null,
247+
parserValid,
247248
ctrl = this;
248249

249250
this.$$setOptions = function(options) {
@@ -516,16 +517,12 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
516517
// the model although neither viewValue nor the model on the scope changed
517518
var modelValue = ctrl.$$rawModelValue;
518519

519-
// Check if the there's a parse error, so we don't unset it accidentially
520-
var parserName = ctrl.$$parserName || 'parse';
521-
var parserValid = ctrl.$error[parserName] ? false : undefined;
522-
523520
var prevValid = ctrl.$valid;
524521
var prevModelValue = ctrl.$modelValue;
525522

526523
var allowInvalid = ctrl.$options && ctrl.$options.allowInvalid;
527524

528-
ctrl.$$runValidators(parserValid, modelValue, viewValue, function(allValid) {
525+
ctrl.$$runValidators(modelValue, viewValue, function(allValid) {
529526
// If there was no change in validity, don't update the model
530527
// This prevents changing an invalid modelValue to undefined
531528
if (!allowInvalid && prevValid !== allValid) {
@@ -543,12 +540,12 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
543540

544541
};
545542

546-
this.$$runValidators = function(parseValid, modelValue, viewValue, doneCallback) {
543+
this.$$runValidators = function(modelValue, viewValue, doneCallback) {
547544
currentValidationRunId++;
548545
var localValidationRunId = currentValidationRunId;
549546

550547
// check parser error
551-
if (!processParseErrors(parseValid)) {
548+
if (!processParseErrors()) {
552549
validationDone(false);
553550
return;
554551
}
@@ -558,21 +555,22 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
558555
}
559556
processAsyncValidators();
560557

561-
function processParseErrors(parseValid) {
558+
function processParseErrors() {
562559
var errorKey = ctrl.$$parserName || 'parse';
563-
if (parseValid === undefined) {
560+
if (parserValid === undefined) {
564561
setValidity(errorKey, null);
565562
} else {
566-
setValidity(errorKey, parseValid);
567-
if (!parseValid) {
563+
if (!parserValid) {
568564
forEach(ctrl.$validators, function(v, name) {
569565
setValidity(name, null);
570566
});
571567
forEach(ctrl.$asyncValidators, function(v, name) {
572568
setValidity(name, null);
573569
});
574-
return false;
575570
}
571+
// Set the parse error last, to prevent unsetting it, should a $validators key == parserName
572+
setValidity(errorKey, parserValid);
573+
return parserValid;
576574
}
577575
return true;
578576
}
@@ -667,7 +665,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
667665
this.$$parseAndValidate = function() {
668666
var viewValue = ctrl.$$lastCommittedViewValue;
669667
var modelValue = viewValue;
670-
var parserValid = isUndefined(modelValue) ? undefined : true;
668+
parserValid = isUndefined(modelValue) ? undefined : true;
671669

672670
if (parserValid) {
673671
for (var i = 0; i < ctrl.$parsers.length; i++) {
@@ -693,7 +691,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
693691

694692
// Pass the $$lastCommittedViewValue here, because the cached viewValue might be out of date.
695693
// This can happen if e.g. $setViewValue is called from inside a parser
696-
ctrl.$$runValidators(parserValid, modelValue, ctrl.$$lastCommittedViewValue, function(allValid) {
694+
ctrl.$$runValidators(modelValue, ctrl.$$lastCommittedViewValue, function(allValid) {
697695
if (!allowInvalid) {
698696
// Note: Don't check ctrl.$valid here, as we could have
699697
// external validators (e.g. calculated on the server),
@@ -814,6 +812,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
814812
// TODO(perf): why not move this to the action fn?
815813
if (modelValue !== ctrl.$modelValue) {
816814
ctrl.$modelValue = ctrl.$$rawModelValue = modelValue;
815+
parserValid = undefined;
817816

818817
var formatters = ctrl.$formatters,
819818
idx = formatters.length;
@@ -826,7 +825,7 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
826825
ctrl.$viewValue = ctrl.$$lastCommittedViewValue = viewValue;
827826
ctrl.$render();
828827

829-
ctrl.$$runValidators(undefined, modelValue, viewValue, noop);
828+
ctrl.$$runValidators(modelValue, viewValue, noop);
830829
}
831830
}
832831

‎test/ng/directive/ngModelSpec.js‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,6 +1221,96 @@ describe('ngModel', function() {
12211221
expect(ctrl.$validators.mock).toHaveBeenCalledWith('a', 'ab');
12221222
expect(ctrl.$validators.mock.calls.length).toEqual(2);
12231223
});
1224+
1225+
it('should validate correctly when $parser name equals $validator key', function() {
1226+
1227+
ctrl.$validators.parserOrValidator = function(value) {
1228+
switch (value) {
1229+
case 'allInvalid':
1230+
case 'parseValid-validatorsInvalid':
1231+
case 'stillParseValid-validatorsInvalid':
1232+
return false;
1233+
default:
1234+
return true;
1235+
}
1236+
};
1237+
1238+
ctrl.$validators.validator = function(value) {
1239+
switch (value) {
1240+
case 'allInvalid':
1241+
case 'parseValid-validatorsInvalid':
1242+
case 'stillParseValid-validatorsInvalid':
1243+
return false;
1244+
default:
1245+
return true;
1246+
}
1247+
};
1248+
1249+
ctrl.$$parserName = 'parserOrValidator';
1250+
ctrl.$parsers.push(function(value) {
1251+
switch (value) {
1252+
case 'allInvalid':
1253+
case 'stillAllInvalid':
1254+
case 'parseInvalid-validatorsValid':
1255+
case 'stillParseInvalid-validatorsValid':
1256+
return undefined;
1257+
default:
1258+
return value;
1259+
}
1260+
});
1261+
1262+
//Parser and validators are invalid
1263+
scope.$apply('value = "allInvalid"');
1264+
expect(scope.value).toBe('allInvalid');
1265+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1266+
1267+
ctrl.$validate();
1268+
expect(scope.value).toEqual('allInvalid');
1269+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1270+
1271+
ctrl.$setViewValue('stillAllInvalid');
1272+
expect(scope.value).toBeUndefined();
1273+
expect(ctrl.$error).toEqual({parserOrValidator: true});
1274+
1275+
ctrl.$validate();
1276+
expect(scope.value).toBeUndefined();
1277+
expect(ctrl.$error).toEqual({parserOrValidator: true});
1278+
1279+
//Parser is valid, validators are invalid
1280+
scope.$apply('value = "parseValid-validatorsInvalid"');
1281+
expect(scope.value).toBe('parseValid-validatorsInvalid');
1282+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1283+
1284+
ctrl.$validate();
1285+
expect(scope.value).toBe('parseValid-validatorsInvalid');
1286+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1287+
1288+
ctrl.$setViewValue('stillParseValid-validatorsInvalid');
1289+
expect(scope.value).toBeUndefined();
1290+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1291+
1292+
ctrl.$validate();
1293+
expect(scope.value).toBeUndefined();
1294+
expect(ctrl.$error).toEqual({parserOrValidator: true, validator: true});
1295+
1296+
//Parser is invalid, validators are valid
1297+
scope.$apply('value = "parseInvalid-validatorsValid"');
1298+
expect(scope.value).toBe('parseInvalid-validatorsValid');
1299+
expect(ctrl.$error).toEqual({});
1300+
1301+
ctrl.$validate();
1302+
expect(scope.value).toBe('parseInvalid-validatorsValid');
1303+
expect(ctrl.$error).toEqual({});
1304+
1305+
ctrl.$setViewValue('stillParseInvalid-validatorsValid');
1306+
expect(scope.value).toBeUndefined();
1307+
expect(ctrl.$error).toEqual({parserOrValidator: true});
1308+
1309+
ctrl.$validate();
1310+
expect(scope.value).toBeUndefined();
1311+
expect(ctrl.$error).toEqual({parserOrValidator: true});
1312+
});
1313+
12241314
});
12251315
});
12261316

0 commit comments

Comments
 (0)