[Validator] Added {{ min }} and {{ max }} placeholder to Range constraint for minMessage and maxMessage

[bartrail]

I recognized that {{ limit }} can not be used for the Range constraint if both min and max parameter are given as currently documented.

The docs say that one should rather use notInRangeMessage instead but the information that minMessage and maxMessage can still be used but with the placeholders {{ min }} and {{ max }} instead of {{ limit }} is missing.

Also, if you don't want to use the notInRangeMessage which always includes the upper and lower value but rather just the one that failed, this information is quite useful.

[OskarStark]

@xabbuh could you please review this PR? Thanks

[xabbuh]

The "or use the parameter {{ max }} instead" part does not look right to me.

[bartrail]

what do you suggest instead?

[xabbuh]

Maybe I am misreading the code, but is there actually an alternative to using notInRangeMessage?

[bartrail]

yes exactly. That's what I am trying to say with this MR. It is just not documented..
You can still choose to use minMessage and maxMessage but with with {{ min }} respectively {{ max }} instead of {{ limit }} if you choose to set both parameters.

This is confusing, for sure. And maybe this could be changed (the behavior of the validator) but this is just status quo since 4.4

[xabbuh]

I think we should not document deprecated usages.

If both options are defined, use notInRangeMessage_ instead.

Shortening the sentence to this should IMO be enough.

[bartrail]

Updated the text. I didn't know that the usage was deprecated. Deprecating this feature doesn't really make sense to me as I think it's a pretty useful feature to either show one of the messages and not always both.. Currently the user has the choice, in future the user is forced to always show "min" and "max" in one sentence. Not really an improvement imho.

[OskarStark]

If you think the deprecation should be reverted for good reasons, can you please open an issue on symfony/symfony and explain the use cases there? Thanks

In this case we should not merge this PR for now and wait the decisions to avoid back and forth here 👍🏻

@xabbuh id this is deprecated, shouldn't this be mentioned in the docs? 🧐

@cabb

[xabbuh]

This doesn't look right to me. I don't see where {{ max }} is actually used.

[bartrail]

it's used here https://github.com/symfony/validator/blob/d3a98390b77b2a8854c7b5d9058588d4b45794fb/Constraints/RangeValidator.php#L114

[xabbuh]

There it's only used if both min and max are configured where you shouldn't use the maxMessage, but should use the notInRangeMessage instead.

[bartrail]

Yes that's exactly the reason I opened this MR. Because I thought this is missing in documentation. Not knowing that one should not use it anymore. The reason for that is not clear to me though.

[xabbuh]

I would have expected that you would see a deprecation message if you were using both the min and max option together with one or both of minMessage or maxMessage. Wasn't that the case for you?

[bartrail]

Yes, that makes sense. I just checked my project but did not get any deprecation messages - at least when the form is submitted.

[xabbuh]

Would be interesting to see why that's the case. Which exact version do you use? Would you be able to create a small project that allows to reproduce it?

[bartrail]

Symfony version is 5.2.0. I see what I can do to give you a full example. For now, this is the assertion I am using if this helps in any way:

    /**
     * @var mixed
     * @Assert\Sequentially(
     *     @Assert\NotNull(
     *          message="Bitte geben Sie die Anzahl an.",
     *          groups={"invoice_address"}
     *      ),
     *     @Assert\Type(
     *          type="integer",
     *          message="Die Anzahl muss eine ganze Zahl sein.",
     *          groups={"invoice_address"}
     *     ),
     *     @Assert\Range(
     *          min=5,
     *          max=1000,
     *          minMessage="Die Anzahl muss größer als {{ min }} sein.",
     *          maxMessage="Die Anzahl darf max. {{ max }} betragen",
     *          groups={"invoice_address"}
     *     )
     * )
     */
    public $quantity;
    ```

[bartrail]

I created a sample project here https://github.com/bartrail/symfony-range-deprecation
you can start it up using vagrant and then run the curl command posted in the README.md
if you open the profiler afterwards, you will see that there is no deprecation message shown.

[xabbuh]

Thanks, that helped to understand what is going on. In fact the deprecation is triggered. But the constraint is only instantiated once and then cached. That's why the deprecation is not displayed in the profiler. I don't know yet how we can fix that. Could you please open an issue in the code repository so that we can keep track of this?

[javiereguiluz]

What should we do here? Close? Merge? Wait for some changes and merge? Thanks!

[bartrail]

I would appreciate if you could have a look at the referenced issue where I added some arguments to remove the deprecation of {{ minMessage }} and {{ maxMessage }} placeholder. If this you guys decide to keep the feature, this MR could be merged I guess. Otherwise it can be closed..