diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #18935 [Form] Consider a violation even if the form is not submitted (egeloen) 

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Consider a violation even if the form is not submitted

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes (only for the behavior)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #11493
| License       | MIT
| Doc PR        |

Hey!

I'm currently implementing an API using the form component in order to validate the payload sent (in conjonction with the FOSRestBundle). Unfortunatelly, we dig into an issue about the PATCH request which don't map some of our validation rules to the form. Basically, the violations are lost in the middle of the process.

### Use case

We have an entity with the following fields "type", "image" & "video". The field "type"can be either "default", "image" or "video" and then accordingly we use the appropriate field (none for the "default" type, video for the "video" type and image for the "image" type. Then, in our form, we change the validation groups according to our entity type in order to make the "image" field mandatory if the type is "image" and the same for the video field if the type is "video".

### Current behavior

The current behavior (since 2.5) seems to not propages a violation to a form if this form is not submitted but in our use case, changing the field "type" via a PATCH request triggers some new validation which should be reported to end user (inform that a field (video or image) is missing in the PATCH request).

### Expected behavior

The current behavior was introduced in #10567 but IMO, this update is a bug as suggested by @webmozart in https://github.com/symfony/symfony/issues/11493#issuecomment-59549054 Instead, the form component should still map validation errors to the form even if the field was not submitted. If the initial data is not valid, then your initial data was buggy from the beginning but the form should not accept it and instead of silently ignoring the errors, end users should be informed and fix it.

WDYT?

Commits
-------

c483a0f [Form] Consider a violation even if the form is not submitted
This commit is contained in:
Fabien Potencier 2016-06-21 22:19:14 +02:00
parent bbb75faa15 c483a0f928
commit f28eb9a617
2 changed files with 9 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php
View File

@ -275,9 +275,6 @@ class ViolationMapper implements ViolationMapperInterface
*/
private function acceptsErrors(FormInterface $form)
{
// Ignore non-submitted forms. This happens, for example, in PATCH
// requests.
// https://github.com/symfony/symfony/pull/10567
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
return $this->allowNonSynchronized || $form->isSynchronized();
}
}

16
src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php
View File

@ -212,7 +212,7 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
}
public function testAbortMappingIfNotSubmitted()
public function testMappingIfNotSubmitted()
{
$violation = $this->getConstraintViolation('children[address].data.street');
$parent = $this->getForm('parent');
@ -230,12 +230,12 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent);
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error');
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
}
public function testAbortDotRuleMappingIfNotSubmitted()
public function testDotRuleMappingIfNotSubmitted()
{
$violation = $this->getConstraintViolation('data.address');
$parent = $this->getForm('parent');
@ -255,9 +255,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->mapper->mapViolation($violation, $parent);
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error');
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have an error');
}
public function provideDefaultTests()