[Form] Split the option "modifiable" of the "collection" type into "allow_add" and "allow_delete"

UPDATE.md

@@ -89,6 +89,24 @@ beta1 to beta2
 
       app/Resources/translations/catalogue.fr.xml
 
+* The option "modifiable" of the "collection" form type was split into two
+  options "allow_add" and "allow_delete".
+
+    Before:
+
+      $builder->add('tags', 'collection', array(
+          'type' => 'text',
+          'modifiable' => true,
+      ));
+
+    After:
+
+      $builder->add('tags', 'collection', array(
+          'type' => 'text',
+          'allow_add' => true,
+          'allow_delete' => true,
+      ));
+
 PR12 to beta1
 -------------
 

src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php

@@ -38,13 +38,14 @@ class ResizeFormListener implements EventSubscriberInterface
     /**
      * @var Boolean
      */
-    private $resizeOnBind;
+    private $allowAdd;
 
-    public function __construct(FormFactoryInterface $factory, $type, $resizeOnBind = false)
+    public function __construct(FormFactoryInterface $factory, $type, $allowAdd = false, $allowDelete = false)
     {
         $this->factory = $factory;
         $this->type = $type;
-        $this->resizeOnBind = $resizeOnBind;
+        $this->allowAdd = $allowAdd;
+        $this->allowDelete = $allowDelete;
     }
 
     public static function getSubscribedEvents()
