8000 merged branch bschussek/form-submit-2.3 (PR #8828) · symfony/symfony@6f5a163 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6f5a163

Browse files
fabpotfabpot
committed
merged branch bschussek/form-submit-2.3 (PR #8828)
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
2 parents 8e1cb3e + 7a34d96 commit 6f5a163

File tree

9 files changed

+335
-44
lines changed

9 files changed

+335
-44
lines changed

‎src/Symfony/Component/Form/Button.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ public function getErrors()
200200
*/
201201
public function setData($modelData)
202202
{
203-
throw new BadMethodCallException('Buttons cannot have data.');
203+
// called during initialization of the form tree
204+
// noop
204205
}
205206

206207
/**

‎src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,28 +35,28 @@ class PropertyPathMapper implements DataMapperInterface
3535
*/
3636
public function __construct(PropertyAccessorInterface $propertyAccessor = null)
3737
{
38-
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::getPropertyAccessor();
38+
$this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor();
3939
}
4040

4141
/**
4242
* {@inheritdoc}
4343
*/
4444
public function mapDataToForms($data, $forms)
4545
{
46-
if (null === $data || array() === $data) {
47-
return;
48-
}
46+
$empty = null === $data || array() === $data;
4947

50-
if (!is_array($data) && !is_object($data)) {
48+
if (!$empty && !is_array($data) && !is_object($data)) {
5149
throw new UnexpectedTypeException($data, 'object, array or empty');
5250
}
5351

5452
foreach ($forms as $form) {
5553
$propertyPath = $form->getPropertyPath();
5654
$config = $form->getConfig();
5755

58-
if (null !== $propertyPath && $config->getMapped()) {
56+
if (!$empty && null !== $propertyPath && $config->getMapped()) {
5957
$form->setData($this->propertyAccessor->getValue($data, $propertyPath));
58+
} else {
59+
$form->setData($form->getConfig()->getData());
6060
}
6161
}
6262
}

‎src/Symfony/Component/Form/Form.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,10 @@ public function submit($submittedData, $clearMissing = true)
536536
$submittedData = array();
537537
}
538538

539-
foreach ($this->children as $name => $child) {
539+
for (reset($this->children); false !== current($this->children); next($this->children)) {
540+
$child = current($this->children);
541+
$name = key($this->children);
542+
540543
if (array_key_exists($name, $submittedData) || $clearMissing) {
541544
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
542545
unset($submittedData[$name]);
@@ -762,7 +765,7 @@ public function getErrorsAsString($level = 0)
762765
/**
763766
* {@inheritdoc}
764767
*/
765-
public function all()
768+
public function &all()
766769
{
767770
return $this->children;
768771
}
@@ -833,7 +836,8 @@ public function add($child, $type = null, array $options = array())
833836
$child->setParent($this);
834837

835838
if (!$this->lockSetData && $this->defaultDataSet && !$this->config->getInheritData()) {
836-
$childrenIterator = new InheritDataAwareIterator(array($child));
839+
$children = array($child);
840+
$childrenIterator = new InheritDataAwareIterator($children);
837841
$childrenIterator = new \RecursiveIteratorIterator($childrenIterator);
838842
$this->config->getDataMapper()->mapDataToForms($viewData, $childrenIterator);
839843
}

‎src/Symfony/Component/Form/Tests/CompoundFormTest.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Form\Tests;
1313

14+
use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper;
1415
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
1516
use Symfony\Component\Form\FormError;
1617
use Symfony\Component\Form\Forms;
@@ -372,6 +373,41 @@ public function testAddDoesNotMapViewDataToFormIfInheritData()
372373
$form->add($child);
373374
}
374375

376+
public function testSetDataSupportsDynamicAdditionAndRemovalOfChildren()
377+
{
378+
$form = $this->getBuilder()
379+
->setCompound(true)
380+
// We test using PropertyPathMapper on purpose. The traversal logic
381+
// is currently contained in InheritDataAwareIterator, but even
382+
// if that changes, this test should still function.
383+
->setDataMapper(new PropertyPathMapper())
384+
->getForm();
385+
386+
$child = $this->getMockForm('child');
387+
$childToBeRemoved = $this->getMockForm('removed');
388+
$childToBeAdded = $this->getMockForm('added');
389+
390+
$form->add($child);
391+
$form->add($childToBeRemoved);
392+
393+
$child->expects($this->once())
394+
->method('setData')
395+
->will($this->returnCallback(function () use ($form, $childToBeAdded) {
396+
$form->remove('removed');
397+
$form->add($childToBeAdded);
398+
}));
399+
400+
$childToBeRemoved->expects($this->never())
401+
->method('setData');
402+
403+
// once when it it is created, once when it is added
404+
$childToBeAdded->expects($this->exactly(2))
405+
->method('setData');
406+
407+
// pass NULL to all children
408+
$form->setData(array());
409+
}
410+
375411
public function testSetDataMapsViewDataToChildren()
376412
{
377413
$test = $this;
@@ -399,6 +435,34 @@ public function testSetDataMapsViewDataToChildren()
399435
$form->setData('foo');
400436
}
401437

438+
public function testSubmitSupportsDynamicAdditionAndRemovalOfChildren()
439+
{
440+
$child = $this->getMockForm('child');
441+
$childToBeRemoved = $this->getMockForm('removed');
442+
$childToBeAdded = $this->getMockForm('added');
443+
444+
$this->form->add($child);
445+
$this->form->add($childToBeRemoved);
446+
447+
$form = $this->form;
448+
449+
$child->expects($this->once())
450+
->method('submit')
451+
->will($this->returnCallback(function () use ($form, $childToBeAdded) {
452+
$form->remove('removed');
453+
$form->add($childToBeAdded);
454+
}));
455+
456+
$childToBeRemoved->expects($this->never())
457+
->method('submit');
458+
459+
$childToBeAdded->expects($this->once())
460+
->method('submit');
461+
462+
// pass NULL to all children
463+
$this->form->submit(array());
464+
}
465+
402466
public function testSubmitMapsSubmittedChildrenOntoExistingViewData()
403467
{
404468
$test = $this;

‎src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,9 @@ public function testMapDataToFormsIgnoresUnmapped()
169169
$this->assertNull($form->getData());
170170
}
171171

172-
public function testMapDataToFormsIgnoresEmptyData()
172+
public function testMapDataToFormsSetsDefaultDataIfPassedDataIsNull()
173173
{
174+
$default = new \stdClass();
174175
$propertyPath = $this->getPropertyPath('engine');
175176

176177
$this->propertyAccessor->expects($this->never())
@@ -179,11 +180,43 @@ public function testMapDataToFormsIgnoresEmptyData()
179180
$config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher);
180181
$config->setByReference(true);
181182
$config->setPropertyPath($propertyPath);
182-
$form = $this->getForm($config);
183+
$config->setData($default);
184+
185+
$form = $this->getMockBuilder('Symfony\Component\Form\Form')
186+
->setConstructorArgs(array($config))
187+
->setMethods(array('setData'))
188+
->getMock();
189+
190+
$form->expects($this->once())
191+
->method('setData')
192+
->with($default);
183193

184194
$this->mapper->mapDataToForms(null, array($form));
195+
}
185196

186-
$this->assertNull($form->getData());
197+
public function testMapDataToFormsSetsDefaultDataIfPassedDataIsEmptyArray()
198+
{
199+
$default = new \stdClass();
200+
$propertyPath = $this->getPropertyPath('engine');
201+
202+
$this->propertyAccessor->expects($this->never())
203+
->method('getValue');
204+
205+
$config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher);
206+
$config->setByReference(true);
207+
$config->setPropertyPath($propertyPath);
208+
$config->setData($default);
209+
210+
$form = $this->getMockBuilder('Symfony\Component\Form\Form')
211+
->setConstructorArgs(array($config))
212+
->setMethods(array('setData'))
213+
->getMock();
214+
215+
$form->expects($this->once())
216+
->method('setData')
217+
->with($default);
218+
219+
$this->mapper->mapDataToForms(array(), array($form));
187220
}
188221

189222
public function testMapFormsToDataWritesBackIfNotByReference()

‎src/Symfony/Component/Form/Tests/Util/InheritDataAwareIteratorTest.php

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Form\Tests\Util;
13+
14+
use Symfony\Component\Form\Util\InheritDataAwareIterator;
15+
16+
/**
17+
* @author Bernhard Schussek <bschussek@gmail.com>
18+
*/
19+
class InheritDataAwareIteratorTest extends \PHPUnit_Framework_TestCase
20+
{
21+
public function testSupportDynamicModification()
22+
{
23+
$form = $this->getMockForm('form');
24+
$formToBeAdded = $this->getMockForm('added');
25+
$formToBeRemoved = $this->getMockForm('removed');
26+
27+
$forms = array('form' => $form, 'removed' => $formToBeRemoved);
28+
$iterator = new InheritDataAwareIterator($forms);
29+
30+
$iterator->rewind();
31+
$this->assertTrue($iterator->valid());
32+
$this->assertSame('form', $iterator->key());
33+
$this->assertSame($form, $iterator->current());
34+
35+
// dynamic modification
36+
unset($forms['removed']);
37+
$forms['added'] = $formToBeAdded;
38+
39+
// continue iteration
40+
$iterator->next();
41+
$this->assertTrue($iterator->valid());
42+
$this->assertSame('added', $iterator->key());
43+
$this->assertSame($formToBeAdded, $iterator->current());
44+
45+
// end of array
46+
$iterator->next();
47+
$this->assertFalse($iterator->valid());
48+
}
49+
50+
public function testSupportDynamicModificationInRecursiveCall()
51+
{
52+
$inheritingForm = $this->getMockForm('inheriting');
53+
$form = $this->getMockForm('form');
54+
$formToBeAdded = $this->getMockForm('added');
55+
$formToBeRemoved = $this->getMockForm('removed');
56+
57+
$inheritingForm->getConfig()->expects($this->any())
58+
->method('getInheritData')
59+
->will($this->returnValue(true));
60+
61+
$inheritingForm->add($form);
62+
$inheritingForm->add($formToBeRemoved);
63+
64+
$forms = array('inheriting' => $inheritingForm);
65+
$iterator = new InheritDataAwareIterator($forms);
66+
67+
$iterator->rewind();
68+
$this->assertTrue($iterator->valid());
69+
$this->assertSame('inheriting', $iterator->key());
70+
$this->assertSame($inheritingForm, $iterator->current());
71+
$this->assertTrue($iterator->hasChildren());
72+
73+
// enter nested iterator
74+
$nestedIterator = $iterator->getChildren();
75+
$this->assertSame('form', $nestedIterator->key());
76+
$this->assertSame($form, $nestedIterator->current());
77+
$this->assertFalse($nestedIterator->hasChildren());
78+
79+
// dynamic modification
80+
$inheritingForm->remove('removed');
81+
$inheritingForm->add($formToBeAdded);
82+
83+
// continue iteration - nested iterator discovers change in the form
84+
$nestedIterator->next();
85+
$this->assertTrue($nestedIterator->valid());
86+
$this->assertSame('added', $nestedIterator->key());
87+
$this->assertSame($formToBeAdded, $nestedIterator->current());
88+
89+
// end of array
90+
$nestedIterator->next();
91+
$this->assertFalse($nestedIterator->valid());
92+
}
93+
94+
/**
95+
* @param string $name
96+
*
97+
* @return \PHPUnit_Framework_MockObject_MockObject
98+
*/
99+
protected function getMockForm($name = 'name')
100+
{
101+
$config = $this->getMock('Symfony\Component\Form\FormConfigInterface');
102+
103+
$config->expects($this->any())
104+
->method('getName')
105+
->will($this->returnValue($name));
106+
$config->expects($this->any())
107+
->method('getCompound')
108+
->will($this->returnValue(true));
109+
$config->expects($this->any())
110+
->method('getDataMapper')
111+
->will($this->returnValue($this->getMock('Symfony\Component\Form\DataMapperInterface')));
112+
$config->expects($this->any())
113+
->method('getEventDispatcher')
114+
->will($this->returnValue($this->getMock('Symfony\Component\EventDispatcher\EventDispatcher')));
115+
116+
return $this->getMockBuilder('Symfony\Component\Form\Form')
117+
->setConstructorArgs(array($config))
118+
->disableArgumentCloning()
119+
->setMethods(array('getViewData'))
120+
->getMock();
121+
}
122+
}

‎src/Symfony/Component/Form/Util/InheritDataAwareIterator.php

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,17 @@
1212
namespace Symfony\Component\Form\Util;
1313

1414
/**
15-
* Iterator that returns only forms from a form tree that do not inherit their
16-
* parent data.
15+
* Iterator that traverses an array of forms.
1716
*
18-
* If the iterator encounters a form that inherits its parent data, it enters
19-
* the form and traverses its children as well.
17+
* Contrary to \ArrayIterator, this iterator recognizes changes in the original
18+
* array during iteration.
19+
*
20+
* You can wrap the iterator into a {@link \RecursiveIterator} in order to
21+
* enter any child form that inherits its parent's data and iterate the children
22+
* of that form as well.
2023
*
2124
* @author Bernhard Schussek <bschussek@gmail.com>
2225
*/
2326
class InheritDataAwareIterator extends VirtualFormAwareIterator
2427
{
25-
/**
26-
* Creates a new iterator.
27-
*
28-
* @param \Symfony\Component\Form\FormInterface[] $forms An array
29-
*/
30-
public function __construct(array $forms)
31-
{
32-
// Skip the deprecation error
33-
\ArrayIterator::__construct($forms);
34-
}
3528
}

0 commit comments

Comments
 (0)
0