Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean

*/
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is "one time use", IMO it should go to inside if at line 150.

$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
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']
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterrehm
comma at the end is highly appreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is not an array that would break the code.

);

$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 src/Symfony/Component/Form/Form.php
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
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,79 @@ 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 src/Symfony/Component/Form/Tests/Fixtures/Author.php
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