@@ -71,7 +72,7 @@ class ResizeFormListener implements EventSubscriberInterface
 
         // First remove all rows except for the prototype row
         foreach ($form as $name => $child) {
-            if (!($this->resizeOnBind && '$$name$$' === $name)) {
+            if (!($this->allowAdd && '$$name$$' === $name)) {
                 $form->remove($name);
             }
         }
@@ -86,10 +87,6 @@ class ResizeFormListener implements EventSubscriberInterface
 
     public function preBind(DataEvent $event)
     {
-        if (!$this->resizeOnBind) {
-            return;
-        }
-
         $form = $event->getForm();
         $data = $event->getData();
 
@@ -102,28 +99,28 @@ class ResizeFormListener implements EventSubscriberInterface
         }
 
         // Remove all empty rows except for the prototype row
-        foreach ($form as $name => $child) {
-            if (!isset($data[$name]) && '$$name$$' !== $name) {
-                $form->remove($name);
+        if ($this->allowDelete) {
+            foreach ($form as $name => $child) {
+                if (!isset($data[$name]) && '$$name$$' !== $name) {
+                    $form->remove($name);
+                }
             }
         }
 
         // Add all additional rows
-        foreach ($data as $name => $value) {
-            if (!$form->has($name)) {
-                $form->add($this->factory->createNamed($this->type, $name, null, array(
-                    'property_path' => '['.$name.']',
-                )));
+        if ($this->allowAdd) {
+            foreach ($data as $name => $value) {
+                if (!$form->has($name)) {
+                    $form->add($this->factory->createNamed($this->type, $name, null, array(
+                        'property_path' => '['.$name.']',
+                    )));
+                }
             }
         }
     }
 
     public function onBindNormData(FilterDataEvent $event)
     {
-        if (!$this->resizeOnBind) {
-            return;
-        }
-
         $form = $event->getForm();
         $data = $event->getData();
 
@@ -135,9 +132,11 @@ class ResizeFormListener implements EventSubscriberInterface
             throw new UnexpectedTypeException($data, 'array or \Traversable');
         }
 
-        foreach ($data as $name => $child) {
-            if (!$form->has($name)) {
-                unset($data[$name]);
+        if ($this->allowDelete) {
+            foreach ($data as $name => $child) {
+                if (!$form->has($name)) {
+                    unset($data[$name]);
+                }
             }
         }
 

src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php

@@ -13,13 +13,15 @@ namespace Symfony\Component\Form\Extension\Core\Type;
 
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\FormBuilder;
+use Symfony\Component\Form\FormView;
+use Symfony\Component\Form\FormInterface;
 use Symfony\Component\Form\Extension\Core\EventListener\ResizeFormListener;
 
 class CollectionType extends AbstractType
 {
     public function buildForm(FormBuilder $builder, array $options)
     {
-        if ($options['modifiable'] && $options['prototype']) {
+        if ($options['allow_add'] && $options['prototype']) {
             $builder->add('$$name$$', $options['type'], array(
                 'property_path' => false,
                 'required' => false,
@@ -27,15 +29,24 @@ class CollectionType extends AbstractType
         }
 
         $listener = new ResizeFormListener($builder->getFormFactory(),
-                $options['type'], $options['modifiable']);
+                $options['type'], $options['allow_add'], $options['allow_delete']);
 
-        $builder->addEventSubscriber($listener);
+        $builder->addEventSubscriber($listener)
+            ->setAttribute('allow_add', $options['allow_add'])
+            ->setAttribute('allow_delete', $options['allow_delete']);
+    }
+
+    public function buildView(FormView $view, FormInterface $form)
+    {
+        $view->set('allow_add', $form->getAttribute('allow_add'));
+        $view->set('allow_delete', $form->getAttribute('allow_delete'));
     }
 
     public function getDefaultOptions(array $options)
     {
         return array(
-            'modifiable' => false,
+            'allow_add' => false,
+            'allow_delete' => false,
             'prototype'  => true,
             'type' => 'text',
         );

tests/Symfony/Tests/Component/Form/Extension/Core/EventListener/ResizeFormListenerTest.php

@@ -59,7 +59,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = array(1 => 'string', 2 => 'string');
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preSetData($event);
 
         $this->assertFalse($this->form->has('0'));
@@ -67,25 +67,25 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
         $this->assertTrue($this->form->has('2'));
     }
 
-    public function testPreSetDataRemovesPrototypeRowIfNotResizeOnBind()
+    public function testPreSetDataRemovesPrototypeRowIfNotAllowAdd()
     {
         $this->form->add($this->getForm('$$name$$'));
 
         $data = array();
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preSetData($event);
 
         $this->assertFalse($this->form->has('$$name$$'));
     }
 
-    public function testPreSetDataDoesNotRemovePrototypeRowIfResizeOnBind()
+    public function testPreSetDataDoesNotRemovePrototypeRowIfAllowAdd()
     {
         $this->form->add($this->getForm('$$name$$'));
 
         $data = array();
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', true, false);
         $listener->preSetData($event);
 
         $this->assertTrue($this->form->has('$$name$$'));
@@ -98,7 +98,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
     {
         $data = 'no array or traversable';
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preSetData($event);
     }
 
@@ -108,28 +108,40 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = null;
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preSetData($event);
     }
 
-    public function testPreBindResizesFormIfResizable()
+    public function testPreBindResizesUpIfAllowAdd()
     {
         $this->form->add($this->getForm('0'));
-        $this->form->add($this->getForm('1'));
 
         $this->factory->expects($this->once())
             ->method('createNamed')
-            ->with('text', 2, null, array('property_path' => '[2]'))
-            ->will($this->returnValue($this->getForm('2')));
+            ->with('text', 1, null, array('property_path' => '[1]'))
+            ->will($this->returnValue($this->getForm('1')));
 
-        $data = array(0 => 'string', 2 => 'string');
+        $data = array(0 => 'string', 1 => 'string');
+        $event = new DataEvent($this->form, $data);
+        $listener = new ResizeFormListener($this->factory, 'text', true, false);
+        $listener->preBind($event);
+
+        $this->assertTrue($this->form->has('0'));
+        $this->assertTrue($this->form->has('1'));
+    }
+
+    public function testPreBindResizesDownIfAllowDelete()
+    {
+        $this->form->add($this->getForm('0'));
+        $this->form->add($this->getForm('1'));
+
+        $data = array(0 => 'string');
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->preBind($event);
 
         $this->assertTrue($this->form->has('0'));
         $this->assertFalse($this->form->has('1'));
-        $this->assertTrue($this->form->has('2'));
     }
 
     // fix for https://github.com/symfony/symfony/pull/493
@@ -139,20 +151,20 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = array();
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->preBind($event);
 
         $this->assertFalse($this->form->has('0'));
     }
 
-    public function testPreBindDoesNothingIfNotResizable()
+    public function testPreBindDoesNothingIfNotAllowAddNorAllowDelete()
     {
         $this->form->add($this->getForm('0'));
         $this->form->add($this->getForm('1'));
 
         $data = array(0 => 'string', 2 => 'string');
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preBind($event);
 
         $this->assertTrue($this->form->has('0'));
@@ -167,7 +179,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
     {
         $data = 'no array or traversable';
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->preBind($event);
     }
 
@@ -177,7 +189,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = null;
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->preBind($event);
 
         $this->assertFalse($this->form->has('1'));
@@ -190,31 +202,31 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = '';
         $event = new DataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->preBind($event);
 
         $this->assertFalse($this->form->has('1'));
     }
 
-    public function testOnBindNormDataRemovesEntriesMissingInTheFormIfResizable()
+    public function testOnBindNormDataRemovesEntriesMissingInTheFormIfAllowDelete()
     {
         $this->form->add($this->getForm('1'));
 
         $data = array(0 => 'first', 1 => 'second', 2 => 'third');
         $event = new FilterDataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->onBindNormData($event);
 
         $this->assertEquals(array(1 => 'second'), $event->getData());
     }
 
-    public function testOnBindNormDataDoesNothingIfNotResizable()
+    public function testOnBindNormDataDoesNothingIfNotAllowDelete()
     {
         $this->form->add($this->getForm('1'));
 
         $data = array(0 => 'first', 1 => 'second', 2 => 'third');
         $event = new FilterDataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', false);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->onBindNormData($event);
 
         $this->assertEquals($data, $event->getData());
@@ -227,7 +239,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
     {
         $data = 'no array or traversable';
         $event = new FilterDataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, false);
         $listener->onBindNormData($event);
     }
 
@@ -237,7 +249,7 @@ class ResizeFormListenerTest extends \PHPUnit_Framework_TestCase
 
         $data = null;
         $event = new FilterDataEvent($this->form, $data);
-        $listener = new ResizeFormListener($this->factory, 'text', true);
+        $listener = new ResizeFormListener($this->factory, 'text', false, true);
         $listener->onBindNormData($event);
 
         $this->assertEquals(array(), $event->getData());

tests/Symfony/Tests/Component/Form/Extension/Core/Type/CollectionTypeTest.php

@@ -45,15 +45,15 @@ class CollectionFormTest extends TypeTestCase
         $this->assertEquals('foo@baz.com', $form[0]->getData());
     }
 
-    public function testSetDataAdjustsSizeIfModifiable()
+    public function testSetDataAddsPrototypeIfAllowAdd()
     {
         $form = $this->factory->create('collection', null, array(
             'type' => 'field',
-            'modifiable' => true,
+            'allow_add' => true,
             'prototype' => true,
         ));
-        $form->setData(array('foo@foo.com', 'foo@bar.com'));
 
+        $form->setData(array('foo@foo.com', 'foo@bar.com'));
         $this->assertTrue($form[0] instanceof Form);
         $this->assertTrue($form[1] instanceof Form);
         $this->assertTrue($form['$$name$$'] instanceof Form);
@@ -75,20 +75,6 @@ class CollectionFormTest extends TypeTestCase
         $form->setData(new \stdClass());
     }
 
-    public function testModifiableCollectionsContainExtraForm()
-    {
-        $form = $this->factory->create('collection', null, array(
-            'type' => 'field',
-            'modifiable' => true,
-            'prototype' => true,
-        ));
-        $form->setData(array('foo@bar.com'));
-
-        $this->assertTrue($form['0'] instanceof Form);
-        $this->assertTrue($form['$$name$$'] instanceof Form);
-        $this->assertEquals(2, count($form));
-    }
-
     public function testNotResizedIfBoundWithMissingData()
     {
         $form = $this->factory->create('collection', null, array(
@@ -103,11 +89,11 @@ class CollectionFormTest extends TypeTestCase
         $this->assertEquals(null, $form[1]->getData());
     }
 
-    public function testResizedIfBoundWithMissingDataAndModifiable()
+    public function testResizedDownIfBoundWithMissingDataAndAllowDelete()
     {
         $form = $this->factory->create('collection', null, array(
             'type' => 'field',
-            'modifiable' => true,
+            'allow_delete' => true,
         ));
         $form->setData(array('foo@foo.com', 'bar@bar.com'));
         $form->bind(array('foo@bar.com'));
@@ -115,6 +101,7 @@ class CollectionFormTest extends TypeTestCase
         $this->assertTrue($form->has('0'));
         $this->assertFalse($form->has('1'));
         $this->assertEquals('foo@bar.com', $form[0]->getData());
+        $this->assertEquals(array('foo@bar.com'), $form->getData());
     }
 
     public function testNotResizedIfBoundWithExtraData()
@@ -130,11 +117,11 @@ class CollectionFormTest extends TypeTestCase
         $this->assertEquals('foo@foo.com', $form[0]->getData());
     }
 
-    public function testResizedUpIfBoundWithExtraDataAndModifiable()
+    public function testResizedUpIfBoundWithExtraDataAndAllowAdd()
     {
         $form = $this->factory->create('collection', null, array(
             'type' => 'field',
-            'modifiable' => true,
+            'allow_add' => true,
         ));
         $form->setData(array('foo@bar.com'));
         $form->bind(array('foo@foo.com', 'bar@bar.com'));
@@ -146,29 +133,14 @@ class CollectionFormTest extends TypeTestCase
         $this->assertEquals(array('foo@foo.com', 'bar@bar.com'), $form->getData());
     }
 
-    public function testModifableButNoPrototype()
+    public function testAllowAddButNoPrototype()
     {
         $form = $this->factory->create('collection', null, array(
             'type' => 'field',
-            'modifiable' => true,
+            'allow_add' => true,
             'prototype' => false,
         ));
 
         $this->assertFalse($form->has('$$name$$'));
     }
-
-    public function testResizedDownIfBoundWithLessDataAndModifiable()
-    {
-        $form = $this->factory->create('collection', null, array(
-            'type' => 'field',
-            'modifiable' => true,
-        ));
-        $form->setData(array('foo@bar.com', 'bar@bar.com'));
-        $form->bind(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());
-    }
 }