Skip to content

[Validator] Add @Validate constraint #17622

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

Closed
backbone87 opened this issue Jan 31, 2016 · 9 comments
Closed

[Validator] Add @Validate constraint #17622

backbone87 opened this issue Jan 31, 2016 · 9 comments

Comments

@backbone87
Copy link
Contributor

To solve problems like #3622 I propose adding a new constraint named Validate. The parameters of this constraint are groups and foreignGroups.

The constraint validator of Validate starts a new subvalidation in the same context as the original validation, but uses foreignGroups:

public function validate($value, Constraint $constraint) {
  // ...
  $this->context->getValidator()
    ->inContext($this->context)
    ->validate($value, null, $constraint->foreignGroups);
}
@linaori
Copy link
Contributor

linaori commented Jan 31, 2016

I wonder why the decision was made to not bother cascading everything by default. When I want my form to be valid, what possible use case would be that I could want my embedded form to be ignored? This default behavior is weird to me and I'd expect embedded forms to always be validated unless told otherwise with active validation groups being respected. This confusion is causing a lot of bugs in a lot of code.

The current shape leads to a lot of extra work to get embedded forms properly validated when using conditional validation.

@craue
Copy link
Contributor

craue commented Feb 1, 2016

@iltar, I had also complained about this when the cascade_validation option was going to get deprecated in #12237. Having to explicitly enable validation in these cases is still annoying.

@peterrehm
Copy link
Contributor

I would assume one reason is that it now forces you to define the validation properly in the entity which makes sense as you might need validation in different contexts.

With a slightly different intent I proposed #16984 which lacks feedback so far.

@backbone87
Copy link
Contributor Author

While this proposal may postively influence problems when using validation in conjunction with forms, the original problem is more about isolation of validation metadata of shared code (like a reuseable address class, that defines various validation groups, to validate the address under certain aspects like completeness or validity of single values).
From my understanding this is also the main point of the OP of #3622

@webmozart
Copy link
Contributor

Why is this needed? I see a solution but don't understand the problem.

@backbone87
Copy link
Contributor Author

Ok, I try to present a (legit) use-case:

  • Address model defines constraints and assigns them into 2 validation groups: valid (constraints checking for valid values in the address fields, but accepts null values and therefore considering all fields optional when validating against that group) and required (requires all mandatory fields to be not empty/not null)
  • User model uses an address field with an Address model. The User model has also other fields and constraints defined on them, that are assigned into 2 validation groups too: valid (constraints checking for valid values in the user fields, but accepts null values and therefore considering all fields optional when validating against that group) and required (requires all mandatory fields to be not empty/not null)
  • The address in User model is optional in all cases, but not the name field

When i now validate a user in the required group, the address generates violations for empty address fields, but the address is optional. Prefixing the validation groups is also not a great solution because, if i have another model containing a mandatory Address, i need to know that i have to validate this model with the prefixed *_required group of the Address, which exposes internals of this model.

@webmozart
Copy link
Contributor

Thanks for the explanation! I think this could be solved by adding support for the groups option to the @Valid constraint as suggested in #3622. I don't think it is necessary to add a new constraint.

@backbone87
Copy link
Contributor Author

I dont think, groups on Valid would solve all cases. Another benefit of my proposal is the isolation of groups names to code directly interacting with the (sub-)validated entity.

@xabbuh
Copy link
Member

xabbuh commented Dec 31, 2016

I warmly welcome everyone of you to take a look at the implementation in #21111 and see if it solves your issues.

fabpot added a commit that referenced this issue Aug 5, 2017
… (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
@fabpot fabpot closed this as completed Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants