Fix invalidMessage in Range constraint #35998
Conversation
| @@ -34,7 +34,7 @@ class Range extends Constraint | |||
|
|
|||
| public $minMessage = 'This value should be {{ limit }} or more.'; | |||
| public $maxMessage = 'This value should be {{ limit }} or less.'; | |||
| public $invalidMessage = 'This value should be a valid number.'; | |||
| public $invalidMessage = 'This value should be a valid number or a valid datetime.'; | |||
There was a problem hiding this comment.
Changing the message here is a BC break (the fact that you needed to update the existing translation messages indicates it). We need to update the translation in the validators.en.xlf file instead.
There was a problem hiding this comment.
@xabbuh As I understand, all translations can be modified, as long as only the target field is modified and not the source field?
db976fa to
b6dcbea
Compare
| @@ -128,7 +128,7 @@ | |||
| </trans-unit> | |||
| <trans-unit id="35"> | |||
| <source>This value should be a valid number.</source> | |||
| <target>This value should be a valid number.</target> | |||
| <target>This value should be a valid number or a valid datetime.</target> | |||
There was a problem hiding this comment.
this is displayed in the final UI,isn't it?
in the UI context, it might be strange to display this "or" - in a field where only a number has meaning.
Is this valid concern? Can we do something about it?
There was a problem hiding this comment.
We could add a second invalid message and refactor the RangeValidator a little, so that at least you don't get the message This value should be a valid number., when it's clear that you're validating datetimes, e.g. in the following scenario:
$this->validator->validate('foo', new Range(['min' => '2020-01-01', 'max' => '2020-01-31']));While we're at it, we could also add two more messages, so that when datetime exceeds limit, you would get an earlier / later info instead of more / less.
There was a problem hiding this comment.
I think adding dedicated messages is a better idea.
There was a problem hiding this comment.
As I understand, adding new message means, that I should switch to master. If so, I will close this PR and I will prepare a separate one.
There was a problem hiding this comment.
You can add a new message in 3.4 as it fixes a display bug.
|
Up to follow up @przemyslaw-bogusz? |
Yes, I will do it. Thanks for the reminder |
|
I've created a new PR (#36326) for the new approach to this issue. |
…dator (przemyslaw-bogusz) This PR was squashed before being merged into the 5.2-dev branch. Discussion ---------- [Validator] Add invalid datetime message in Range validator | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When the validated value is invalid (it isn't a number nor a datetime), but `min` or `max` option indicate that the `Range` constraint is being used to validate a datetime, the displayed message will be `This value should be a valid datetime` instead of `This value should be a valid number`. This PR replaces #35998. Commits ------- c777306 [Validator] Add invalid datetime message in Range validator
Since
Rangeconstraint works with numbers andDatetimeobjects shouldn't the message that is shown if the underlying value is invalid be different?