Skip to content

Commit a8c10ac

Browse files
fabpotfabpot
committed
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()`"
2 parents 4aca703 + ea5375c commit a8c10ac

File tree

6 files changed

+90
-101
lines changed

6 files changed

+90
-101
lines changed

‎src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ public function testSubmitSingleExpandedNull()
261261
$field->submit(null);
262262

263263
$this->assertNull($field->getData());
264-
$this->assertNull($field->getViewData());
264+
$this->assertSame('', $field->getViewData(), 'View data is always a string');
265265
}
266266

267267
public function testSubmitSingleNonExpandedNull()

‎src/Symfony/Bridge/Doctrine/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"require-dev": {
2323
"symfony/stopwatch": "~2.2",
2424
"symfony/dependency-injection": "~2.2",
25-
"symfony/form": "~2.7,>=2.7.1",
25+
"symfony/form": "~2.7.12|~2.8.5",
2626
"symfony/http-kernel": "~2.2",
2727
"symfony/property-access": "~2.3",
2828
"symfony/security": "~2.2",

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

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@
1111

1212
namespace Symfony\Component\Form\Extension\Core\DataMapper;
1313

14-
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
1514
use Symfony\Component\Form\DataMapperInterface;
16-
use Symfony\Component\Form\Exception\TransformationFailedException;
15+
use Symfony\Component\Form\Exception\UnexpectedTypeException;
1716

1817
/**
1918
* Maps choices to/from checkbox forms.
@@ -26,16 +25,6 @@
2625
*/
2726
class CheckboxListMapper implements DataMapperInterface
2827
{
29-
/**
30-
* @var ChoiceListInterface
31-
*/
32-
private $choiceList;
33-
34-
public function __construct(ChoiceListInterface $choiceList)
35-
{
36-
$this->choiceList = $choiceList;
37-
}
38-
3928
/**
4029
* {@inheritdoc}
4130
*/
@@ -46,22 +35,12 @@ public function mapDataToForms($choices, $checkboxes)
4635
}
4736

4837
if (!is_array($choices)) {
49-
throw new TransformationFailedException('Expected an array.');
50-
}
51-
52-
try {
53-
$valueMap = array_flip($this->choiceList->getValuesForChoices($choices));
54-
} catch (\Exception $e) {
55-
throw new TransformationFailedException(
56-
'Can not read the choices from the choice list.',
57-
$e->getCode(),
58-
$e
59-
);
38+
throw new UnexpectedTypeException($choices, 'array');
6039
}
6140

6241
foreach ($checkboxes as $checkbox) {
6342
$value = $checkbox->getConfig()->getOption('value');
64-
$checkbox->setData(isset($valueMap[$value]) ? true : false);
43+
$checkbox->setData(in_array($value, $choices, true));
6544
}
6645
}
6746

@@ -70,6 +49,10 @@ public function mapDataToForms($choices, $checkboxes)
7049
*/
7150
public function mapFormsToData($checkboxes, &$choices)
7251
{
52+
if (!is_array($choices)) {
53+
throw new UnexpectedTypeException($choices, 'array');
54+
}
55+
7356
$values = array();
7457

7558
foreach ($checkboxes as $checkbox) {
@@ -79,14 +62,6 @@ public function mapFormsToData($checkboxes, &$choices)
7962
}
8063
}
8164

82-
try {
83-
$choices = $this->choiceList->getChoicesForValues($values);
84-
} catch (\Exception $e) {
85-
throw new TransformationFailedException(
86-
'Can not read the values from the choice list.',
87-
$e->getCode(),
88-
$e
89-
);
90-
}
65+
$choices = $values;
9166
}
9267
}

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
namespace Symfony\Component\Form\Extension\Core\DataMapper;
1313

14-
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
1514
use Symfony\Component\Form\DataMapperInterface;
15+
use Symfony\Component\Form\Exception\UnexpectedTypeException;
1616

1717
/**
1818
* Maps choices to/from radio forms.
@@ -25,24 +25,18 @@
2525
*/
2626
class RadioListMapper implements DataMapperInterface
2727
{
28-
/**
29-
* @var ChoiceListInterface
30-
*/
31-
private $choiceList;
32-
33-
public function __construct(ChoiceListInterface $choiceList)
34-
{
35-
$this->choiceList = $choiceList;
36-
}
37-
3828
/**
3929
* {@inheritdoc}
4030
*/
41-
public function mapDataToForms($data, $radios)
31+
public function mapDataToForms($choice, $radios)
4232
{
33+
if (!is_string($choice)) {
34+
throw new UnexpectedTypeException($choice, 'string');
35+
}
36+
4337
foreach ($radios as $radio) {
4438
$value = $radio->getConfig()->getOption('value');
45-
$radio->setData($value === $data ? true : false);
39+
$radio->setData($choice === $value);
4640
}
4741
}
4842

@@ -51,6 +45,10 @@ public function mapDataToForms($data, $radios)
5145
*/
5246
public function mapFormsToData($radios, &$choice)
5347
{
48+
if (null !== $choice && !is_string($choice)) {
49+
throw new UnexpectedTypeException($choice, 'null or string');
50+
}
51+
5452
$choice = null;
5553

5654
foreach ($radios as $radio) {
@@ -59,8 +57,7 @@ public function mapFormsToData($radios, &$choice)
5957
return;
6058
}
6159

62-
$value = $radio->getConfig()->getOption('value');
63-
$choice = current($this->choiceList->getChoicesForValues(array($value)));
60+
$choice = $radio->getConfig()->getOption('value');
6461

6562
return;
6663
}

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ public function __construct(ChoiceListFactoryInterface $choiceListFactory = null
6060
public function buildForm(FormBuilderInterface $builder, array $options)
6161
{
6262
if ($options['expanded']) {
63-
$builder->setDataMapper($options['multiple']
64-
? new CheckboxListMapper($options['choice_list'])
65-
: new RadioListMapper($options['choice_list']));
63+
$builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper());
6664

6765
// Initialize all choices before doing the index check below.
6866
// This helps in cases where index checks are optimized for non
@@ -133,30 +131,13 @@ public function buildForm(FormBuilderInterface $builder, array $options)
133131

134132
$event->setData($data);
135133
});
134+
}
136135

137-
if (!$options['multiple']) {
138-
// For radio lists, transform empty arrays to null
139-
// This is kind of a hack necessary because the RadioListMapper
140-
// is not invoked for forms without choices
141-
$builder->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) {
142-
if (array() === $event->getData()) {
143-
$event->setData(null);
144-
}
145-
});
146-
// For radio lists, pre selection of the choice needs to pre set data
147-
// with the string value so it can be matched in
148-
// {@link \Symfony\Component\Form\Extension\Core\DataMapper\RadioListMapper::mapDataToForms()}
149-
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
150-
$choiceList = $event->getForm()->getConfig()->getOption('choice_list');
151-
$value = current($choiceList->getValuesForChoices(array($event->getData())));
152-
$event->setData((string) $value);
153-
});
154-
}
155-
} elseif ($options['multiple']) {
156-
// <select> tag with "multiple" option
136+
if ($options['multiple']) {
137+
// <select> tag with "multiple" option or list of checkbox inputs
157138
$builder->addViewTransformer(new ChoicesToValuesTransformer($options['choice_list']));
158139
} else {
159-
// <select> tag without "multiple" option
140+
// <select> tag without "multiple" option or list of radio inputs
160141
$builder->addViewTransformer(new ChoiceToValueTransformer($options['choice_list']));
161142
}
162143

@@ -255,7 +236,11 @@ public function configureOptions(OptionsResolver $resolver)
255236
$choiceListFactory = $this->choiceListFactory;
256237

257238
$emptyData = function (Options $options) {
258-
if ($options['multiple'] || $options['expanded']) {
239+
if ($options['expanded'] && !$options['multiple']) {
240+
return;
241+
}
242+
243+
if ($options['multiple']) {
259244
return array();
260245
}
261246

0 commit comments

Comments
 (0)