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

merged branch bschussek/form-submit-2.3 (PR #8828)

Browse Source 
This PR was merged into the 2.3 branch.

Discussion
----------

[Form][2.3] Fixed Form::submit() and Form::setData() to react to dynamic form modifications

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

ref #3767, #3768, #4548, #8748
cc @Burgov

This PR ensures that fields that are added during the submission process of a form are submitted as well. For example:

```php
$builder = $this->createFormBuilder()
    ->add('country', 'choice', ...)
    ->getForm();

$builder->get('country')->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event) {
    $form = $event->getForm()->getParent();
    $country = $event->getForm()->getData();

    $form->add('province', 'choice', /* ... something with $country ... */);
});
```

Currently, the field "province" will not be submitted, because the submission loop uses `foreach`, which ignores changes in the underyling array.

Contrary to #8827 (for 2.2), this PR also allows dynamic modifications during `setData()`:

```php
$builder = $this->createFormBuilder()
    ->add('country', 'choice', ...)
    ->getForm();

$builder->get('country')->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event) {
    $form = $event->getForm()->getParent();
    $country = $event->getData();

    $form->add('province', 'choice', /* ... something with $country ... */);
});
```

For 2.2, this fix is not possible, because it would require changing the signature `DataMapperInterface::mapDataToForms($data, array $forms)` to `DataMapperInterface::mapDataToForms($data, array &$forms)`, which would be a BC break. For 2.3, this change is not necessary (instead of an array we now pass an iterator, which is passed by reference anyway).

Additionally, this PR ensures that `setData()` is called on *every* element of a form (except those inheriting their parent data) when `setData()` is called on the root element (i.e., during initialization). Currently, when some of the intermediate nodes (e.g. embedded forms) are submitted with an empty value, `setData()` won't be called on the nodes below (i.e. the fields of the embedded form) until `get*Data()` is called on them. If `getData()` is *not* called before `createView()`, any effects of `*_DATA` event listeners attached to those fields will not be visible in the view.

Commits
-------

7a34d96 Merge branch 'form-submit-2.2' into form-submit-2.3
cd27e1f [Form] Extracted ReferencingArrayIterator out of VirtualFormAwareIterator
3cb8a80 [Form] Added a test that ensures that setData() reacts to dynamic modifications of a form's children
07d14e5 [Form] Removed exception in Button::setData(): setData() is now always called for all elements in the form tree during the initialization of the tree
b9a3770 [Form] Removed call to deprecated method
878e27c Merge branch 'form-submit-2.2' into form-submit-2.3
ccaaedf [Form] PropertyPathMapper::mapDataToForms() *always* calls setData() on every child to ensure that all *_DATA events were fired when the initialization phase is over (except for virtual forms)
19b483f [Form] Removed superfluous reset() call
5d60a4f Merge branch 'form-submit-2.2' into form-submit-2.3
00bc270 [Form] Fixed: submit() reacts to dynamic modifications of the form children
This commit is contained in:
Fabien Potencier 2013-08-23 17:13:57 +02:00
commit 6f5a163991
9 changed files with 335 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/Symfony/Component/Form/Button.php
View File

@ -200,7 +200,8 @@ class Button implements \IteratorAggregate, FormInterface
*/
public function setData($modelData)
{
throw new BadMethodCallException('Buttons cannot have data.');
// called during initialization of the form tree
// noop
}
/**

12
src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php
View File

@ -35,7 +35,7 @@ class PropertyPathMapper implements DataMapperInterface
*/
public function __construct(PropertyAccessorInterface $propertyAccessor = null)
{
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::getPropertyAccessor();
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
}
/**
@ -43,11 +43,9 @@ class PropertyPathMapper implements DataMapperInterface
*/
public function mapDataToForms($data, $forms)
{
if (null === $data || array() === $data) {
return;
}
$empty = null === $data || array() === $data;
if (!is_array($data) && !is_object($data)) {
if (!$empty && !is_array($data) && !is_object($data)) {
throw new UnexpectedTypeException($data, 'object, array or empty');
}
@ -55,8 +53,10 @@ class PropertyPathMapper implements DataMapperInterface
$propertyPath = $form->getPropertyPath();
$config = $form->getConfig();
if (null !== $propertyPath && $config->getMapped()) {
if (!$empty && null !== $propertyPath && $config->getMapped()) {
$form->setData($this->propertyAccessor->getValue($data, $propertyPath));
} else {
$form->setData($form->getConfig()->getData());
}
}
}

10
src/Symfony/Component/Form/Form.php
View File

@ -536,7 +536,10 @@ class Form implements \IteratorAggregate, FormInterface
$submittedData = array();
}
foreach ($this->children as $name => $child) {
for (reset($this->children); false !== current($this->children); next($this->children)) {
$child = current($this->children);
$name = key($this->children);
if (array_key_exists($name, $submittedData) || $clearMissing) {
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
unset($submittedData[$name]);
@ -762,7 +765,7 @@ class Form implements \IteratorAggregate, FormInterface
/**
* {@inheritdoc}
*/
public function all()
public function &all()
{
return $this->children;
}
@ -833,7 +836,8 @@ class Form implements \IteratorAggregate, FormInterface
$child->setParent($this);
if (!$this->lockSetData && $this->defaultDataSet && !$this->config->getInheritData()) {
$childrenIterator = new InheritDataAwareIterator(array($child));
$children = array($child);
$childrenIterator = new InheritDataAwareIterator($children);
$childrenIterator = new \RecursiveIteratorIterator($childrenIterator);
$this->config->getDataMapper()->mapDataToForms($viewData, $childrenIterator);
}

64
src/Symfony/Component/Form/Tests/CompoundFormTest.php
View File

@ -11,6 +11,7 @@
namespace Symfony\Component\Form\Tests;
use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper;
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\Forms;
@ -372,6 +373,41 @@ class CompoundFormTest extends AbstractFormTest
$form->add($child);
}
public function testSetDataSupportsDynamicAdditionAndRemovalOfChildren()
{
$form = $this->getBuilder()
->setCompound(true)
// We test using PropertyPathMapper on purpose. The traversal logic
// is currently contained in InheritDataAwareIterator, but even
// if that changes, this test should still function.
->setDataMapper(new PropertyPathMapper())
->getForm();
$child = $this->getMockForm('child');
$childToBeRemoved = $this->getMockForm('removed');
$childToBeAdded = $this->getMockForm('added');
$form->add($child);
$form->add($childToBeRemoved);
$child->expects($this->once())
->method('setData')
->will($this->returnCallback(function () use ($form, $childToBeAdded) {
$form->remove('removed');
$form->add($childToBeAdded);
}));
$childToBeRemoved->expects($this->never())
->method('setData');
// once when it it is created, once when it is added
$childToBeAdded->expects($this->exactly(2))
->method('setData');
// pass NULL to all children
$form->setData(array());
}
public function testSetDataMapsViewDataToChildren()
{
$test = $this;
@ -399,6 +435,34 @@ class CompoundFormTest extends AbstractFormTest
$form->setData('foo');
}
public function testSubmitSupportsDynamicAdditionAndRemovalOfChildren()
{
$child = $this->getMockForm('child');
$childToBeRemoved = $this->getMockForm('removed');
$childToBeAdded = $this->getMockForm('added');
$this->form->add($child);
$this->form->add($childToBeRemoved);
$form = $this->form;
$child->expects($this->once())
->method('submit')
->will($this->returnCallback(function () use ($form, $childToBeAdded) {
$form->remove('removed');
$form->add($childToBeAdded);
}));
$childToBeRemoved->expects($this->never())
->method('submit');
$childToBeAdded->expects($this->once())
->method('submit');
// pass NULL to all children
$this->form->submit(array());
}
public function testSubmitMapsSubmittedChildrenOntoExistingViewData()
{
$test = $this;

39
src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php
View File

@ -169,8 +169,9 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase
$this->assertNull($form->getData());
}
public function testMapDataToFormsIgnoresEmptyData()
public function testMapDataToFormsSetsDefaultDataIfPassedDataIsNull()
{
$default = new \stdClass();
$propertyPath = $this->getPropertyPath('engine');
$this->propertyAccessor->expects($this->never())
@ -179,11 +180,43 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase
$config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher);
$config->setByReference(true);
$config->setPropertyPath($propertyPath);
$form = $this->getForm($config);
$config->setData($default);
$form = $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($config))
->setMethods(array('setData'))
->getMock();
$form->expects($this->once())
->method('setData')
->with($default);
$this->mapper->mapDataToForms(null, array($form));
}
$this->assertNull($form->getData());
public function testMapDataToFormsSetsDefaultDataIfPassedDataIsEmptyArray()
{
$default = new \stdClass();
$propertyPath = $this->getPropertyPath('engine');
$this->propertyAccessor->expects($this->never())
->method('getValue');
$config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher);
$config->setByReference(true);
$config->setPropertyPath($propertyPath);
$config->setData($default);
$form = $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($config))
->setMethods(array('setData'))
->getMock();
$form->expects($this->once())
->method('setData')
->with($default);
$this->mapper->mapDataToForms(array(), array($form));
}
public function testMapFormsToDataWritesBackIfNotByReference()

