[Validator] Add support of nested attributes by alexandre-daubois · Pull Request #41994 · symfony/symfony

alexandre-daubois · 2021-07-05T15:12:35Z

Q	A
Branch?	5.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #38503
License	MIT
Doc PR	symfony/symfony-docs#15541

Although the RFC (https://wiki.php.net/rfc/new_in_initializers) is in the voting phase (until 14 July), it is already well on its way to passing.

Based on @nikic's development (php/php-src#7153), this makes the development of support possible. It will obviously take a little while before this pull request is merged.

If this pull request is OK for you, I'll get to work on writing the existing documentation for the attribute validation constraints.

Not sure about the Symfony version to target, as AtLeastOneOf has been introduced in 5.1. Although, I couldn't find attributes validation documentation for 4.4.

carsonbot · 2021-07-06T11:15:57Z

Hey!

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

Cheers!

Carsonbot

OskarStark · 2021-07-06T21:00:20Z

As this is a new feature it should target 5.4 branch

alexandre-daubois · 2021-07-09T12:48:30Z

Alright, I'll rebase to 5.4!

nicolas-grekas · 2021-07-10T09:00:52Z

src/Symfony/Component/Validator/Tests/Fixtures/Attribute/Nested/Entity.php

+        ]),
+        Assert\Choice(choices: ['A', 'B'], message: 'Must be one of %choices%')
+    ]
+    public $firstName;


the Ci reports PHP Fatal error: Constant expression contains invalid operations

nicolas-grekas

Let's wait for the RFC to be merged before merging :)

derrabus · 2021-07-18T19:11:22Z

src/Symfony/Component/Validator/Tests/Fixtures/Attribute/Nested/Entity.php

+        Assert\All([
+            'constraints' => [
+                new Assert\NotNull(),
+                new Assert\Range(min: 3),
+            ],
+        ]),


Suggested change

Assert\All([

'constraints' => [

new Assert\NotNull(),

new Assert\Range(min: 3),

],

]),

Assert\All(

constraints: [

new Assert\NotNull(),

new Assert\Range(min: 3),

],

),

Let's switch to named arguments, like we did for the other constraints.

derrabus · 2021-07-18T19:14:06Z

src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php

        }
