[Form] Disabled violation mapping of unsubmitted forms

This commit is contained in:
Bernhard Schussek 2014-03-28 10:30:00 +01:00
parent 95f8e43205
commit 9dfebd529e
5 changed files with 73 additions and 29 deletions

View File

@ -11,6 +11,7 @@ CHANGELOG
* [BC BREAK] added two optional parameters to FormInterface::getErrors() and
changed the method to return a Symfony\Component\Form\FormErrorIterator
instance instead of an array
* errors mapped to unsubmitted forms are discarded now
2.4.0
-----

View File

@ -294,6 +294,9 @@ class ViolationMapper implements ViolationMapperInterface
*/
private function acceptsErrors(FormInterface $form)
{
return $this->allowNonSynchronized || $form->isSynchronized();
// 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());
}
}

View File

@ -556,8 +556,10 @@ class Form implements \IteratorAggregate, FormInterface
}
foreach ($this->children as $name => $child) {
if (array_key_exists($name, $submittedData) || $clearMissing) {
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
$isSubmitted = array_key_exists($name, $submittedData);
if ($isSubmitted || $clearMissing) {
$child->submit($isSubmitted ? $submittedData[$name] : null, $clearMissing);
unset($submittedData[$name]);
if (null !== $this->clickedButton) {
@ -739,18 +741,12 @@ class Form implements \IteratorAggregate, FormInterface
return false;
}
if (count($this->errors) > 0) {
return false;
}
if ($this->isDisabled()) {
return true;
}
foreach ($this->children as $child) {
if ($child->isSubmitted() && !$child->isValid()) {
return false;
}
if (count($this->getErrors(true)) > 0) {
return false;
}
return true;

View File

@ -51,22 +51,6 @@ class CompoundFormTest extends AbstractFormTest
$this->assertFalse($this->form->isValid());
}
public function testValidIfChildIsNotSubmitted()
{
$this->form->add($this->getBuilder('firstName')->getForm());
$this->form->add($this->getBuilder('lastName')->getForm());
$this->form->submit(array(
'firstName' => 'Bernhard',
));
// "lastName" is not "valid" because it was not submitted. This happens
// for example in PATCH requests. The parent form should still be
// considered valid.
$this->assertTrue($this->form->isValid());
}
public function testDisabledFormsValidEvenIfChildrenInvalid()
{
$form = $this->getBuilder('person')

View File

@ -125,6 +125,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($child);
$child->add($grandChild);
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@ -150,6 +152,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$child->add($grandChild);
$grandChild->add($grandGrandChild);
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
@ -170,7 +174,7 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($child);
$child->add($grandChild);
// submit to invoke the transformer and mark the form unsynchronized
// invoke the transformer and mark the form unsynchronized
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
@ -194,7 +198,7 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($child);
$child->add($grandChild);
// submit to invoke the transformer and mark the form unsynchronized
// invoke the transformer and mark the form unsynchronized
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
@ -204,6 +208,54 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
}
public function testAbortMappingIfNotSubmitted()
{
$violation = $this->getConstraintViolation('children[address].data.street');
$parent = $this->getForm('parent');
$child = $this->getForm('address', 'address');
$grandChild = $this->getForm('street' , 'street');
$parent->add($child);
$child->add($grandChild);
// Disable automatic submission of missing fields
$parent->submit(array(), false);
$child->submit(array(), false);
// $grandChild is not submitted
$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');
}
public function testAbortDotRuleMappingIfNotSubmitted()
{
$violation = $this->getConstraintViolation('data.address');
$parent = $this->getForm('parent');
$child = $this->getForm('address', 'address', null, array(
'.' => 'street',
));
$grandChild = $this->getForm('street');
$parent->add($child);
$child->add($grandChild);
// Disable automatic submission of missing fields
$parent->submit(array(), false);
$child->submit(array(), false);
// $grandChild is not submitted
$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');
}
public function provideDefaultTests()
{
// The mapping must be deterministic! If a child has the property path "[street]",
@ -743,6 +795,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($child);
$child->add($grandChild);
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
if (self::LEVEL_0 === $target) {
@ -1211,6 +1265,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($distraction);
}
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
if ($target !== self::LEVEL_0) {
@ -1396,6 +1452,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($errorChild);
$child->add($grandChild);
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
if (self::LEVEL_0 === $target) {
@ -1459,6 +1517,8 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase
$parent->add($child);
$child->add($grandChild);
$parent->submit(array());
$this->mapper->mapViolation($violation, $parent);
if (self::LEVEL_0 === $target) {