122
src/Symfony/Component/Form/Tests/Util/InheritDataAwareIteratorTest.php Normal file
View File

@ -0,0 +1,122 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Form\Tests\Util;
use Symfony\Component\Form\Util\InheritDataAwareIterator;
/**
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class InheritDataAwareIteratorTest extends \PHPUnit_Framework_TestCase
{
public function testSupportDynamicModification()
{
$form = $this->getMockForm('form');
$formToBeAdded = $this->getMockForm('added');
$formToBeRemoved = $this->getMockForm('removed');
$forms = array('form' => $form, 'removed' => $formToBeRemoved);
$iterator = new InheritDataAwareIterator($forms);
$iterator->rewind();
$this->assertTrue($iterator->valid());
$this->assertSame('form', $iterator->key());
$this->assertSame($form, $iterator->current());
// dynamic modification
unset($forms['removed']);
$forms['added'] = $formToBeAdded;
// continue iteration
$iterator->next();
$this->assertTrue($iterator->valid());
$this->assertSame('added', $iterator->key());
$this->assertSame($formToBeAdded, $iterator->current());
// end of array
$iterator->next();
$this->assertFalse($iterator->valid());
}
public function testSupportDynamicModificationInRecursiveCall()
{
$inheritingForm = $this->getMockForm('inheriting');
$form = $this->getMockForm('form');
$formToBeAdded = $this->getMockForm('added');
$formToBeRemoved = $this->getMockForm('removed');
$inheritingForm->getConfig()->expects($this->any())
->method('getInheritData')
->will($this->returnValue(true));
$inheritingForm->add($form);
$inheritingForm->add($formToBeRemoved);
$forms = array('inheriting' => $inheritingForm);
$iterator = new InheritDataAwareIterator($forms);
$iterator->rewind();
$this->assertTrue($iterator->valid());
$this->assertSame('inheriting', $iterator->key());
$this->assertSame($inheritingForm, $iterator->current());
$this->assertTrue($iterator->hasChildren());
// enter nested iterator
$nestedIterator = $iterator->getChildren();
$this->assertSame('form', $nestedIterator->key());
$this->assertSame($form, $nestedIterator->current());
$this->assertFalse($nestedIterator->hasChildren());
// dynamic modification
$inheritingForm->remove('removed');
$inheritingForm->add($formToBeAdded);
// continue iteration - nested iterator discovers change in the form
$nestedIterator->next();
$this->assertTrue($nestedIterator->valid());
$this->assertSame('added', $nestedIterator->key());
$this->assertSame($formToBeAdded, $nestedIterator->current());
// end of array
$nestedIterator->next();
$this->assertFalse($nestedIterator->valid());
}
/**
* @param string $name
*
* @return \PHPUnit_Framework_MockObject_MockObject
*/
protected function getMockForm($name = 'name')
{
$config = $this->getMock('Symfony\Component\Form\FormConfigInterface');
$config->expects($this->any())
->method('getName')
->will($this->returnValue($name));
$config->expects($this->any())
->method('getCompound')
->will($this->returnValue(true));
$config->expects($this->any())
->method('getDataMapper')
->will($this->returnValue($this->getMock('Symfony\Component\Form\DataMapperInterface')));
$config->expects($this->any())
->method('getEventDispatcher')
->will($this->returnValue($this->getMock('Symfony\Component\EventDispatcher\EventDispatcher')));
return $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($config))
->disableArgumentCloning()
->setMethods(array('getViewData'))
->getMock();
}
}

