Validation bug

[brettahale]

Using a form / schema combo like this:

{
    "form":[
        {
            "key":"Name",
            "description":"error",
            "validationMessage":{
                "201":"String is too long ({{viewValue.length}} chars), maximum {{schema.maxLength}}",
                "202":"Alpha, Numeric and Hyphens only",
                "302":"required",
                "uniqueness":"name must be unique."
            }
        }
    ],
    "schema":{
        "type":"object",
        "required":[
            "Name"
        ],
        "properties":{
            "Name":{
                "title":"Name",
                "type":"string",
                "maxLength":25,
                "pattern":"^[A-Za-z0-9\\-]+$"
            }
        }
    }
}

The validation doesn't work as expected. If you type in "_" then backspace, retype "_", you won't get a validation message. If you remove line 1787 in schemaForm.js, everything seems to work and update as needed.

Edit: Added backtick, markdown ate the _ otherwise. (@david.lgj)

[davidlgj]

The bug was that the validation message wasn't displayed properly. We use ngModel.$parsers to hook up the validation, and if a parser fail angular adds the error parse automatically. It ended up first of the errors and therefore the real error got muted.

It's in development branch, but it's a oneliner if you like to backport it before next release (this one might take a while)

[brettahale]

I looked around but couldn't find another issue. Anyway, thanks!

[davidlgj]

Thanks for reporting it!

[brettahale]

@davidlgj still get weird behavior because the model gets returned as undefined when the validation fails and throws the required error. So, in my case, when i get to 26 chars (maxLength:25), it shows the required validation message. However, adding one more character shows the correct message. I'm not sure if the proposed change I suggested has other implications but it sure seems to fix this issue.

[davidlgj]

It does. removing the return undefined as you suggested means that the model gets populated with unvalid values. "standard" angular validation does not do this so that is what we're trying to emulate here.

What version of schema form are you using? And what version of angular? I'll try it out a bit more later today

[brettahale]

angular 1.3.11
sf 0.8.2

Thanks for looking into this.

On Tue, May 19, 2015 at 10:58 PM, David Jensen notifications@github.com
wrote:

Reopened #397 #397.

—
Reply to this email directly or view it on GitHub
#397 (comment)
.

[davidlgj]

:) no problem. I did a quick test, and I can't reproduce it in current development branch. I'll try to look into it more later, or set up a plunker or somesuch.

[brettahale]

I was reading through the docs and saw this:

https://docs.angularjs.org/api/ng/type/ngModel.NgModelController#$parsers
Returning undefined from a parser means a parse error occurred. In that case, no $validators will run and the ngModel will be set to undefined unless ngModelOptions.allowInvalid is set to true. The parse error is stored in ngModel.$error.parse.

When you push the validation function into the $parsers and return undefined after failed validation, it's throwing the parse error that is filtered out. Always returning the viewValue, unless there actually is a parse error, seems like the correct action.

[davidlgj]

Yes, true. But we're not using $parsers for parsing, its a little bit of a hack using it for validation as we do today.

So basically you are right that it is correct according to the spec to always return view value, but returning undefined is correct according to user expectation. If you put a standard angular validation like ng-minlength on an input with ng-model, the model value will never be unvalid. If you input a unvalid value the value will be removed from the model until it validates. If we always returned the viewValue that would not be true for our validation, it would update the model even if the value isn't valid.

But maybe we can make some kind of workaround instead, returning undefined and then filtering out parse error feels like a hack. For Angular 1.3 we probably could still use $parsers to trigger the validation but then use a $validators to actually stop the model from being updated. That would be cleaner. For 1.2 I don't know, but I'm not sure returning undefined from $parsers in 1.2 gives you the parse errror, I'll have to test that.

[brettahale]

Gotcha, thanks for the explanation.

[davidlgj]

Hi again @brettahale, I had some time working on this tonight and I'd missed that when you sent the event schemaFormRedraw you get required instead of the correct message.

I think I got it all sorted out now, though, It was a bit complicated, especially since we support both Angular 1.2 and 1.3+

Anyways it's in the development branch right now: 71a3053

[brettahale]

@davidlgj Excellent, thanks!