[Form] Disabled violation mapping of unsubmitted forms
This commit is contained in:
parent
95f8e43205
commit
9dfebd529e
@ -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
|
||||
-----
|
||||
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
@ -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,19 +741,13 @@ 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()) {
|
||||
if (count($this->getErrors(true)) > 0) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
@ -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')
|
||||
|
@ -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) {
|
||||
|
Reference in New Issue
Block a user