21
src/Symfony/Component/Form/Util/InheritDataAwareIterator.php
View File

@ -12,24 +12,17 @@
namespace Symfony\Component\Form\Util;
/**
* Iterator that returns only forms from a form tree that do not inherit their
* parent data.
* Iterator that traverses an array of forms.
*
* If the iterator encounters a form that inherits its parent data, it enters
* the form and traverses its children as well.
* Contrary to \ArrayIterator, this iterator recognizes changes in the original
* array during iteration.
*
* You can wrap the iterator into a {@link \RecursiveIterator} in order to
* enter any child form that inherits its parent's data and iterate the children
* of that form as well.
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class InheritDataAwareIterator extends VirtualFormAwareIterator
{
/**
* Creates a new iterator.
*
* @param \Symfony\Component\Form\FormInterface[] $forms An array
*/
public function __construct(array $forms)
{
// Skip the deprecation error
\ArrayIterator::__construct($forms);
}
}

78
src/Symfony/Component/Form/Util/ReferencingArrayIterator.php Normal file
View File

@ -0,0 +1,78 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Form\Util;
/**
* Iterator that traverses an array.
*
* Contrary to {@link \ArrayIterator}, this iterator recognizes changes in the
* original array during iteration.
*
* @author Bernhard Schussek <bschussek@gmail.com>
*/
class ReferencingArrayIterator implements \Iterator
{
/**
* @var array
*/
private $array;
/**
* Creates a new iterator.
*
* @param array $array An array
*/
public function __construct(array &$array)
{
$this->array = &$array;
}
/**
*{@inheritdoc}
*/
public function current()
{
return current($this->array);
}
/**
*{@inheritdoc}
*/
public function next()
{
next($this->array);
}
/**
*{@inheritdoc}
*/
public function key()
{
return key($this->array);
}
/**
*{@inheritdoc}
*/
public function valid()
{
return null !== key($this->array);
}
/**
*{@inheritdoc}
*/
public function rewind()
{
reset($this->array);
}
}

