Skip to content

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

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: 4.4
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ec2e68
Added `{{ min }}` and `{{ max }}` parameters to Range constraint if b…
Jan 5, 2021
f15143c
updated description according
Jan 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions reference/constraints/Range.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,15 +346,17 @@ maxMessage
**type**: ``string`` **default**: ``This value should be {{ limit }} or less.``

The message that will be shown if the underlying value is more than the
`max`_ option, and no `min`_ option has been defined (if both are defined, use
`notInRangeMessage`_).
`max`_ option, and **no** `min`_ option has been defined.

If **both** are defined, use `notInRangeMessage`_.

You can use the following parameters in this message:

=============== ==============================================================
Parameter Description
=============== ==============================================================
``{{ limit }}`` The upper limit
``{{ limit }}`` The upper limit (if only `max`_ is defined)
``{{ max }}`` The upper limit (if both `min`_ and `max`_ are defined)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
    ```

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

``{{ value }}`` The current (invalid) value
=============== ==============================================================

Expand Down Expand Up @@ -394,15 +396,17 @@ minMessage
**type**: ``string`` **default**: ``This value should be {{ limit }} or more.``

The message that will be shown if the underlying value is less than the
`min`_ option, and no `max`_ option has been defined (if both are defined, use
`notInRangeMessage`_).
`min`_ option, and **no** `max`_ option has been defined.

If **both** are defined, use `notInRangeMessage`_.

You can use the following parameters in this message:

=============== ==============================================================
Parameter Description
=============== ==============================================================
``{{ limit }}`` The lower limit
``{{ limit }}`` The lower limit (if only `min`_ is defined)
``{{ min }}`` The lower limit (if both `min`_ and `max`_ are defined)
``{{ value }}`` The current (invalid) value
=============== ==============================================================

Expand Down