Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[Form] Fixed patched forms to be valid even if children are not submi…
…tted
  • Loading branch information
webmozart committed Aug 1, 2013
1 parent 50f201e commit 85330a6
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 77 deletions.
Expand Up @@ -78,9 +78,9 @@ public function mapFormsToData($forms, &$data)
$propertyPath = $form->getPropertyPath();
$config = $form->getConfig();

// Write-back is disabled if the form is not synchronized (transformation failed)
// and if the form is disabled (modification not allowed)
if (null !== $propertyPath && $config->getMapped() && $form->isSynchronized() && !$form->isDisabled()) {
// Write-back is disabled if the form is not synchronized (transformation failed),
// if the form was not submitted and if the form is disabled (modification not allowed)
if (null !== $propertyPath && $config->getMapped() && $form->isSubmitted() && $form->isSynchronized() && !$form->isDisabled()) {
// If the data is identical to the value in $data, we are
// dealing with a reference
if (!is_object($data) || !$config->getByReference() || $form->getData() !== $this->propertyAccessor->getValue($data, $propertyPath)) {
Expand Down
12 changes: 7 additions & 5 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -710,11 +710,13 @@ public function isValid()
return false;
}

if (!$this->isDisabled()) {
foreach ($this->children as $child) {
if (!$child->isValid()) {
return false;
}
if ($this->isDisabled()) {
return true;
}

foreach ($this->children as $child) {
if ($child->isSubmitted() && !$child->isValid()) {
return false;
}
}

Expand Down
32 changes: 0 additions & 32 deletions src/Symfony/Component/Form/Tests/AbstractFormTest.php
Expand Up @@ -89,38 +89,6 @@ protected function getMockForm($name = 'name')
return $form;
}

/**
* @param string $name
*
* @return \PHPUnit_Framework_MockObject_MockObject
*/
protected function getValidForm($name)
{
$form = $this->getMockForm($name);

$form->expects($this->any())
->method('isValid')
->will($this->returnValue(true));

return $form;
}

/**
* @param string $name
*
* @return \PHPUnit_Framework_MockObject_MockObject
*/
protected function getInvalidForm($name)
{
$form = $this->getMockForm($name);

$form->expects($this->any())
->method('isValid')
->will($this->returnValue(false));

return $form;
}

/**
* @return \PHPUnit_Framework_MockObject_MockObject
*/
Expand Down
73 changes: 38 additions & 35 deletions src/Symfony/Component/Form/Tests/CompoundFormTest.php
Expand Up @@ -22,8 +22,8 @@ class CompoundFormTest extends AbstractFormTest
{
public function testValidIfAllChildrenAreValid()
{
$this->form->add($this->getValidForm('firstName'));
$this->form->add($this->getValidForm('lastName'));
$this->form->add($this->getBuilder('firstName')->getForm());
$this->form->add($this->getBuilder('lastName')->getForm());

$this->form->submit(array(
'firstName' => 'Bernhard',
Expand All @@ -35,17 +35,51 @@ public function testValidIfAllChildrenAreValid()

public function testInvalidIfChildIsInvalid()
{
$this->form->add($this->getValidForm('firstName'));
$this->form->add($this->getInvalidForm('lastName'));
$this->form->add($this->getBuilder('firstName')->getForm());
$this->form->add($this->getBuilder('lastName')->getForm());

$this->form->submit(array(
'firstName' => 'Bernhard',
'lastName' => 'Schussek',
));

$this->form->get('lastName')->addError(new FormError('Invalid'));

$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')
->setDisabled(true)
->setCompound(true)
->setDataMapper($this->getDataMapper())
->add($this->getBuilder('name'))
->getForm();

$form->submit(array('name' => 'Jacques Doe'));

$form->get('name')->addError(new FormError('Invalid'));

$this->assertTrue($form->isValid());
}

public function testSubmitForwardsNullIfValueIsMissing()
{
$child = $this->getMockForm('firstName');
Expand Down Expand Up @@ -121,37 +155,6 @@ public function testNotEmptyIfChildNotEmpty()
$this->assertFalse($this->form->isEmpty());
}

public function testValidIfSubmittedAndDisabledWithChildren()
{
$this->factory->expects($this->once())
->method('createNamedBuilder')
->with('name', 'text', null, array())
->will($this->returnValue($this->getBuilder('name')));

$form = $this->getBuilder('person')
->setDisabled(true)
->setCompound(true)
->setDataMapper($this->getDataMapper())
->add('name', 'text')
->getForm();
$form->submit(array('name' => 'Jacques Doe'));

$this->assertTrue($form->isValid());
}

public function testNotValidIfChildNotValid()
{
$child = $this->getMockForm();
$child->expects($this->once())
->method('isValid')
->will($this->returnValue(false));

$this->form->add($child);
$this->form->submit(array());

$this->assertFalse($this->form->isValid());
}

public function testAdd()
{
$child = $this->getBuilder('foo')->getForm();
Expand Down
Expand Up @@ -64,17 +64,21 @@ private function getPropertyPath($path)
* @param Boolean $synchronized
* @return \PHPUnit_Framework_MockObject_MockObject
*/
private function getForm(FormConfigInterface $config, $synchronized = true)
private function getForm(FormConfigInterface $config, $synchronized = true, $submitted = true)
{
$form = $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($config))
->setMethods(array('isSynchronized'))
->setMethods(array('isSynchronized', 'isSubmitted'))
->getMock();

$form->expects($this->any())
->method('isSynchronized')
->will($this->returnValue($synchronized));

$form->expects($this->any())
->method('isSubmitted')
->will($this->returnValue($submitted));

return $form;
}

Expand Down Expand Up @@ -263,6 +267,24 @@ public function testMapFormsToDataIgnoresUnmapped()
$this->mapper->mapFormsToData(array($form), $car);
}

public function testMapFormsToDataIgnoresUnsubmittedForms()
{
$car = new \stdClass();
$engine = new \stdClass();
$propertyPath = $this->getPropertyPath('engine');

$this->propertyAccessor->expects($this->never())
->method('setValue');

$config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher);
$config->setByReference(true);
$config->setPropertyPath($propertyPath);
$config->setData($engine);
$form = $this->getForm($config, true, false);

$this->mapper->mapFormsToData(array($form), $car);
}

public function testMapFormsToDataIgnoresEmptyData()
{
$car = new \stdClass();
Expand Down

0 comments on commit 85330a6

Please sign in to comment.