Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

$setValidity somehow breaks model binding in 1.3.0 Beta 12 and after #8080

Closed
mpiecko opened this issue Jul 5, 2014 · 15 comments
Closed

$setValidity somehow breaks model binding in 1.3.0 Beta 12 and after #8080

mpiecko opened this issue Jul 5, 2014 · 15 comments

Comments

@mpiecko
Copy link

mpiecko commented Jul 5, 2014

Since version 1.3.0 Beta 12 my models are losing properties after manually invalidating input fields. I created a JSFiddle to demonstrate this behavior.

Type something in the field, press the button to invalidate the field, and type something in the field again. In Beta 11 my model stays up to date with the input field (see "Debug Model" below):
http://jsfiddle.net/mpiecko/q2s7Z/

Now watch the "Debug Model" if you repeat these steps with Beta 12. The "fullName" property disappears when you type something in the field, invalidate it with the button, and then type something again:
http://jsfiddle.net/mpiecko/q2s7Z/1/

I'm not sure if this is a bug or an upcoming change in handling of form errors.

Michael

@caitp
Copy link
Contributor

caitp commented Jul 5, 2014

Hi, so what's actually happening is that the fullName field is undefined (it would ordinarily be undefined in the first case too, if this validation had happened under normal circumstances).

It's not being removed from the form, it's just that toJson() won't show you keys with undefined values.

http://jsfiddle.net/QruVT/ here's an example --- if you type a name which contains a hyphen, you should see the property disappear (it becomes invalid), but if it's valid (no hyphen), the property is visible. As you can see from http://jsfiddle.net/GZ5Nt/, this works the same in beta.11.

Basically, the issue here is that validation is very much tied to the reporting of the value. I've proposed a change which would separate these things and make them less of a maze, so that it's easier to understand. But currently yeah, since values and validity are very tied, you need to do some work to make this use case work.

If you're manually setting the view value to defer validation, you might want to look into using ngModelOptions instead, which allows for this deferral.

@mpiecko
Copy link
Author

mpiecko commented Jul 5, 2014

Thanks, i think i got it. So basically an invalid field will also make its model value undefined (which then is not removed, but invisible due to toJson()), right?

I used this until now to invalidate fields after getting errors from server (i'm not validating with AngularJS). I get a JSON array with field names and errors, and then populate them to the fields with $setValidity().

But this makes it impossible to re-submit the model again to the server, because now the server always complains about empty undefined values (even if the user already changed a value inside the input).

So the only way i see for now is to remove somehow the false validity on an input, once the user changed the value of it. I tried it here http://jsfiddle.net/mpiecko/q2s7Z/3/ with another button, but the fullName property stays undefined until i edit the input again ... Hmmm.

Michael

@mpiecko
Copy link
Author

mpiecko commented Jul 5, 2014

So just as a quick update and notice: since 1.3.0 Beta 12 an input field with a manually set $setValidity("error", false) will no longer update the model, until you $setValidity("error", true) again (it did until Beta 11).

Here a JSFiddle to try it: http://jsfiddle.net/mpiecko/q2s7Z/4/

Michael

@petebacondarwin
Copy link
Member

I guess this is related to the discussion at #7976

@Narretz
Copy link
Contributor

Narretz commented Jul 6, 2014

That the model is no longer updated when at least one $validity prop is invalid is actually consistent. It shouldn't matter if it is done by a $validator or by directly calling $setValidity. What we really need is a way to set modelValues to model even if they are invalid, aka decouple checking validity and setting model value

And we should check more thoroughly for breaking changes

@mpiecko
Copy link
Author

mpiecko commented Jul 7, 2014

I see the point why an invalid input shouldn't update the model. But up until Beta 11 (and all versions before) this was an easy and understandable way to bypass Angular validation and do it server side (also mentioned in many posts on the net):

  1. Submit the model to the server
  2. Get an error JSON with field names and errors
  3. Mark all invalid fields with $setValidity to false
  4. User updates the value in the input field (which also updates the model)
  5. Re-submit the model to the server again
  6. Remove invalid fields with $setValidity to true

Since Beta 12 this no longer works, because in step 3 the model property gets undefined and step 4 no longer updates the model. So a re-submit in step 5 fails because of missing fields in the request.

But maybe server side validation should be handled differently in Angular 1.3?

@Narretz
Copy link
Contributor

Narretz commented Jul 7, 2014

Yeah, I think I used that way in some project, too. I'm not saying it should stay the way it is, but that it worked before is also not a sign that this was good design.

But it's very broken. You cannot even get the $modelValue from the submit handler because it's never set. :/

@drewhamlett
Copy link

Has this been fixed?

@lintaho
Copy link

lintaho commented Aug 27, 2014

doesn't seem like it. Are there any plans for this to make it into one of the 1.3.0 rc's?

@yiziz
Copy link

yiziz commented Sep 3, 2014

I think this is the logic that's setting $modelValue to undefined and causing all the trouble. To work with this logic, you'll have to check ctrl.$valid and then, if false, get the value from ctrl.$$invalidModelValue.

if (isValid) {

this.$setValidity = function(validationErrorKey, isValid) {
...
  if (isValid) {
...
  } else if (!$error[validationErrorKey]) {
    invalidCount++;
    if (!pendingCount) {
      toggleValidCss(false);
      ctrl.$invalid = true;
      ctrl.$valid = false;
    }
  }
...

ctrl.$modelValue = ctrl.$valid ? modelValue : undefined;

this.$$updateValidModelValue = function(modelValue) {
  ctrl.$modelValue         = ctrl.$valid ? modelValue : undefined;
  ctrl.$$invalidModelValue = ctrl.$valid ? undefined : modelValue;
};

@jeffbcross
Copy link
Contributor

@tbosch this issue seems related to #8357 Want to take this as well?

@Narretz
Copy link
Contributor

Narretz commented Sep 9, 2014

@jeffbcross This is actually the same issue as described in #8357 (comment)
Since we now have $validators, I think the best way to handle this is the allowInvalid flag, which is being implemented here: #8313

@tbosch
Copy link
Contributor

tbosch commented Sep 9, 2014

Ok, the allowInvalid flag has landed in master, which should solve the problem of external validation using ctrl.$setValidity.
The better docs are still missing though ...(see #8357).

Closing this then. Please comment if you have a usecase that could not be solved via the new allowInvalid flag.

@tbosch tbosch closed this as completed Sep 9, 2014
@tbosch tbosch self-assigned this Sep 9, 2014
@tbosch
Copy link
Contributor

tbosch commented Sep 9, 2014

Reopening this, as we probably don't want to force users to use the allowInvalid flag...

@tbosch tbosch reopened this Sep 9, 2014
tbosch added a commit to tbosch/angular.js that referenced this issue Sep 10, 2014
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 angular#8357
Fixes angular#8080
@tbosch tbosch closed this as completed in 9314719 Sep 10, 2014
@tbosch
Copy link
Contributor

tbosch commented Sep 10, 2014

Here is an updated jsfiddle: http://jsfiddle.net/q2s7Z/7/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.