30
src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php
View File

@ -12,39 +12,35 @@
namespace Symfony\Component\Form\Util;
/**
* Iterator that returns only forms from a form tree that do not inherit their
* parent data.
* Iterator that traverses an array of forms.
*
* If the iterator encounters a form that inherits its parent data, it enters
* the form and traverses its children as well.
* Contrary to {@link \ArrayIterator}, this iterator recognizes changes in the
* original array during iteration.
*
* You can wrap the iterator into a {@link \RecursiveIterator} in order to
* enter any child form that inherits its parent's data and iterate the children
* of that form as well.
*
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @deprecated Deprecated since version 2.3, to be removed in 3.0. Use
* {@link InheritDataAwareIterator} instead.
*/
class VirtualFormAwareIterator extends \ArrayIterator implements \RecursiveIterator
class VirtualFormAwareIterator extends ReferencingArrayIterator implements \RecursiveIterator
{
/**
* Creates a new iterator.
*
* @param \Symfony\Component\Form\FormInterface[] $forms An array
* {@inheritdoc}
*/
public function __construct(array $forms)
{
// Uncomment this as soon as the deprecation note should be shown
// trigger_error('VirtualFormAwareIterator is deprecated since version 2.3 and will be removed in 3.0. Use InheritDataAwareIterator instead.', E_USER_DEPRECATED);
parent::__construct($forms);
}
public function getChildren()
{
return new static($this->current()->all());
}
/**
*{@inheritdoc}
*/
public function hasChildren()
{
return $this->current()->getConfig()->getInheritData();
return (bool) $this->current()->getConfig()->getInheritData();
}
}