bug #18180 [Form] fixed BC break with pre selection of choices with `…

…ChoiceType` and its children (HeahDude) This PR was merged into the 2.7 branch. Discussion ---------- [Form] fixed BC break with pre selection of choices with `ChoiceType` and its children | Q | A | ------------- | --- | Branch | 2.7+ | Bugfix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18173, #14712, #17789 | License | MIT | Doc PR | - - f7eea72 reverts BC break introduced in #17760 - 58e8ed0 fixes pre selection of choice with model values such as `false`, `null` or empty string without BC break. `ChoiceType` now always use `ChoiceToValueTransformer` or `ChoicesToValuesTransformer` depending on `multiple` option. Hence `CheckboxListMapper` and `RadioListMapper` don't handle the transformation anymore. Commits ------- ea5375c [Form] refactor CheckboxListMapper and RadioListMapper 71841c7 Revert "[Form] refactor `RadioListMapper::mapDataToForm()`"
symfony · Apr 28, 2016 · a8c10ac · a8c10ac
2 parents 4aca703 + ea5375c
commit a8c10ac
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 101 deletions.
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
@@ -261,7 +261,7 @@ public function testSubmitSingleExpandedNull()
         $field->submit(null);
 
         $this->assertNull($field->getData());
-        $this->assertNull($field->getViewData());
+        $this->assertSame('', $field->getViewData(), 'View data is always a string');
     }
 
     public function testSubmitSingleNonExpandedNull()

diff --git a/src/Symfony/Bridge/Doctrine/composer.json b/src/Symfony/Bridge/Doctrine/composer.json
@@ -22,7 +22,7 @@
     "require-dev": {
         "symfony/stopwatch": "~2.2",
         "symfony/dependency-injection": "~2.2",
-        "symfony/form": "~2.7,>=2.7.1",
+        "symfony/form": "~2.7.12|~2.8.5",
         "symfony/http-kernel": "~2.2",
         "symfony/property-access": "~2.3",
         "symfony/security": "~2.2",

diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php
@@ -11,9 +11,8 @@
 
 namespace Symfony\Component\Form\Extension\Core\DataMapper;
 
-use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\DataMapperInterface;
-use Symfony\Component\Form\Exception\TransformationFailedException;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * Maps choices to/from checkbox forms.
@@ -26,16 +25,6 @@
  */
 class CheckboxListMapper implements DataMapperInterface
 {
-    /**
-     * @var ChoiceListInterface
-     */
-    private $choiceList;
-
-    public function __construct(ChoiceListInterface $choiceList)
-    {
-        $this->choiceList = $choiceList;
-    }
-
     /**
      * {@inheritdoc}
      */
@@ -46,22 +35,12 @@ public function mapDataToForms($choices, $checkboxes)
         }
 
         if (!is_array($choices)) {
-            throw new TransformationFailedException('Expected an array.');
-        }
-
-        try {
-            $valueMap = array_flip($this->choiceList->getValuesForChoices($choices));
-        } catch (\Exception $e) {
-            throw new TransformationFailedException(
-                'Can not read the choices from the choice list.',
-                $e->getCode(),
-                $e
-            );
+            throw new UnexpectedTypeException($choices, 'array');
         }
 
         foreach ($checkboxes as $checkbox) {
             $value = $checkbox->getConfig()->getOption('value');
-            $checkbox->setData(isset($valueMap[$value]) ? true : false);
+            $checkbox->setData(in_array($value, $choices, true));
         }
     }
 
@@ -70,6 +49,10 @@ public function mapDataToForms($choices, $checkboxes)
      */
     public function mapFormsToData($checkboxes, &$choices)
     {
+        if (!is_array($choices)) {
+            throw new UnexpectedTypeException($choices, 'array');
+        }
+
         $values = array();
 
         foreach ($checkboxes as $checkbox) {
@@ -79,14 +62,6 @@ public function mapFormsToData($checkboxes, &$choices)
             }
         }
 
-        try {
-            $choices = $this->choiceList->getChoicesForValues($values);
-        } catch (\Exception $e) {
-            throw new TransformationFailedException(
-                'Can not read the values from the choice list.',
-                $e->getCode(),
-                $e
-            );
-        }
+        $choices = $values;
     }
 }
diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php
@@ -11,8 +11,8 @@
 
 namespace Symfony\Component\Form\Extension\Core\DataMapper;
 
-use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
 use Symfony\Component\Form\DataMapperInterface;
+use Symfony\Component\Form\Exception\UnexpectedTypeException;
 
 /**
  * Maps choices to/from radio forms.
@@ -25,24 +25,18 @@
  */
 class RadioListMapper implements DataMapperInterface
 {
-    /**
-     * @var ChoiceListInterface
-     */
-    private $choiceList;
-
-    public function __construct(ChoiceListInterface $choiceList)
-    {
-        $this->choiceList = $choiceList;
-    }
-
     /**
      * {@inheritdoc}
      */
-    public function mapDataToForms($data, $radios)
+    public function mapDataToForms($choice, $radios)
     {
+        if (!is_string($choice)) {
+            throw new UnexpectedTypeException($choice, 'string');
+        }
+
         foreach ($radios as $radio) {
             $value = $radio->getConfig()->getOption('value');
-            $radio->setData($value === $data ? true : false);
+            $radio->setData($choice === $value);
         }
     }
 
@@ -51,6 +45,10 @@ public function mapDataToForms($data, $radios)
      */
     public function mapFormsToData($radios, &$choice)
     {
+        if (null !== $choice && !is_string($choice)) {
+            throw new UnexpectedTypeException($choice, 'null or string');
+        }
+
         $choice = null;
 
         foreach ($radios as $radio) {
@@ -59,8 +57,7 @@ public function mapFormsToData($radios, &$choice)
                     return;
                 }
 
-                $value = $radio->getConfig()->getOption('value');
-                $choice = current($this->choiceList->getChoicesForValues(array($value)));
+                $choice = $radio->getConfig()->getOption('value');
 
                 return;
             }

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
@@ -60,9 +60,7 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
         if ($options['expanded']) {
-            $builder->setDataMapper($options['multiple']
-                ? new CheckboxListMapper($options['choice_list'])
-                : new RadioListMapper($options['choice_list']));
+            $builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper());
 
             // Initialize all choices before doing the index check below.
             // This helps in cases where index checks are optimized for non
@@ -133,30 +131,13 @@ public function buildForm(FormBuilderInterface $builder, array $options)
 
                 $event->setData($data);
             });
+        }
 
-            if (!$options['multiple']) {
-                // For radio lists, transform empty arrays to null
-                // This is kind of a hack necessary because the RadioListMapper
-                // is not invoked for forms without choices
-                $builder->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) {
-                    if (array() === $event->getData()) {
-                        $event->setData(null);
-                    }
-                });
-                // For radio lists, pre selection of the choice needs to pre set data
-                // with the string value so it can be matched in
-                // {@link \Symfony\Component\Form\Extension\Core\DataMapper\RadioListMapper::mapDataToForms()}
-                $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
-                    $choiceList = $event->getForm()->getConfig()->getOption('choice_list');
-                    $value = current($choiceList->getValuesForChoices(array($event->getData())));
-                    $event->setData((string) $value);
-                });
-            }
-        } elseif ($options['multiple']) {
-            // <select> tag with "multiple" option
+        if ($options['multiple']) {
+            // <select> tag with "multiple" option or list of checkbox inputs
             $builder->addViewTransformer(new ChoicesToValuesTransformer($options['choice_list']));
         } else {
-            // <select> tag without "multiple" option
+            // <select> tag without "multiple" option or list of radio inputs
             $builder->addViewTransformer(new ChoiceToValueTransformer($options['choice_list']));
         }
 
@@ -255,7 +236,11 @@ public function configureOptions(OptionsResolver $resolver)
         $choiceListFactory = $this->choiceListFactory;
 
         $emptyData = function (Options $options) {
-            if ($options['multiple'] || $options['expanded']) {
+            if ($options['expanded'] && !$options['multiple']) {
+                return;
+            }
+
+            if ($options['multiple']) {
                 return array();
             }