Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation improvements #1111

Closed
moschel opened this issue Jun 25, 2014 · 26 comments
Closed

Validation improvements #1111

moschel opened this issue Jun 25, 2014 · 26 comments

Comments

@moschel
Copy link
Contributor

moschel commented Jun 25, 2014

VOTE HERE

It seems like can/map/validate is sort of like can/map/define::setter but has several differences, and it might make sense to merge them.

Setter supports async, validations doesn't.

Validations supports an "errors" object and "errors" events, both of which are very useful.

I propose merging the validations plugin into define. Or at least making them work together rather than separately.

A couple thoughts:

  • Although validate would be similar to set, I think validate should be separate, because I can think of cases where you'd want the hook provided by set and validation logic separate.
  • The validate callback should be called with a success, error so you can do serverside validations
  • errors should be observable (Validations - using the errors object in views #355)
@justinbmeyer
Copy link
Contributor

What do you mean by

should be called with a success,

Sent from my iPhone

On Jun 25, 2014, at 12:31 AM, Brian Moschel [email protected] wrote:

It seems like can/map/validate is sort of like can/map/define::setter but has several differences, and it might make sense to merge them.

Setter supports async, validations doesn't.

Validations supports an "errors" object and "errors" events, both of which are very useful.

I propose merging the validations plugin into define. Or at least making them work together rather than separately.

A couple thoughts:

Although validate would be similar to set, I think validate should be separate, because I can think of cases where you'd want the hook provided by set and validation logic separate.
The validate callback should be called with a success, error so you can do serverside validations

Reply to this email directly or view it on GitHub.

@moschel
Copy link
Contributor Author

moschel commented Jun 27, 2014

I meant it should be called with a success handler (callback func) and error handler

@justinbmeyer
Copy link
Contributor

@retro might have some input on this.

@justinbmeyer justinbmeyer changed the title validate plugin is out of date Validation improvements Aug 26, 2014
@justinbmeyer justinbmeyer added this to the 2.2.0 milestone Aug 26, 2014
@justinbmeyer
Copy link
Contributor

can.compute validations discussed in #111 are now part of this.

@gsmeets
Copy link
Contributor

gsmeets commented Aug 27, 2014

What's the status on this feature? Is anyone working on it?

@sporto
Copy link
Contributor

sporto commented Aug 27, 2014

Related issue #355

I think that validations require more discussion. At the moment you cannot bind validations in a template easily. .errors need to be an observable object that can be transparently used in templates.

@rjgotten
Copy link

At the moment you cannot bind validations in a template easily. .errors need to be an observable object that can be transparently used in templates.

When .errors runs validations it already uses .attr to retrieve the attribute values that need to be validated. The entire thing is already observable if you wrap a can.compute around it.

@sporto
Copy link
Contributor

sporto commented Aug 28, 2014

@rjgotten care to make a fiddle showing this?

@retro
Copy link
Contributor

retro commented Sep 8, 2014

Here's my take on validations. I've talked about it with @justinbmeyer today, and we agreed that validations plugin should cover these cases:

  1. Validation of the current state
  2. Checking if setting attribute to a certain value would produce the error state

Validation of the current state

This one is pretty easy, and I already have an "external" validation plugin implemented that covers the following cases:

  1. Validation of attributes
  2. Validation of nested attributes
  3. Validation of arrays
  4. Validation of arrays that contain simple values (strings, numbers)
  5. Validation of a whole object

1. Validation of attributes

This is works similarly to the current CanJS validation plugin:

var validator = new ValidatorContext(modelInstance, {
    username : [ValidatorContext.rules.presenceOf()],
    password : [function(value, path){
        if(!passwordIsValid){
            return "is invalid";
        }
    }]
})

validator.errors()
// returns
{
  username : "can't be empty",
  password : "is invalid"
}

2. Validation of nested attributes

This is same as the validation of attributes, but you can use the whole path:

var validator = new ValidatorContext(modelInstance, {
    "user.username" : [ValidatorContext.rules.presenceOf()]
})

validator.errors()
//returns 
{
  "user.username" : "can't be empty"
}

3. Validation of arrays

This will validate password for each user in the users array:

var validator = new ValidatorContext(modelInstance, {
  "users.*.password" : [ValidatorContext.rules.presenceOf()]
})

validator.errors()
//returns
{
  "users.1.password" : "can't be empty"
}

Format of the attributes that is returned from the validator is similar to the paths returned from the events triggered by can.Map

You can go to any depth with it. This will validate username property of the socialNetworks array of each user in the users array:

var validator = new ValidatorContext(modelInstance, {
  "users.*.socialNetworks.*.username" : [ValidatorContext.rules.presenceOf()]
})

validator.errors()
// returns
{
  "users.1.socialNetworks.2.username" : "can't be empty"
}

4. Validation of arrays that contain simple values (strings, numbers)

You can validate strings and numbers in arrays too:

var validator = new ValidatorContext(modelInstance, {
  "users.*.socialNetworkUsernames.*" : [ValidatorContext.rules.formatOf(REGEX)]
})

validator.errors()
// returns
{
  "users.1.socialNetworkUsernames.2" : "is invalid"
}

5. Validation of a whole object

Sometimes you wan't to decide which attribute is invalid in the context of the validation:

var validator = new ValidatorContext(modelInstance, {
  "*" : [function(){
    if(this.someThing){
      return {
        username : 'is invalid'
      }
    }
  }]
})

validator.errors()
// returns
{
  "username" : "is invalid"
}

In my experience this covers all possible validation needs. Although this library is "external", we could implement the validate plugin that would provide glue between the can.Map or can.List and the Validator. I think something like this could work:

var User = can.Map.extend({
  validate : {
    username : [can.validate.presenceOf()]
  }
})

This could set a special @errors attribute with the validation results. That would fix any problem of live binding in the templates.

@errors should also be settable from the outside so you can set it with the errors returned from the API layer:

user.save().then(success, function(req){
  user.attr('@errors', JSON.parse(req.responseText));
})

Validation of a future state

AKA: will setting this attribute put my map into the error state?

I'm not sure how to approach this, but I believe that it would be best to simply clone the current object, set the value on it and return errors.

Something like this could work:

var errors = user.errorsFor({attribute : 'value'});

if(errors){
  // do something
}

Any feedback on this API is welcome.

@justinbmeyer
Copy link
Contributor

@retro What about observing when an error happens? That was the 3rd case.

@justinbmeyer
Copy link
Contributor

@retro I care a lot about getting the "simple" case right too. How could one add a simple email validation in the view layer? Consider something like angular: https://docs.angularjs.org/api/ng/input/input%5Bemail%5D

<input can-value="{email}" can-validate-pattern="\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,4}\b"/>
{{#if email.@errors}}
  <div>{{#each email.@errors}}{{.}}{{/each}}
{{/if}}

@gsmeets
Copy link
Contributor

gsmeets commented Sep 9, 2014

I still think validation is a model concern, not a view concern. That's something just about every framework out there does "wrong" as far as I'm concerned.

@rjgotten
Copy link

rjgotten commented Sep 9, 2014

@justinbmeyer

I'm seconding @gsmeets here:
Allowing validation a hard link into the view tier is a pandora's box: people will abuse that and application code readability; discoverability and reusability will suffer for it. I've seen it occuring far, far too many times to have any faith left that people can use such a shortcut responsibly.

You're validating attribute values set on a can.Map, that in this case just so happens to be updated via cross-binding to an <input> element. The actual <input> element does not factor into the actual validation cycle anywhere. Keep it that way, please.

Also:

a simple email validation

Nice that you picked up exactly this example, because it allows me to request an additional validation feature.

An oversimplified e-mail validator spitting out a hard error and not allowing a user to set a perfectly valid e-mail address is pretty much the poster-child of bad form-entry UX. (You do know that, I hope.)
The consensus is that if you perform client-side regex-based e-mail validation, the result should at most be a suggestive warning and never a hard error.

So; how about adding 'error levels' to validation while we're at it? Explicitly marking hard errors and soft warnings would make it a bit easier to wire up such operations as deciding whether a model is 'error-free' and fit to be processed further, e.g., when a user is working in a form that can be 'posted'.

@sporto
Copy link
Contributor

sporto commented Sep 9, 2014

I agree, validation should be only in the models, I don't really like the way Angular forces you declare the validation in the form, that is just their workaround because they don't have models. Some people argue that you might want different validations in different contexts, but I don't ever see that need in real apps.

@justinbmeyer
Copy link
Contributor

Validation is not only a model concern. It is a view model concern. It is possible to want validations for the view model independent of the model. This is something we are currently dealing with.

A good system allows both.

This is akin to what we are suggesting for can.connect by separating CRUD behavior from the data model while still allowing them to be together in a nice package.

Sent from my iPhone

On Sep 9, 2014, at 1:51 AM, Guido Smeets [email protected] wrote:

I still think validation is a model concern, not a view concern. That's something just about every framework out there does "wrong" as far as I'm concerned.


Reply to this email directly or view it on GitHub.

@justinbmeyer
Copy link
Contributor

We are seeing this in real apps. This is why we want to support both.

Making a validator its own, data model independent, entity is the first step. Making that mixin-able in a model or a view is the second step.

Sent from my iPhone

On Sep 9, 2014, at 2:37 AM, Sebastian Porto [email protected] wrote:

I agree, validation should be only in the models, I don't really like the way Angular forces you declare the validation in the form, that is just their workaround because they don't have models. Some people argue that you might want different validations in different contexts, but I don't ever see that need in real apps.


Reply to this email directly or view it on GitHub.

@justinbmeyer
Copy link
Contributor

Reusability is greater with validation detachable from the model.

Why are pure view validations are not discoverable? Things in the view tend to be the most discoverable.

The input, as you somewhat accidentally allude to later (talking about email validation), does factor into the validation. Is something valid while someone is typing, after the input loses focus, or after all fields have been submitted?

There's a tremendous amount of ways people do validations. The model-based validations simply have not scaled for us. We are seeking ways of having simple model-based validations and view-based validations based on a validation primitive.

And yes, I'm aware of the great many problems facing making workable validation libraries. Most of the time you present the error as a warning, other times you prevent keyboard input (ex: numeric-only). Error levels are unnecessary if you can easily express what the behavior of being in a error state, or transitioning to or from an error state should be.

Sent from my iPhone

On Sep 9, 2014, at 2:32 AM, rjgotten [email protected] wrote:

@justinbmeyer

I'm seconding @gsmeets here:
Allowing validation a hard link into the view tier is a pandora's box: people will abuse that and application code readability; discoverability and reusability will suffer for it. I've seen it occuring far, far too many times to have any faith left that people can use such a shortcut responsibly.

You're validating attribute values set on a can.Map, that in this case just so happens to be updated via cross-binding to an element. The actual element does not factor into the actual validation cycle anywhere. Keep it that way, please.

Also:

a simple email validation

Nice that you picked up exactly this example, because it allows me to request an additional validation feature.

An oversimplified e-mail validator spitting out a hard error and not allowing a user to set a perfectly valid e-mail address is pretty much the poster-child of bad form-entry UX. (You do know that, I hope.)
The consensus is that if you perform client-side regex-based e-mail validation, the result should at most be a suggestive warning and never a hard error.

So; how about adding 'error levels' to validation while we're at it? Explicitly marking hard errors and soft warnings would make it a bit easier to wire up such operations as deciding whether a model is 'error-free' and fit to be processed further. (For instance; when a user is editing a form.)


Reply to this email directly or view it on GitHub.

@cherifGsoul
Copy link
Member

Im with @justinbmeyer validation in the view (if is in declarative manner) will be great flexibility IMO validation on client side should be different from the server (like Rails), maybe parsleyjs can be considerend as example too http://parsleyjs.org/

@retro
Copy link
Contributor

retro commented Sep 9, 2014

When we talk about validations it's important to separate them in 3 groups:

  • Protocol validation (format of data: email, ip address, url...)
  • Business logic validation (something that can't be easily shared between models)
  • Server side validation (uniqueness and other stuff that can't be validated on the client)

In my opinion it does make sense to expose protocol validations through the view interface:

<input type="text" can-validate-format="email">

Or through some similar API.

The trick is to have a system that can combine all three of these in to one errors object.

That way view based validations could just be weaved in with the rest of the validations.

@shcarrico
Copy link
Contributor

I am wary of putting something like a regex into a template.. templates are (hypothetically) a simple representation of underlying functionality, and there is nothing simple about a regex. Additionally, it is going to defeat syntax validation engines attempts to help people with regex syntax.

Specifically, having a validation called out ala can-validate="email" is declarative, and provides context for the input field about what sort of validation it might need. Something like can-validate-pattern="\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,4}\b", at least to me, not only complicates the template with ugly regex, it encourages people to overload their templates with patterns that a template developer level person will not understand, and likely will copy/paste incorrectly from other implementations.

@booc0mtaco
Copy link
Contributor

Agree with @shcarrico . The worst scenario is when someone mis-copies a template regex like this, and introduces subtle, incorrect behavior.

There are also the form field types in HTML5. Could we use those http://diveintohtml5.info/forms.html#type-email? The new field types would affect the protocol validations mentioned above, and are well-defined, so they could be baked in. Using these types could have several benefits:

  • preferring type="email" instead of something like can-validate-format="email" would provide enough information to select the right type of default validator
  • type="email" would trigger the appropriate input keyboard on mobile devices (e.g., include the .com button on the keyboard)
  • would allow for any type of user-specified overrides in addition to a default email validation if necessary.

Other types of validation could rely on the other standard input field attributes and would be written to "just work" when the validation engine is enabled.

@justinbmeyer
Copy link
Contributor

Having a regexp was just an example of passing some parameter to some validation function. Don't get hung up on it.

Sent from my iPhone

On Sep 9, 2014, at 3:16 PM, Andrew Holloway [email protected] wrote:

Agree with @shcarrico . The worst scenario is when someone mis-copies a template regex like this, and introduces subtle, incorrect behavior.

There are also the form field types in HTML5. Could we use those http://diveintohtml5.info/forms.html#type-email? The new field types would affect the protocol validations mentioned above, and are well-defined, so they could be baked in. Using these types could have several benefits:

preferring type="email" instead of something like can-validate-format="email" would provide enough information to select the right type of default validator
type="email" would trigger the appropriate input keyboard on mobile devices (e.g., include the .com button on the keyboard)
would allow for any type of user-specified overrides in addition to a default email validation if necessary.
Other types of validation could rely on the other standard input field attributes and would be written to "just work" when the validation engine is enabled.


Reply to this email directly or view it on GitHub.

@sporto
Copy link
Contributor

sporto commented Sep 9, 2014

Just to be clear on what is being proposed here, several people are keen to have a way to declare validation directly on the templates, but how would that interact with validation on the models? Would you have to duplicate the validation rules?

I like having validation in my models as I can use that validation in several places. What is missing for me is a clean way to bind that validation to the views.

Re state of validations
For me a model is either valid or not at any given time. The fact that I might want to show a validation message as you type or when you hit submit is a view concern, so this I would like to see declared in the view somehow.

@rjgotten
Copy link

For me a model is either valid or not at any given time. The fact that I might want to show a validation message as you type or when you hit submit is a view concern, so this I would like to see declared in the view somehow.

'On blur' is the default behavior; a cross-binding based on, afaik, the change event. You can change that to 'as you type' by having the cross-binding be established based on the input event. This event is not supported by older IE browsers, but can be fairly accurately polyfilled using their proprietary propertychange event. Have it as an option for the cross-binding; in the view.

The 'on submit' flavor is trickier. You could do it as an option on the cross-binding, sure: it could be based on the submit event of the form element inside which the input element resides. However, you cannot guarantee that there will be a real form element present. (It's kind of a good practice for screenreaders though.)

Still, a better way to implement it is to enable users to toggle on/off the behavior where validation is run whenever a model attribute is updated. When the application user tries to perform a 'submit' / 'confirm' / 'save' / etc. action, a manual validate() call could pump validation one time.

Having such a toggle interactive would also support a scenario where validation feedback is live, but doesn't show until the first time a user tries to perform a submit-like action. One would simply flip the toggle back on when the application user performs the action and live validation feedback jumps back into action (based on either 'on blur' or 'as you type', depending on how cross-bindings are configured).

@gsmeets
Copy link
Contributor

gsmeets commented Sep 12, 2014

I've worked around this using components now in a non-livebound fashion. i.e. I trigger validation manually on change, and I force a validation of pre-set (and possibly invalid) data on submit again using a broadcast to all said components. From then on out the components (and the submit button that triggered it) remain in an invalid state until all errors are resolved.

I'm wondering if folding all of that into full observable behaviour isn't going to increase the complexity all over. I'm quite happy with the terseness of my views at the moment.

@daffl daffl modified the milestones: 2.3.0, 2.2.0 Jan 10, 2015
@daffl daffl removed this from the 2.3.0 milestone Oct 22, 2015
@daffl
Copy link
Contributor

daffl commented Oct 22, 2015

I think we can move this discussion into https://github.com/canjs/can-validate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants