Problem/Motivation

https://github.com/symfony/symfony/pull/32435 improved the Symfony Constraint violation message for the 'range' constraint. Previously the constraint would report 'This value should be %limit or more.' or 'This value should be %limit or less.' if the value was below the minimum or above the maximum. Now it will provide a 'This value should be between {{ min }} and {{ max }}.' message, unless either min or max is omitted.

I think this message is more helpful and should probably be adopted at some point, but will need the proper localization, and bc considerations.

Proposed resolution

For now, lets override the validator and force the current logic to continue in SF4

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 interdiff.3074645.5-7.txt939 bytesmikelutz
#7 3074645-7.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.31 KBmikelutz
#5 interdiff.3074645.2-5.txt1 KBmikelutz
#5 3074645-5.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.31 KBmikelutz
#2 3074645-2.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.3 KBmikelutz

Comments

mikelutz created an issue. See original summary.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Active » Needs review
Issue tags: +Symfony 4
StatusFileSize
new 3074645-2.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.3 KB

Adding custom validator.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented 
+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/RangeConstraintValidator.php
@@ -0,0 +1,72 @@
+    // Convert strings to DateTimes if comparing another DateTime

Highly minor: missing dot at end of line. But I assume you just copied the Symfony code outright?

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented

I did, and fixed standards errors that I saw. For some reason my phpcs is erroring out right now locally (I think because I'm throwing all my dependencies out of whack going back and forth between SF3 and SF4). I just decided to throw it against testbot and fix anything else that I missed when I got the report. :-)

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
StatusFileSize
new 3074645-5.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.31 KB
new interdiff.3074645.2-5.txt1 KB

phpcs didn't even complain about it after I got it working again. Anyway:

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

Sorry :(

+++ b/core/lib/Drupal/Core/Validation/Plugin/Validation/Constraint/RangeConstraintValidator.php
@@ -36,10 +36,10 @@ public function validate($value, Constraint $constraint) {
-    // http://php.net/manual/en/datetime.formats.php
+    // http://php.net/manual/en/datetime.formats.php .

Space.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 3074645-7.drupal.Account-for-changing-violation-message-in-symfony-range-constraint-between-SF3-and-SF44.patch3.31 KB
new interdiff.3074645.5-7.txt939 bytes

I did that on purpose, to not mix the period in the url, but this is better I think, and coder sniffer seems to like it.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Superb, thanks.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs review

Could we just override the $notInRangeMessage property on the constraint with the message we currently return or is that not possible with the logic changes?

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented

@lawrolan - not logically possible.

Currently it says EITHER "$value must be $min or higher" or "$value must be $max or lower" depending on whether $value was too high or too low.

in 4.4, as long as min and max are both set it will say "$value must be between $min and $max"

So it's not just a message change, but also removing the logic to decide which of the two messages to display.

Personally, I like the new message, it's more useful, but I'm not sure how we introduce it, since any multi-lingual site would need to have it's translation files updated prior to the messaging change, There may be a policy on this, but I just don't know what it is.

The plan with this patch is to ensure consistency between D8 and D9 right now, and if we are able to update strings like this in D9, then we can do it in a controlled manner at a time of our choosing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Ok, should be good to put back to RTBC then

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs review » Reviewed & tested by the community

Moving back to RTBC per #11

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed d4ae87e and pushed to 8.8.x. Thanks!

  • larowlan committed d4ae87e on 8.8.x 
    Issue #3074645 by mikelutz: Account for changing violation message in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.