[Validator] Add the `When` constraint and validator #42593

wuchen90 · 2021-08-16T16:43:17Z

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	no
License	MIT
Doc PR	symfony/symfony-docs#15722

This constraint allows you to apply constraints validation only if the provided condition is matched.

Usage:

namespace App\Model;

use Symfony\Component\Validator\Constraints as Assert;

class Discount
{
    private $type; // 'percent' or 'absolute'

    /**
     * @Assert\GreaterThan(0)
     * @Assert\When(
     *     expression="this.type == 'percent'",
     *     constraints={@LessThan(100, message="The value should be between 0 and 100!")}
     * )
     */
    private $value;
    // ...
}

See the documentation for details.

carsonbot · 2021-08-16T16:43:20Z

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Always add tests
Keep backward compatibility (see https://symfony.com/bc).
Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
Features and deprecations must be submitted against the 5.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

carsonbot · 2021-08-17T12:47:00Z

Hey!

I think @lmasforne has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

wuchen90 · 2021-08-18T09:59:40Z

Actually there is no Deprecation or Bug in this PR but I don't know how to remove them.

lyrixx · 2021-08-18T10:03:25Z

How does it play with PHP Attribute?

wuchen90 · 2021-08-18T12:32:40Z

How does it play with PHP Attribute?

@lyrixx
I can't find a way to make it work with PHP attribute because the constraints option expects an array of constraint instances as value.
Maybe in future version of PHP attribute it will be permitted.

nicolas-grekas · 2021-08-18T14:36:03Z

what about Assert\When(expression="...", ...)?

src/Symfony/Component/Validator/Constraints/Conditional.php

derrabus · 2021-08-18T14:39:18Z

Maybe in future version of PHP attribute it will be permitted.

PHP 8.1. The future is now. 😉

See also #41994

Seb33300 · 2021-08-19T07:37:48Z

Looks related to #32877

wuchen90 · 2021-08-19T12:21:54Z

what about Assert\When(expression="...", ...)?

I don't have a preference for the name.
I would let the community decide on this and I will make the changes.

1️⃣ Assert\Conditional(condition="", ...)
2️⃣ Assert\When(expression="...", ...)
3️⃣ Assert\Condition(condition="", ...)

wuchen90 · 2021-08-19T13:13:21Z

Maybe in future version of PHP attribute it will be permitted.

PHP 8.1. The future is now. 😉

See also #41994

@derrabus Thanks!

I added PHP attributes support but I don't know how to fix PHPUnit / Tests (8.0) (pull_request) test...

julienfalque · 2021-11-19T13:28:27Z

src/Symfony/Component/Validator/Constraints/ConditionalValidator.php

+    {
+        if (null === $this->expressionLanguage) {
+            if (!class_exists(ExpressionLanguage::class)) {
+                throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');


Suggested change

throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');

throw new LogicException(sprintf('The "symfony/expression-language" component is required to use the "%s" constraint.', __CLASS__));

Maybe move this check to Conditional's constructor like it's done for Expression?

There is already the check in the constructor.

Indeed. This makes this check unnecessary then, because you can't reach this line without an instance of Conditional, which would have already performed the same check via its constructor.

Actually we can reach this line with an instance of a class that extends Conditional and with different constructor.
I get it won't almost never happen but it seems logic in the context of this validator class.
To be honest, I took this check from

symfony/src/Symfony/Component/Validator/Constraints/ExpressionValidator.php

Lines 74 to 76 in e0cadfb

if (!class_exists(ExpressionLanguage::class)) {

throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');

}

Right. Maybe the constraint class could be made final?

I don't think I should make it final because it will make the class non extendable for further implementation.

src/Symfony/Component/Validator/Tests/Constraints/ConditionalTest.php

src/Symfony/Component/Validator/Tests/Constraints/ConditionalValidatorTest.php

maxhelias · 2022-07-01T08:14:27Z

Oh I forgot, +1 for Assert\When(expression="...", ...) to be consistent with the dependency injection attribute

wuchen90 · 2022-08-08T12:44:51Z

@maxhelias
Thanks for the review.
I addressed the changes and did the same for the docs.

src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.php

src/Symfony/Component/Validator/Constraints/When.php

src/Symfony/Component/Validator/Constraints/WhenValidator.php

xabbuh

LGTM (except for the minor comment)

src/Symfony/Component/Validator/CHANGELOG.md

src/Symfony/Component/Validator/Constraints/WhenValidator.php

src/Symfony/Component/Validator/Constraints/When.php

xabbuh · 2022-09-11T09:36:36Z

Thank you @wuchen90 and congratulations to your first Symfony contribution!

carsonbot added Status: Needs Review Bug Deprecation Feature Validator labels Aug 16, 2021

wuchen90 force-pushed the conditional-validator branch from 28c2de6 to 965f556 Compare August 17, 2021 09:58

nicolas-grekas added this to the 5.4 milestone Aug 18, 2021

nicolas-grekas reviewed Aug 18, 2021

View reviewed changes

src/Symfony/Component/Validator/Constraints/Conditional.php Outdated Show resolved Hide resolved

wuchen90 force-pushed the conditional-validator branch 4 times, most recently from e5f889f to 523e13f Compare August 19, 2021 13:10

fabpot modified the milestones: 5.4, 6.1 Nov 3, 2021

noniagriconomie mentioned this pull request Nov 17, 2021

[Validator] Add Conditionally constraint #44116

Closed

julienfalque reviewed Nov 20, 2021

View reviewed changes

wuchen90 force-pushed the conditional-validator branch from 523e13f to 8c0f9ea Compare November 26, 2021 20:50

wuchen90 requested review from chalasr, dunglas and jderusse as code owners November 26, 2021 20:50

carsonbot added the Status: Reviewed label Jul 1, 2022

wuchen90 force-pushed the conditional-validator branch 2 times, most recently from 6097bee to 56de60c Compare August 8, 2022 12:17

wuchen90 changed the title ~~[Validator] Add the Conditional constraint and validator~~ [Validator] Add the When constraint and validator Aug 8, 2022

xabbuh removed Deprecation Bug labels Aug 9, 2022

xabbuh requested changes Aug 9, 2022

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.php Show resolved Hide resolved

src/Symfony/Component/Validator/Constraints/When.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Reviewed labels Aug 9, 2022

wuchen90 force-pushed the conditional-validator branch from 56de60c to e265ad5 Compare August 9, 2022 13:25

xabbuh reviewed Aug 9, 2022

View reviewed changes

src/Symfony/Component/Validator/Constraints/WhenValidator.php Outdated Show resolved Hide resolved

wuchen90 force-pushed the conditional-validator branch 2 times, most recently from 974e2de to e265ad5 Compare August 9, 2022 15:32

xabbuh approved these changes Aug 10, 2022

View reviewed changes

src/Symfony/Component/Validator/CHANGELOG.md Show resolved Hide resolved

carsonbot added Status: Reviewed and removed Status: Needs Work labels Aug 10, 2022

wuchen90 force-pushed the conditional-validator branch from e265ad5 to ab07c8b Compare August 10, 2022 08:26

fabpot approved these changes Aug 10, 2022

View reviewed changes

src/Symfony/Component/Validator/Constraints/WhenValidator.php Outdated Show resolved Hide resolved

src/Symfony/Component/Validator/Constraints/WhenValidator.php Outdated Show resolved Hide resolved

wuchen90 force-pushed the conditional-validator branch from ab07c8b to 421520f Compare August 12, 2022 09:29

OskarStark changed the title ~~[Validator] Add the When constraint and validator~~ [Validator] Add the When constraint and validator Aug 12, 2022

OskarStark reviewed Aug 12, 2022

View reviewed changes

src/Symfony/Component/Validator/Constraints/When.php Outdated Show resolved Hide resolved

[Validator] Add the When constraint and validator

9b7bdc9

wuchen90 force-pushed the conditional-validator branch from 421520f to 9b7bdc9 Compare August 12, 2022 14:00

xabbuh merged commit c5d14a6 into symfony:6.2 Sep 11, 2022

wuchen90 deleted the conditional-validator branch September 11, 2022 21:24

fabpot mentioned this pull request Oct 24, 2022

Release v6.2.0-BETA1 #47972

Merged

derrabus mentioned this pull request Sep 10, 2023

[FrameworkBundle] Remove obsolete feature detection #51620

Merged

	throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');
	throw new LogicException(sprintf('The "symfony/expression-language" component is required to use the "%s" constraint.', __CLASS__));

	if (!class_exists(ExpressionLanguage::class)) {
	throw new LogicException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');
	}

Uh oh!

[Validator] Add the When constraint and validator #42593

[Validator] Add the When constraint and validator #42593

Uh oh!

Conversation

wuchen90 commented Aug 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carsonbot commented Aug 16, 2021

Uh oh!

carsonbot commented Aug 17, 2021

Uh oh!

wuchen90 commented Aug 18, 2021

Uh oh!

lyrixx commented Aug 18, 2021

Uh oh!

wuchen90 commented Aug 18, 2021

Uh oh!

nicolas-grekas commented Aug 18, 2021

Uh oh!

Uh oh!

derrabus commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Seb33300 commented Aug 19, 2021

Uh oh!

wuchen90 commented Aug 19, 2021

Uh oh!

wuchen90 commented Aug 19, 2021

Uh oh!

julienfalque Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wuchen90 Nov 26, 2021

Choose a reason for hiding this comment

Uh oh!

julienfalque Nov 27, 2021

Choose a reason for hiding this comment

Uh oh!

wuchen90 Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julienfalque Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

wuchen90 May 24, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxhelias commented Jul 1, 2022

Uh oh!

wuchen90 commented Aug 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xabbuh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xabbuh commented Sep 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

[Validator] Add the `When` constraint and validator #42593

[Validator] Add the `When` constraint and validator #42593

wuchen90 commented Aug 16, 2021 •

edited

Loading

derrabus commented Aug 18, 2021 •

edited

Loading

julienfalque Nov 19, 2021 •

edited

Loading

wuchen90 Dec 1, 2021 •

edited

Loading