Skip to content
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.

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

[Validator] Validate all groups when special group name * is specified in validate() method #50679

Open
wants to merge 5 commits into
base: 7.1
Choose a base branch
from

Conversation

dwgebler
Copy link

@dwgebler dwgebler commented Jun 15, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #49932
License MIT
Doc PR symfony/symfony-docs#18420

Adds a special group name * which can be used with the validator to validate all groups on the object being validated. This is comparable to how the group name * can be used on Serializer component, ref. https://symfony.com/doc/current/components/serializer.html#attributes-groups

@carsonbot
Copy link

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 6.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

@dwgebler dwgebler force-pushed the feature/validator-all-groups branch from e3de840 to a279c56 Compare June 15, 2023 22:50
@xabbuh
Copy link
Member

xabbuh commented Jun 16, 2023

I am afraid that doing this change would be a BC break in case someone actually uses this "special" group.

@ro0NL

This comment was marked as resolved.

@dwgebler
Copy link
Author

From the issue, im curious about your usecase

or put different, IMHO context should tell you with groups to validate, isnt it

there can also be contradictions between groups IIUC

I had a case of a large object graph (so object to be validated had several other objects with Valid constraint on them, which all also needed to be validated) and there were about 25 groups in total. So to validate the entire object every possible group on the object and every object it contained had to be specified.

It very much seems like there should be an easy way to validate an entire object regardless of group metadata; it doesn't strike me as a particularly niche case.

@dwgebler
Copy link
Author

I am afraid that doing this change would be a BC break in case someone actually uses this "special" group.

I'm happy to change the approach if necessary (open to discussion on that), or target 7.0.

@ro0NL

This comment was marked as resolved.

@dwgebler
Copy link
Author

i guess it could work in some cases, till it doesnt, eg.:

#[Blank(groups: ['a'])]
#[NotBlank(groups: ['b'])]
public $a;

but maybe that's more of a user concern 👍

Yeah I mean you'll get a violation there, but that's expected behaviour because the constraints can't both be valid. If you had contradicting constraints, it'd be a solid indicator validating everything at once isn't your use case.

@param string|GroupSequence|array<string|GroupSequence>|null $groups

maybe support true here?

Possibly. I'd be interested to get more feedback on this and other options from other maintainers before I update the PR with any new approach.

@OskarStark OskarStark changed the title Validate all groups when special group name "*" is specified in validate() Validate all groups when special group name * is specified in validate() method Jun 19, 2023
@carsonbot carsonbot changed the title Validate all groups when special group name * is specified in validate() method [Validator] Validate all groups when special group name * is specified in validate() method Jun 19, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include option in validator to validate all groups
6 participants