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

[Validator] Various improvements proposal to Range constraint #31503

Closed
Lctrs opened this issue May 15, 2019 · 0 comments
Closed

[Validator] Various improvements proposal to Range constraint #31503

Lctrs opened this issue May 15, 2019 · 0 comments

Comments

@Lctrs
Copy link
Contributor

Lctrs commented May 15, 2019

Range constraint is basically LessThan and GreaterThan, but it lacks a few things which I detail below.

  1. The error messages doesn't carry a notion of range at all.

Currently, there are 2 different error messages. One for when min is not null and the value is less than min, and the other one for when max is not null and the value is greater than max.
IMHO, we should have one more message that says something like This value should be between {{ lowerBound }} and {{ upperBound }}. when max and min are not null.
This way, it's clear that the value must be in the defined range.

  1. No property path support as in comparison constraints. (EDIT : PR is here [Validator] Allow to use property paths to get limits in range constraint #31511)
  2. No support for LessThanOrEqual and GreaterThanOrEqual when comparing to bounds.

I can work on a (or multiple ?) PRs to add these features. WDYT ?

@Lctrs Lctrs changed the title [Validator] Various improvements to Range constraints [Validator] Various improvements to Range constraint May 15, 2019
@Lctrs Lctrs changed the title [Validator] Various improvements to Range constraint [Validator] Various improvements proposal to Range constraint May 15, 2019
fabpot added a commit that referenced this issue Jul 8, 2019
…in range constraint (Lctrs)

This PR was squashed before being merged into the 4.4 branch (closes #31511).

Discussion
----------

[Validator] Allow to use property paths to get limits in range constraint

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | Part of #31503
| License       | MIT
| Doc PR        | symfony/symfony-docs#11793

Similar as #22576, but for the `Range` constraint.

Commits
-------

2b50990 [Validator] Allow to use property paths to get limits in range constraint
fabpot added a commit that referenced this issue Jul 12, 2019
… both min and max (Lctrs)

This PR was squashed before being merged into the 4.4 branch (closes #32435).

Discussion
----------

[Validator] Add a new constraint message when there is both min and max

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | Part of #31503
| License       | MIT
| Doc PR        | to do

Currently, the failed validation messages in the `Range` constraint doesn't carry a notion of range. This can be confusing and error-prone if we report these messages to the user as-is.

This PR introduces a new message to the `Range` constraint (`This value should be between {{ min }} and {{ max }}.`) that will be displayed if both `min` and `max` are not `null`.

Commits
-------

c5488bc [Validator] Add a new constraint message when there is both min and max
fabpot added a commit that referenced this issue Jul 16, 2019
…or (plozmun)

This PR was merged into the 3.4 branch.

Discussion
----------

[Validator] Update Spanish translation for Range validator

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | Part of #31503
| License       | MIT
| Doc PR        |  -

Commits
-------

b5a9640 Update validators.es.xlf
@Lctrs Lctrs closed this as completed Dec 5, 2019
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

2 participants