NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[DX][Form][Validator] Add ability check if cocrete constraint fails.#19496

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:masterfromKoc:get-errors-by-code

Mar 22, 2017

Merged

[DX][Form][Validator] Add ability check if cocrete constraint fails.#19496

fabpot merged 1 commit intosymfony:masterfromKoc:get-errors-by-code

Mar 22, 2017

Conversation

Copy link

Contributor

Koc commentedAug 1, 2016•
edited
Loading

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	wait for travis
Fixed tickets	#15154
License	MIT
Doc PR	should open

Sometimes for big forms with multiple constraints we should handle some errors separately.

// when using validator$constraintViolations =$validator->validate(...);if (count($constraintViolations->findByCodes(UniqueEntity::NOT_UNIQUE_ERROR))) {// display some message or send email or etc}// when using formsif (count($form->getErrors()->findByCodes(UniqueEntity::NOT_UNIQUE_ERROR))) {// display some message or send email or etc}

This PR add some useful methods to handle this. Before we should iterate all failed constraints using foreach.

Feel free to suggest better names for new methods.

carsonbot added Status: Needs Review DX

DX = Developer eXperience (anything that improves the experience of using Symfony)

Form Validator Feature labels

Aug 1, 2016

HeahDude reviewed

Aug 1, 2016

View reviewed changes

src/Symfony/Component/Form/FormErrorIterator.php Outdated

		}
		}

		publicfunctiongetFormErrorByCode($code)

Copy link

Contributor

HeahDudeAug 1, 2016•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

What abouthasErrorCode? And what if there are many errors with the same code?

I suggest either to return a boolean: false => no such error, true => error(s), or empty array/array of error(s).

Copy link

ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@HeahDude I would prefer return array/empty array, what name give for this method?

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Maybe just add an "s" to error

Copy link

Contributor

ro0NLAug 7, 2016•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

FormErrorIterator::getFormErrorsByCode duplicates terms, the context is clear due the class name. We're dealing with form errors. Ie. you could go withFormErrorIterator::getByCode or maybefindByCode.

However if we can get anewFormErrorIterator instance for oneor more codes that would be truly convenient.. ie.
$specificErrros = $errors->withCodes(array(Constraint::CODE, OtherConstraint::CODE));

For quick one-liners, you could consider addinghasCode.

Copy link

ContributorAuthor

Koc commentedAug 2, 2016

//cc@webmozart

Koc force-pushed theget-errors-by-code branch from19d66ba tof3e2159Compare

September 21, 2016 16:10

ro0NL reviewed

Sep 21, 2016

View reviewed changes

src/Symfony/Component/Validator/ConstraintViolationList.php

		{
		$violations =array();
		foreach ($thisas$violation) {
		if (in_array($violation->getCode(),$codes,true)) {

Copy link

Contributor

ro0NLSep 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

$codes should be casted to array. Imo this may also be typehinted.

Copy link

ContributorAuthor

KocSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@ro0NL I've added support passing code or array of codes. No need wrap single code with array

Copy link

Contributor

ro0NLSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Personally i'd favor API like

finalpublicfunctionfindByCode($code){return$this->findByCodes(array($code));}publicfunctionfindByCodes(array$codes){ }

But this is fine 👍

Koc force-pushed theget-errors-by-code branch 2 times, most recently fromc2e1323 to5eb0822Compare

September 24, 2016 10:18

ro0NL reviewed

Sep 24, 2016

View reviewed changes

src/Symfony/Component/Form/FormErrorIterator.php

		*/
		publicfunctionfindByCodes($codes)
		{
		$codes = (array)$codes;

Copy link

Contributor

ro0NLSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Could be$errors

ro0NL reviewed

Sep 24, 2016

View reviewed changes

src/Symfony/Component/Form/FormErrorIterator.php Outdated

		publicfunctionfindByCodes($codes)
		{
		$codes = (array)$codes;
		$formErrors =array();

Copy link

Contributor

ro0NLSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Should be$error

ro0NL reviewed

Sep 24, 2016

View reviewed changes

src/Symfony/Component/Form/FormErrorIterator.php

		* @param string\|string[] $codes The codes to find
		*
		* @return static New instance which contains only specific errors.
		*/

Copy link

Contributor

ro0NLSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

As this only works for errors caused by the validator component...im not sure coupling hardcoded is appropriate. It's convenient though.. but maybe this should befindByCause..

$errros->findByCause(function($cause)use ($codes) {return$causeinstanceof ConstraintViolation &&in_array($cause->getCode(), (array)$codes,true);});

Copy link

ContributorAuthor

KocSep 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I haven't see any benefits from public methodfindByCause - too much code for writing closure. It equals to writing foreach.

Add ability retrieve errors by their code.

29a3a7e

Koc force-pushed theget-errors-by-code branch from5eb0822 to29a3a7eCompare

September 24, 2016 20:11

Copy link

ContributorAuthor

Koc commentedSep 28, 2016

@fabpot @webmozart tests pass, examples have updated

ro0NL reviewed

Sep 28, 2016

View reviewed changes

src/Symfony/Component/Form/Tests/FormErrorIteratorTest.php

		$form->addError(newFormError('Error 2!',null,array(),null,$cause));
		$cause =newConstraintViolation('Error 3!',null,array(),null,'',null,null,'code2');
		$form->addError(newFormError('Error 3!',null,array(),null,$cause));
		$formErrors =$form->getErrors();

Copy link

Contributor

ro0NLSep 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Perhaps should be tested with nested errors.

nicolas-grekas added this to the3.x milestone

Dec 6, 2016

Copy link

Member

fabpot commentedMar 22, 2017

Thank you@Koc.

fabpot merged commit29a3a7e intosymfony:master

Mar 22, 2017

fabpot added a commit that referenced this pull request

Mar 22, 2017

feature#19496[DX][Form][Validator] Add ability check if cocrete con…

ce067a3

…straint fails. (Koc)This PR was merged into the 3.3-dev branch.Discussion----------[DX][Form][Validator] Add ability check if cocrete constraint fails.| Q | A || --- | --- || Branch? | master || Bug fix? | no || New feature? | yes || BC breaks? | no || Deprecations? | no || Tests pass? | wait for travis || Fixed tickets |#15154 || License | MIT || Doc PR | should open |Sometimes for big forms with multiple constraints we should handle some errors separately.``` php// when using validator$constraintViolations = $validator->validate(...);if (count($constraintViolations->findByCodes(UniqueEntity::NOT_UNIQUE_ERROR))) {  // display some message or send email or etc}// when using formsif (count($form->getErrors()->findByCodes(UniqueEntity::NOT_UNIQUE_ERROR))) {  // display some message or send email or etc}```This PR add some useful methods to handle this. Before we should iterate all failed constraints using foreach.Feel free to suggest better names for new methods.Commits-------29a3a7e Add ability retrieve errors by their code.

nicolas-grekas modified the milestones:3.x,3.3

Mar 24, 2017