Skip to content

[Validator] Added range constraint #3893

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

Closed
wants to merge 1 commit into from
Closed

[Validator] Added range constraint #3893

wants to merge 1 commit into from

Conversation

felds
Copy link
Contributor

@felds felds commented Apr 12, 2012

When using the Min and Max constraints in combination and a non-numeric value is submitted, both show an "invalid" message at the same time.

Another approach that I can see could be making their type validation optional, ignoring every non-numeric value when asked to, but this would change their current API, and I don't think it's a smart move for something so critical as validation.

This new Constraint works around that by creating a combined min/max constraint without changing their existing API and also simplifying the configuration by reducing the number of annotations needed to perform this validation.

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

@felds Your initial issue could be used by using validation groups (i.e. validate the numeric type before validating the value). So the only problem this is solving is saving some chars.

To be complete you should also update the ValidatorTypeGuesser (see how Min and Max are used there).

@felds
Copy link
Contributor Author

felds commented Apr 12, 2012

@vicb do you mean running the validators twice?

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

I mean using group sequences.

@felds
Copy link
Contributor Author

felds commented Apr 12, 2012

@vicb I didn't know about that (and google isn't helping). How can I learn more about that?

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

There should be a couple of PR / issues that you can look for in Sf or Sf-doc (#2947 for example). I am not sure if this is already documented... Can you check ? (and if it is not, please create an issue or even better a PR)

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

And also by looking at the tests

@jalliot
Copy link
Contributor

jalliot commented Apr 12, 2012

@felds Besides such a constraint already exists (at least in master, not sure about 2.0): the Size constraint.
The name may not be appropriate but it does the exact same thing.

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

@jalliot you are right, thanks. @felds can you still check the doc ?

@jalliot
Copy link
Contributor

jalliot commented Apr 12, 2012

@vicb AFAIK @bschussek (or anyone else) never had time to write something in the doc about group sequences and he seems to be the only one who know them well enough.

I created an issue in the doc: symfony/symfony-docs#1248

@vicb
Copy link
Contributor

vicb commented Apr 12, 2012

thanks

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

Successfully merging this pull request may close these issues.

3 participants