Skip to content

Commit

Permalink
feature #9773 [Form] Added delete_empty option to allow proper emptyD…
Browse files Browse the repository at this point in the history 
…ata handling of collections (peterrehm)

This PR was squashed before being merged into the 2.5-dev branch (closes #9773).

Discussion
----------

[Form] Added delete_empty option to allow proper emptyData handling of collections

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | yes/no?
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9375
| License       | MIT
| Doc PR        | symfony/symfony-docs#3338

This PR adresses the issue that if you have a form collection with the option required => false and you submit an empty collection so you will get a ArrayCollection that contains en element with the value null.

This behaviour was introduced with the following changes from symfony/symfony#3257

In addition to the above mentioned ticket there is also a description about the same issue here: http://stackoverflow.com/questions/19474872/symfony2-form-collection-allow-add-and-allow-delete-null-error-silex

With the changes of this PR the new option empty_data is introduced. With this option you will be able to ignore/delete such empty collections, so they will neither be validated nor appear as empty field in the result.

The option will remove/ignore such empty collections if you add them newly and if allow_add is enabled and
removes such empty collections only if allow_delete is enabled.

With setting required and empty_data accordingly it will be now flexible to customize to the outcome you want to achieve.

Thanks to @bschussek for the great work together - We have to discuss how to name this option so if delete
or ignore is the appropriate name.

Commits
-------

8bdb7a0 [Form] Added delete_empty option to allow proper emptyData handling of collections
  • Loading branch information
@fabpot
fabpot committed Dec 17, 2013
2 parents 2f44b34 + f3f50d4 commit f06f3e9
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 4 deletions.
28 changes: 26 additions & 2 deletions Extension/Core/EventListener/ResizeFormListener.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,18 @@ class ResizeFormListener implements EventSubscriberInterface
*/
protected $allowDelete;

public function __construct($type, array $options = array(), $allowAdd = false, $allowDelete = false)
/**
* @var bool
*/
private $deleteEmpty;

public function __construct($type, array $options = array(), $allowAdd = false, $allowDelete = false, $deleteEmpty = false)
{
$this->type = $type;
$this->allowAdd = $allowAdd;
$this->allowDelete = $allowDelete;
$this->options = $options;
$this->deleteEmpty = $deleteEmpty;
}

public static function getSubscribedEvents()
Expand Down Expand Up @@ -126,8 +132,13 @@ public function preSubmit(FormEvent $event)
public function onSubmit(FormEvent $event)
{
$form = $event->getForm();
$previousData = $event->getForm()->getData();
$data = $event->getData();

// At this point, $data is an array or an array-like object that already contains the
// new entries, which were added by the data mapper. The data mapper ignores existing
// entries, so we need to manually unset removed entries in the collection.

if (null === $data) {
$data = array();
}
Expand All @@ -136,10 +147,23 @@ public function onSubmit(FormEvent $event)
throw new UnexpectedTypeException($data, 'array or (\Traversable and \ArrayAccess)');
}

if ($this->deleteEmpty) {
foreach ($form as $name => $child) {
$isNew = !isset($previousData[$name]);

// $isNew can only be true if allowAdd is true, so we don't
// need to check allowAdd again
if ($child->isEmpty() && ($isNew || $this->allowDelete)) {
unset($data[$name]);
$form->remove($name);
}
}
}

// The data mapper only adds, but does not remove items, so do this
// here
if ($this->allowDelete) {
foreach ($data as $name => $child) {
foreach ($data as $name => $childData) {
if (!$form->has($name)) {
unset($data[$name]);
}
Expand Down
4 changes: 3 additions & 1 deletion Extension/Core/Type/CollectionType.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$options['type'],
$options['options'],
$options['allow_add'],
$options['allow_delete']
$options['allow_delete'],
$options['delete_empty']
);

$builder->addEventSubscriber($resizeListener);
Expand Down Expand Up @@ -86,6 +87,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
'prototype_name' => '__name__',
'type' => 'text',
'options' => array(),
'delete_empty' => false,
));

$resolver->setNormalizers(array(
Expand Down
4 changes: 3 additions & 1 deletion Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,9 @@ public function remove($name)
}

if (isset($this->children[$name])) {
$this->children[$name]->setParent(null);
if (!$this->children[$name]->isSubmitted()) {
$this->children[$name]->setParent(null);
}

unset($this->children[$name]);
}
Expand Down
74 changes: 74 additions & 0 deletions Tests/Extension/Core/Type/CollectionTypeTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace Symfony\Component\Form\Tests\Extension\Core\Type;

use Symfony\Component\Form\Form;
use Symfony\Component\Form\Tests\Fixtures\Author;
use Symfony\Component\Form\Tests\Fixtures\AuthorType;

class CollectionTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
{
Expand Down Expand Up @@ -88,6 +90,78 @@ public function testResizedDownIfSubmittedWithMissingDataAndAllowDelete()
$this->assertEquals(array('foo@foo.com'), $form->getData());
}

public function testResizedDownIfSubmittedWithEmptyDataAndDeleteEmpty()
{
$form = $this->factory->create('collection', null, array(
'type' => 'text',
'allow_delete' => true,
'delete_empty' => true,
));

$form->setData(array('foo@foo.com', 'bar@bar.com'));
$form->submit(array('foo@foo.com', ''));

$this->assertTrue($form->has('0'));
$this->assertFalse($form->has('1'));
$this->assertEquals('foo@foo.com', $form[0]->getData());
$this->assertEquals(array('foo@foo.com'), $form->getData());
}

public function testDontAddEmptyDataIfDeleteEmpty()
{
$form = $this->factory->create('collection', null, array(
'type' => 'text',
'allow_add' => true,
'delete_empty' => true,
));

$form->setData(array('foo@foo.com'));
$form->submit(array('foo@foo.com', ''));

$this->assertTrue($form->has('0'));
$this->assertFalse($form->has('1'));
$this->assertEquals('foo@foo.com', $form[0]->getData());
$this->assertEquals(array('foo@foo.com'), $form->getData());
}

public function testNoDeleteEmptyIfDeleteNotAllowed()
{
$form = $this->factory->create('collection', null, array(
'type' => 'text',
'allow_delete' => false,
'delete_empty' => true,
));

$form->setData(array('foo@foo.com'));
$form->submit(array(''));

$this->assertTrue($form->has('0'));
$this->assertEquals('', $form[0]->getData());
}

public function testResizedDownIfSubmittedWithCompoundEmptyDataAndDeleteEmpty()
{
$form = $this->factory->create('collection', null, array(
'type' => new AuthorType(),
// If the field is not required, no new Author will be created if the
// form is completely empty
'options' => array('required' => false),
'allow_add' => true,
'delete_empty' => true,
));

$form->setData(array(new Author('first', 'last')));
$form->submit(array(
array('firstName' => 's_first', 'lastName' => 's_last'),
array('firstName' => '', 'lastName' => ''),
));

$this->assertTrue($form->has('0'));
$this->assertFalse($form->has('1'));
$this->assertEquals(new Author('s_first', 's_last'), $form[0]->getData());
$this->assertEquals(array(new Author('s_first', 's_last')), $form->getData());
}

public function testNotResizedIfSubmittedWithExtraData()
{
$form = $this->factory->create('collection', null, array(
Expand Down
6 changes: 6 additions & 0 deletions Tests/Fixtures/Author.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ class Author

private $privateProperty;

public function __construct($firstName = null, $lastName = null)
{
$this->firstName = $firstName;
$this->lastName = $lastName;
}

public function setLastName($lastName)
{
$this->lastName = $lastName;
Expand Down

0 comments on commit f06f3e9

Please sign in to comment.