Returning undefined, will cause any subsequent validation tests to fail

[blowsie]

An example of the the issue:

 ui-validate="{
    one: false,   // a validation method that would return false
    two: true,    // a validation method that would return true
    three: true   // a validation method that would return true
 }"

Validation returns the following:

Before commit:

• $error.one: false
• $error.two: false
• $error.three: false

After commit:

• $error.one: false
• $error.two: true
• $error.three: true

[deeg]

+1

I would like to see this change too, is there any indication you guys would accept this?

[deeg]

This has been open for a couple months now, just curious why it is not being accepted?

Would anyone else like to see this?

[theothermattm]

+1 I would like it as well. Seems to make sense. @douglasduteil any chance that this might get in in the near future?

[douglasduteil]

@theothermattm Sure. Seems like a bug fix.
I could directly merge it if there were tests with it... I'm just running out of time these days...

[theothermattm]

I started writing some tests and in the process of learning a bit more about the internals came across this blog post http://blog.jdriven.com/2013/09/how-angularjs-directives-renders-model-value-and-parses-user-input/ which seems to indicate that a parser function should be undefined if it is invalid. There's also a related issue: angular/angular.js#1412 Though, I can't find any other documentation on that.

I'm wondering if perhaps the return of undefined needs to stay put then? Is there any harm in changing it in the ui-validate library? Thoughts?

[blowsie]

@theothermattm Could you describe what wrong with not returning undefined? the other statements in the if/else also return valueToValidate

[theothermattm]

My only thought is that if there is a contract to return undefined when something validates as false onto the parser function array, then there could be other consequences I'm not aware of in other directives or areas of Angular. However, you're right, the one which deals with promises does return false when the function validates to false. So I was really just throwing the info out there to see what people thought.

Also, @blowsie could you elaborate on the test case you gave in this PR? The example you gave doesn't fail with what's in master right now, I'm not sure if perhaps there was just a typo. We actually picked up on this PR because we had to make the exact same change to get validation functions to return the invalid value after being validated as false.

[theothermattm]

I went ahead and created a PR for this change along with tests and a slightly different description of the issue: #172

@douglasduteil I'm new to contributing, so let me know if this looks alright and also if this is the proper etiquette for this type of thing - I can't modify @blowsie's fork so I went ahead and created another with a separate PR.

[ProLoser]

The reason I didn't JUST merge this in is because I could have sworn that the documentation says you should NOT return anything if your validation fails.

[ProLoser]

Oh yeah, and if you looked at the code and decided to ALWAYS return it then a proper fix would be merging the 3 return statements together.

[ProLoser]

According to the docs:

$parsers

Array of functions to execute, as a pipeline, whenever the control reads value from the DOM. Each function is called, in turn, passing the value through to the next. Used to sanitize / convert the value as well as validation. For validation, the parsers should update the validity state using $setValidity(), and return undefined for invalid values.

http://docs.angularjs.org/api/ng.directive:ngModel.NgModelController#properties_$parsers

So technically, I should not have merged this in as it's current behavior is correct.

[theothermattm]

@ProLoser If you want some tests for this they are in my PR #172. Also, regarding the behavior, the link you gave to the docs is what I was looking for. Even though it's technically correct based on that, it sounds like many want the functionality of returning the invalid value (see the previous issue I linked).