-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Validator] add groups support to the Valid constraint #21111
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
Conversation
xabbuh
commented
Dec 31, 2016
•
edited
Loading
edited
Q | A |
---|---|
Branch? | 3.4 |
Bug fix? | no |
New feature? | yes |
BC breaks? | no |
Deprecations? | no |
Tests pass? | yes |
Fixed tickets | #3622, #17622 |
License | MIT |
Doc PR | TODO |
throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Valid'); | ||
} | ||
|
||
$violations = $this->context->getValidator()->validate($value, null, array($this->context->getGroup())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not completely sure if handling the traversal really should be done here or if we should better add some special handling for the Valid
constraint to the RecursiveContextualValidator
instead. Any opinions are warmly welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks clean to me like this.
@xabbuh awesome! I confirmed it works as would validation groups work for every other constraint, i.e. in However, I do think there is one super useful use case not covered. Consider a <class name="Profile">
<property name="profilePicture">
<constraint name="Valid" />
</property>
<property name="coverPhoto">
<constraint name="Valid" />
</property>
</class>
<class name="Image">
<property name="file">
<constraint name="File">
<option name="maxSize">100k</option>
<option name="groups">
<value>profile-picture</value>
</option>
</constraint>
<constraint name="File">
<option name="maxSize">1000k</option>
<option name="groups">
<value>cover-photo</value>
</option>
</constraint>
</property>
</class> In $builder->add('profilePicture', ImageType::class, ['validation_groups' => ['profile-picture']]);
$builder->add('coverPhoto', ImageType::class, ['validation_groups' => ['cover-photo']); But try as you might, it seems to me that validation for these $form = $factory->create(ProfileType::class);
$form->handleRequest($request);
$form->isValid(); // returns true for files larger than what was specified Of course this is a bit nonsensical example, but I tried to simplify as much as possible. In one of my projects we for example have |
@kgilden This seems logical to me as the root form itself is validated in the |
@xabbuh ahh yes, sorry for the confusion. I should note that this patch is still required to make my example above work. The same groups must also be added to |
@kgilden You are right. But IMO this behaviour is straight-forward with how validation groups in Symfony work in general. Do you disagree with that? |
@xabbuh it's straight-forward if for example the <class name="Profile">
<property name="profilePicture">
<constraint name="File">
<!-- distinct options -->
</constraint>
</property>
<property name="coverPhoto">
<constraint name="File">
<!-- distinct options -->
</constraint>
</property>
</class> What I think is currently not possible is when you want to validate the same objects deeper. After this PR it's possible to do as follows. <class name="Profile">
<property name="profilePicture">
<constraint name="Valid">
<option name="groups">
<value>profile-picture</value>
</option>
</constraint>
</property>
<property name="coverPhoto">
<constraint name="Valid">
<option name="groups">
<value>cover-photo</value>
</option>
</constraint>
</property>
</class>
<class name="Image">
<property name="file">
<constraint name="File">
<option name="maxSize">100k</option>
<option name="groups">
<value>profile-picture</value>
</option>
</constraint>
<constraint name="File">
<option name="maxSize">1000k</option>
<option name="groups">
<value>cover-photo</value>
</option>
</constraint>
</property>
</class> class ProfileType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder->add('profilePicture', ImageType::class);
$builder->add('coverPhoto', ImageType::class);
}
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefaults(['validation_groups' => ['Default', 'cover-photo', 'profile-picture']]);
}
} Correct me if I'm wrong, but I don't think there was a way to do it prior to this PR. |
Indeed, that's not possible right now. |
+1 for this feature. The use case described by kgilden happens regularly for me (deep cascade validation groups) |
@xabbuh Can you add a note in the CHANGELOG before I can merge? |
@fabpot done, but are we sure that this is the right way to implement it? |
ping @symfony/deciders |
throw new UnexpectedTypeException($constraint, __NAMESPACE__.'\Valid'); | ||
} | ||
|
||
$violations = $this->context->getValidator()->validate($value, null, array($this->context->getGroup())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks clean to me like this.
Thank you @xabbuh. |
… (xabbuh) This PR was merged into the 3.4 branch. Discussion ---------- [Validator] add groups support to the Valid constraint | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #3622, #17622 | License | MIT | Doc PR | TODO Commits ------- 0ca27cc add groups support to the Valid constraint
@Atiragram : This is already supposed to be fixed in #25297, available since 3.4.1. If you still encounter an issue, please open a new issue with a reproducer or a failing test case. Thanks! |