According to#36133, the improvement added in#32435 may lead to a BC break when the developer passmin andmax options and a customminMessage ormaxMessage. In this case it's expected to receive aminMessage/maxMessage in the violation but anotInRangeMessage is received instead.

So in the following conditions:

min andmax options are set
minMessage ormaxMessage is set

A deprecation is triggered. If a limit is violated and matches to the min/max message passed as option (minMessage formin violated andmaxMessage formax violated),minMessage/maxMessage is used in the violation instead ofnotInRangeMessage.

Copy link

Contributor

dmaicher commentedMar 19, 2020

Also see#33700

I also noticed that and back then we decided to document this small behavior change.

l-vo force-pushed theadd_bc_layer_range_message_change branch 2 times, most recently from2d50ed1 tof12794eCompare

March 19, 2020 12:07

nicolas-grekas added this to thenext milestone

Mar 19, 2020

nicolas-grekas reviewed

Mar 19, 2020

View reviewed changes

src/Symfony/Component/Validator/Constraints/RangeValidator.php OutdatedShow resolvedHide resolved

nicolas-grekas reviewed

Mar 19, 2020

View reviewed changes

src/Symfony/Component/Validator/Tests/Constraints/RangeTest.php OutdatedShow resolvedHide resolved

nicolas-grekas reviewed

Mar 19, 2020

View reviewed changes

src/Symfony/Component/Validator/Constraints/Range.php OutdatedShow resolvedHide resolved

l-vo force-pushed theadd_bc_layer_range_message_change branch 2 times, most recently fromd8f0c3f to1136014Compare

March 19, 2020 21:05

chalasr added the Deprecation label

Mar 19, 2020

Copy link

Member

chalasr commentedMar 19, 2020

Adding a BC layer on a maintenance branch feels wrong to me, you shouldn't get new deprecation notices in a patch release. The BC break should either be reverted or documented instead.

xabbuh added the Validator label

Mar 21, 2020

xabbuh reviewed

Mar 21, 2020

View reviewed changes

src/Symfony/Component/Validator/Constraints/Range.php OutdatedShow resolvedHide resolved

l-vo force-pushed theadd_bc_layer_range_message_change branch from1136014 todc9a366Compare

March 21, 2020 09:58

[Validator] Add BC layer for notInRangeMessage when min and max are set

092d85c

l-vo force-pushed theadd_bc_layer_range_message_change branch fromdc9a366 to092d85cCompare

March 21, 2020 10:00

Copy link

Member

nicolas-grekas commentedMar 31, 2020

The BC break should either be reverted or documented instead

Reverting would mean breaking BC on 5.0 - Documenting is what this PR is about.
If we missed triggering a deprecation while something has been deprecated, adding this deprecation is a bug fix to me.

Note that I didn't get the patch so I'm just commenting on the process :)

xabbuh modified the milestones:next,4.4

Apr 1, 2020

HeahDude reviewed

Apr 1, 2020

View reviewed changes

src/Symfony/Component/Validator/Constraints/Range.phpShow resolvedHide resolved

Copy link

Member

fabpot commentedAug 12, 2020

@l-vo What's the status here?

Copy link

ContributorAuthor

l-vo commentedAug 12, 2020

@fabpot from my point of view the fix itself is finished, the blocking point is more a discussion between contributors/core members about what should be done here.

fabpot approved these changes

Aug 12, 2020

View reviewed changes

carsonbot added the Status: Reviewed label

Aug 12, 2020

Copy link

Member

fabpot commentedAug 12, 2020

Thank you@l-vo.

fabpot merged commitf3753e9 intosymfony:4.4

Aug 12, 2020

Copy link

Member

fabpot commentedAug 12, 2020

@l-vo I've merge 4.4 into 5.1, can you create a PR for 5.1 where you remove the BC layer?

Copy link

ContributorAuthor

l-vo commentedAug 12, 2020

Of course, I'm AFAK today but I'm doing it tomorrow.

ogizanagi mentioned this pull request

Aug 12, 2020

[Validator] RangeTest: fix expected deprecation#37810

Merged

fabpot added a commit that referenced this pull request

Aug 12, 2020

minor#37810[Validator] RangeTest: fix expected deprecation (ogizanagi)

969b902

This PR was merged into the 4.4 branch.Discussion----------[Validator] RangeTest: fix expected deprecation| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       |#36140 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->| License       | MIT| Doc PR        | N/AFixeshttps://travis-ci.org/github/symfony/symfony/jobs/717184009#L5830-L5831Commits-------33c8c3a [Validator] RangeTest: fix expected deprecation

l-vo deleted the add_bc_layer_range_message_change branch

August 12, 2020 17:12

Copy link

ContributorAuthor

l-vo commentedAug 12, 2020

@fabpot Do we really want to remove the BC layer in 5.x ? In the early 5.0 and 5.1 versions, the behavior ofRange changed but no deprecations was triggered. If the BC layer is removed now, some users of 5.0 and 5.1 will never be noticed that the behavior has changed. Removing it in 6.0 will allow all the users to be noticed. What is your point of view about that please ? All users must be noticed or it's and edge case with minor impact and it can be removed right now ?