Skip to content

[Validator] Added "payload" option to all constraints for attaching domain-specific data #12005

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 24, 2014
Merged

[Validator] Added "payload" option to all constraints for attaching domain-specific data #12005

merged 1 commit into from
Sep 24, 2014

Conversation

webmozart
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7273
License MIT
Doc PR TODO

The "payload" option can be used to pass whatever data should be attached to a constraint for an application:

/**
 * Domain-specific error codes
 * @NotNull(payload="100")
 */

/**
 * Structured domain-specific data
 * @NotNull(payload={"display": "inline", "highlight": false})
 */

The term "payload" is borrowed from JSR-303.

@webmozart webmozart mentioned this pull request Sep 23, 2014
Closed
@aeoris
Copy link
Contributor

aeoris commented Sep 23, 2014

@webmozart given that you closed/replaced the PR I made (#7276) over a year and half ago after 6+ months on revision, would you mind answering me one question?

How could I use this new PR to assign a code to a ConstraintViolation? The problem still exists imho, as you can't use the code property that still can be found on ConstraintViolation.php

@webmozart
Copy link
Contributor Author

@aeoris You shouldn't. The code property is internal and meant for another purpose (namely internal validation codes).

@webmozart
Copy link
Contributor Author

@aeoris In fact, your PR contained two different things:

  1. it exposed the "code" property so a user could override it
  2. it set the internal violation codes I was talking about

The first point is solved by the generic "payload" property. The second point is partially solved by the new $violation->getConstraint() accessor. Now there are cases where a constraint validator can fail due to multiple reasons (see Length for example). In those cases, getCode() should return an appropriate error code - which you implemented in your PR.

I'll reapply your PR so that we can merge the interesting parts of it. :) Thanks again for your work!

@stof
Copy link
Member

stof commented Sep 23, 2014

This needs to be covered by some tests (to ensure that all mapping loaders are supporting it properly for instance).
Otherwise, this is a great idea

@webmozart
Copy link
Contributor Author

@stof Sure? Isn't passing options to constraints covered sufficiently already?

@stof
Copy link
Member

stof commented Sep 24, 2014

hmm, yeah, indeed.

@webmozart
Copy link
Contributor Author

The test fail is unrelated.

ping @symfony/deciders

@fabpot
Copy link
Member

fabpot commented Sep 24, 2014

👍

1 similar comment
@stof
Copy link
Member

stof commented Sep 24, 2014

👍

@fabpot
Copy link
Member

fabpot commented Sep 24, 2014

Thank you @webmozart.

@fabpot fabpot merged commit e8b7c6d into symfony:master Sep 24, 2014
fabpot added a commit that referenced this pull request Sep 24, 2014
@fabpot
feature #12005 [Validator] Added "payload" option to all constraints …
71b8c39 
…for attaching domain-specific data (webmozart)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[Validator] Added "payload" option to all constraints for attaching domain-specific data

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7273
| License       | MIT
| Doc PR        | TODO

The "payload" option can be used to pass whatever data should be attached to a constraint for an application:

```php
/**
 * Domain-specific error codes
 * @NotNull(payload="100")
 */

/**
 * Structured domain-specific data
 * @NotNull(payload={"display": "inline", "highlight": false})
 */
```

The term "payload" is borrowed from JSR-303.

Commits
-------

e8b7c6d [Validator] Added "payload" option to all constraints for attaching domain-specific data
@webmozart
Copy link
Contributor Author

@aeoris See #12021

@webmozart webmozart mentioned this pull request Sep 25, 2014
Closed
@xabbuh xabbuh mentioned this pull request Sep 25, 2014
Closed
@webmozart webmozart deleted the issue7276 branch October 22, 2014 17:21
@xabbuh xabbuh mentioned this pull request Dec 31, 2014
Merged
weaverryan added a commit to symfony/symfony-docs that referenced this pull request Mar 13, 2015
@weaverryan
feature #4724 [Reference][Constraints] document the validation payloa…
dbdb408 
…d option (xabbuh)

This PR was merged into the 2.6 branch.

Discussion
----------

[Reference][Constraints] document the validation payload option

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes (symfony/symfony#12005)
| Applies to    | 2.6+
| Fixed tickets | #4274

Commits
-------

dca4655 document the validation payload option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Validator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@webmozart @aeoris @stof @fabpot