+
+        if (\PHP_VERSION_ID >= 80100) {
+            yield 'nested_attributes' => ['Symfony\Component\Validator\Tests\Fixtures\Attribute\Nested'];


Suggested change

yield 'nested_attributes' => ['Symfony\Component\Validator\Tests\Fixtures\Attribute\Nested'];

yield 'nested_attributes' => ['Symfony\Component\Validator\Tests\Fixtures\NestedAttribute'];

Let's keep the directory structure flat

alexandre-daubois · 2021-07-19T13:15:10Z

Added missing tests for Sequentially and AtLeastOneOf attributes.
Added support for Collection as well (https://github.com/symfony/symfony/pull/41994/files#diff-d9f229d35c24533de28e1e630ee4e74afa4ce27d7af8f137d788898e0b9efc73R41).
Added documentation pull-request.

Required and Optional don't need update. I still added them in AnnotationLeaderTest.

derrabus · 2021-07-26T10:43:08Z

In order to get the PHP 8.0 testsuite green, you need to exclude your new fixtures in .github/patch-types.php.

derrabus

LGTM, once patch-types.php has been updated.

src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php

alexandre-daubois · 2021-07-27T07:16:05Z

In order to get the PHP 8.0 testsuite green, you need to exclude your new fixtures in .github/patch-types.php.

Will do, thanks!

fabpot · 2021-08-18T14:48:59Z

This is now available in the 8.1 PHP branch.

fabpot · 2021-08-18T14:50:26Z

Thank you @alexandre-daubois.

…constraints (alexandre-daubois) This PR was merged into the 5.4 branch. Discussion ---------- [Validator] Add attributes documentation of composite constraints Related pull-request: symfony/symfony#41994 Not really sure how to precise it only works for attributes in PHP 8.1, not 8.0. If you have a solution, please let me know! Commits ------- 7099e3f [Validator] Add attributes documentation of composite constraints

chekalsky · 2022-08-28T19:36:08Z

@alexandre-daubois @fabpot Optional is still not supported in Attributes if I am not mistaken, is it done consciously? Haven't found any mentions of it.

xabbuh · 2022-09-10T09:15:02Z

@chekalsky Did you experience any issues when using Optional or Required inside an attribute? As far as I can see this should work (and we also have a test case for this).

xabbuh · 2022-09-10T09:15:48Z

src/Symfony/Component/Validator/Tests/Fixtures/NestedAttribute/Entity.php

+                ],
+                'bar' => new Assert\Range(min: 5),
+                'baz' => new Assert\Required([new Assert\Email()]),
+                'qux' => new Assert\Optional([new Assert\NotBlank()]),


@chekalsky this is the test I mean

chekalsky · 2022-09-12T09:04:03Z

@xabbuh Yes, but only when it is not nested (e.g. I am trying to make one of the properties itself optional). Does it make sense?

stof · 2022-09-12T09:05:16Z

Well, Optional and Required don't make sense as top-level attributes. They are meant to be used inside the Collection constraint.

…ructor (derrabus) This PR was merged into the 7.2 branch. Discussion ---------- [Validator] Add $groups and $payload to Compound constructor | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | N/A | License | MIT While reviewing #49547 I noticed that the `$groups` and `$payload` parameters introduced in #41994 cannot be leveraged in compound constraints. I'd like to change that. Commits ------- bf4207c [Validator] Add $groups and $payload to Compound constructor

carsonbot added Status: Needs Review Bug Validator labels Jul 5, 2021

carsonbot added this to the 5.3 milestone Jul 5, 2021

OskarStark modified the milestones: 5.3, 5.4 Jul 6, 2021

nicolas-grekas changed the base branch from 5.3 to 5.4 July 10, 2021 08:58

nicolas-grekas approved these changes Jul 10, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 10, 2021

nicolas-grekas reviewed Jul 10, 2021

View reviewed changes

nicolas-grekas requested changes Jul 10, 2021

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Jul 10, 2021

nicolas-grekas mentioned this pull request Jul 15, 2021

[Validator] [WIP] Composite constraints as attributes #42102

Closed

derrabus requested changes Jul 18, 2021

View reviewed changes

derrabus added Feature and removed Bug labels Jul 18, 2021

derrabus reviewed Jul 18, 2021

View reviewed changes

alexandre-daubois force-pushed the feat-nested-attributes-support branch 7 times, most recently from 11508eb to bc996cc Compare July 19, 2021 13:14

alexandre-daubois force-pushed the feat-nested-attributes-support branch from bc996cc to 6635f91 Compare July 19, 2021 14:33

alexandre-daubois mentioned this pull request Jul 20, 2021

[Validator] Add attributes documentation of composite constraints symfony/symfony-docs#15541

Merged

alexandre-daubois force-pushed the feat-nested-attributes-support branch 2 times, most recently from 0a9b7f5 to 1a34fc3 Compare July 20, 2021 09:28

derrabus approved these changes Jul 26, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Jul 26, 2021

derrabus reviewed Jul 26, 2021

View reviewed changes

src/Symfony/Component/Validator/Tests/Mapping/Loader/AnnotationLoaderTest.php Outdated Show resolved Hide resolved

[Validator] Add support of nested attributes for composite constraints

1449450

alexandre-daubois force-pushed the feat-nested-attributes-support branch from 1a34fc3 to 1449450 Compare August 2, 2021 06:32

derrabus approved these changes Aug 2, 2021

View reviewed changes

derrabus mentioned this pull request Aug 18, 2021

[Validator] Add the When constraint and validator #42593

Merged

fabpot approved these changes Aug 18, 2021

View reviewed changes

fabpot merged commit edecf96 into symfony:5.4 Aug 18, 2021

This was referenced Nov 5, 2021

Release v5.4.0-BETA1 #43937

Merged

Release v6.0.0-BETA1 #43939

Merged

xabbuh reviewed Sep 10, 2022

View reviewed changes

derrabus mentioned this pull request Aug 22, 2024

[Validator] Add $groups and $payload to Compound constructor #58062

Merged

	yield 'nested_attributes' => ['Symfony\Component\Validator\Tests\Fixtures\Attribute\Nested'];
	yield 'nested_attributes' => ['Symfony\Component\Validator\Tests\Fixtures\NestedAttribute'];

Uh oh!

Conversation

alexandre-daubois commented Jul 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carsonbot commented Jul 6, 2021

Uh oh!

OskarStark commented Jul 6, 2021

Uh oh!

alexandre-daubois commented Jul 9, 2021

Uh oh!

nicolas-grekas Jul 10, 2021

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

derrabus Jul 18, 2021

Choose a reason for hiding this comment

Uh oh!

alexandre-daubois Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

derrabus Jul 18, 2021

Choose a reason for hiding this comment

Uh oh!

alexandre-daubois commented Jul 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

derrabus commented Jul 26, 2021

Uh oh!

derrabus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alexandre-daubois commented Jul 27, 2021

Uh oh!

fabpot commented Aug 18, 2021

Uh oh!

fabpot commented Aug 18, 2021

Uh oh!

chekalsky commented Aug 28, 2022

Uh oh!

xabbuh commented Sep 10, 2022

Uh oh!

xabbuh Sep 10, 2022

Choose a reason for hiding this comment

Uh oh!

chekalsky commented Sep 12, 2022

Uh oh!

stof commented Sep 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

alexandre-daubois commented Jul 5, 2021 •

edited

Loading

alexandre-daubois commented Jul 19, 2021 •

edited

Loading