8000 [Form] refactor CheckboxListMapper and RadioListMapper · symfony/symfony@01b8101 · GitHub
[go: up one dir, main page]
Skip to content

Commit 01b8101

Browse files
HeahDudeHeahDude
committed
[Form] refactor CheckboxListMapper and RadioListMapper
fixes #14712 and #17789. `ChoiceType` now always use `ChoiceToValueTransformer` or `ChoicesToValuesTransformer` depending on `multiple` option. Hence `CheckboxListMapper` and `RadioListMapper` don’t handle the transformation anymore. Fixes pre selection of choice with model values such as `null`, `false` or empty string.
1 parent 4842984 commit 01b8101

File tree

5 files changed

+76
-79
lines changed

5 files changed

+76
-79
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->assertEmpty($field->getViewData());
265265
}
266266

267267
public function testSubmitSingleNonExpandedNull()

‎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) ? true : false);
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: 11 additions & 16 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,26 +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
*/
4131
public function mapDataToForms($choice, $radios)
4232
{
43-
$valueMap = array_flip($this->choiceList->getValuesForChoices(array($choice)));
33+
if (!is_string($choice)) {
34+
throw new UnexpectedTypeException($choice, 'string');
35+
}
4436

4537
foreach ($radios as $radio) {
4638
$value = $radio->getConfig()->getOption('value');
47-
$radio->setData(isset($valueMap[$value]) ? true : false);
39+
$radio->setData($choice === $value ? true : false);
4840
}
4941
}
5042

@@ -53,6 +45,10 @@ public function mapDataToForms($choice, $radios)
5345
*/
5446
public function mapFormsToData($radios, &$choice)
5547
{
48+
if (null !== $choice && !is_string($choice)) {
49+
throw new UnexpectedTypeException($choice, 'string');
50+
}
51+
5652
$choice = null;
5753

5854
foreach ($radios as $radio) {
@@ -61,10 +57,9 @@ public function mapFormsToData($radios, &$choice)
6157
return;
6258
}
6359

64-
$value = $radio->getConfig()->getOption('value');
65-
$choice = current($this->choiceList->getChoicesForValues(array($value)));
60+
$choice = $radio->getConfig()->getOption('value');
6661

67-
return;
62+
return $choice;
6863
}
6964
}
7065
}

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

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ public function buildForm(FormBuilderInterface $builder, array $options)
6161
{
6262
if ($options['expanded']) {
6363
$builder->setDataMapper($options['multiple']
64-
? new CheckboxListMapper($options['choice_list'])
65-
: new RadioListMapper($options['choice_list']));
64+
? new CheckboxListMapper()
65+
: new RadioListMapper());
6666

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

134134
$event->setData($data);
135135
});
136+
}
136137

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-
}
147-
} elseif ($options['multiple']) {
138+
if ($options['multiple']) {
148139
// <select> tag with "multiple" option
149140
$builder->addViewTransformer(new ChoicesToValuesTransformer($options['choice_list']));
150141
} else {
151-
// <select> tag without "multiple" option
142+
// <select> tag without "multiple" option or list of radio inputs
152143
$builder->addViewTransformer(new ChoiceToValueTransformer($options['choice_list']));
153144
}
154145

@@ -247,7 +238,11 @@ public function configureOptions(OptionsResolver $resolver)
247238
$choiceListFactory = $this->choiceListFactory;
248239

249240
$emptyData = function (Options $options) {
250-
if ($options['multiple'] || $options['expanded']) {
241+
if ($options['expanded'] && !$options['multiple']) {
242+
return;
243+
}
244+
245+
if ($options['multiple']) {
251246
return array();
252247
}
253248

‎src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
3232
'n/a' => '',
3333
);
3434

35+
private $booleanChoicesWithNull = array(
36+
'Yes' => true,
37+
'No' => false,
38+
'n/a' => null,
39+
);
40+
3541
private $numericChoicesFlipped = array(
3642
0 => 'Bernhard',
3743
1 => 'Fabien',
@@ -183,6 +189,19 @@ public function testExpandedChoiceListWithScalarValues()
183189
$this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
184190
}
185191

192+
public function testExpandedChoiceListWithBooleanAndNullValues()
193+
{
194+
$view = $this->factory->create('choice', null, array(
195+
'choices' => $this->booleanChoicesWithNull,
196+
'choices_as_values' => true,
197+
'expanded' => true,
198+
))->createView();
199+
200+
$this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
201+
$this->assertFalse($view->children[1]->vars['checked'], 'False value should not be pre selected');
202+
$this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
203+
}
204+
186205
public function testExpandedChoiceListWithScalarValuesAndFalseAsPreSetData()
187206
{
188207
$view = $this->factory->create('choice', false, array(
@@ -197,6 +216,19 @@ public function testExpandedChoiceListWithScalarValuesAndFalseAsPreSetData()
197216
$this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
198217
}
199218

219+
public function testExpandedChoiceListWithBooleanAndNullValuesAndFalseAsPreSetData()
220+
{
221+
$view = $this->factory->create('choice', false, array(
222+
'choices' => $this->booleanChoicesWithNull,
223+
'choices_as_values' => true,
224+
'expanded' => true,
225+
))->createView();
226+
227+
$this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
228+
$this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
229+
$this->assertFalse($view->children[2]->vars['checked'], 'Null value should not be pre selected');
230+
}
231+
200232
public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
201233
{
202234
$form = $this->factory->create('choice', null, array(
@@ -319,7 +351,7 @@ public function testPlaceholderWithExpandedBooleanChoices()
319351

320352
$view = $form->createView();
321353

322-
$this->assertSame('', $view->vars['value'], 'Value should be empty');
354+
$this->assertEmpty($view->vars['value'], 'Value should be empty');
323355
$this->assertSame('1', $view->vars['choices'][0]->value);
324356
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
325357
$this->assertFalse($view->children[1]->vars['checked'], 'Choice "false" should not be selected');
@@ -940,7 +972,7 @@ public function testSubmitSingleExpandedRequiredNull()
940972
$form->submit(null);
941973

942974
$this->assertNull($form->getData());
943-
$this->assertNull($form->getViewData());
975+
$this->assertEmpty($form->getViewData());
944976
$this->assertEmpty($form->getExtraData());
945977
$this->assertTrue($form->isSynchronized());
946978

@@ -972,7 +1004,7 @@ public function testSubmitSingleExpandedRequiredNullNoChoices()
9721004
$form->submit(null);
9731005

9741006
$this->assertNull($form->getData());
975-
$this->assertNull($form->getViewData());
1007+
$this->assertEmpty($form->getViewData());
9761008
$this->assertEmpty($form->getExtraData());
9771009
$this->assertTrue($form->isSynchronized());
9781010
}
@@ -990,7 +1022,7 @@ public function testSubmitSingleExpandedRequiredEmpty()
9901022
$form->submit('');
9911023

9921024
$this->assertNull($form->getData());
993-
$this->assertNull($form->getViewData());
1025+
$this->assertEmpty($form->getViewData());
9941026
$this->assertEmpty($form->getExtraData());
9951027
$this->assertTrue($form->isSynchronized());
9961028

@@ -1022,7 +1054,7 @@ public function testSubmitSingleExpandedRequiredEmptyNoChoices()
10221054
$form->submit('');
10231055

10241056
$this->assertNull($form->getData());
1025-
$this->assertNull($form->getViewData());
1057+
$this->assertEmpty($form->getViewData());
10261058
$this->assertEmpty($form->getExtraData());
10271059
$this->assertTrue($form->isSynchronized());
10281060
}
@@ -1040,7 +1072,7 @@ public function testSubmitSingleExpandedRequiredFalse()
10401072
$form->submit(false);
10411073

10421074
$this->assertNull($form->getData());
1043-
$this->assertNull($form->getViewData());
1075+
$this->assertEmpty($form->getViewData());
10441076
$this->assertEmpty($form->getExtraData());
10451077
$this->assertTrue($form->isSynchronized());
10461078

@@ -1072,7 +1104,7 @@ public function testSubmitSingleExpandedRequiredFalseNoChoices()
10721104
$form->submit(false);
10731105

10741106
$this->assertNull($form->getData());
1075-
$this->assertNull($form->getViewData());
1107+
$this->assertEmpty($form->getViewData());
10761108
$this->assertEmpty($form->getExtraData());
10771109
$this->assertTrue($form->isSynchronized());
10781110
}
@@ -1090,7 +1122,7 @@ public function testSubmitSingleExpandedNonRequiredNull()
10901122
$form->submit(null);
10911123

10921124
$this->assertNull($form->getData());
1093-
$this->assertNull($form->getViewData());
1125+
$this->assertEmpty($form->getViewData());
10941126
$this->assertEmpty($form->getExtraData());
10951127
$this->assertTrue($form->isSynchronized());
10961128

@@ -1124,7 +1156,7 @@ public function testSubmitSingleExpandedNonRequiredNullNoChoices()
11241156
$form->submit(null);
11251157

11261158
$this->assertNull($form->getData());
1127-
$this->assertNull($form->getViewData());
1159+
$this->assertEmpty($form->getViewData());
11281160
$this->assertEmpty($form->getExtraData());
11291161
$this->assertTrue($form->isSynchronized());
11301162
}
@@ -1142,7 +1174,7 @@ public function testSubmitSingleExpandedNonRequiredEmpty()
11421174
$form->submit('');
11431175

11441176
$this->assertNull($form->getData());
1145-
$this->assertNull($form->getViewData());
1177+
$this->assertEmpty($form->getViewData());
11461178
$this->assertEmpty($form->getExtraData());
11471179
$this->assertTrue($form->isSynchronized());
11481180

@@ -1176,7 +1208,7 @@ public function testSubmitSingleExpandedNonRequiredEmptyNoChoices()
11761208
$form->submit('');
11771209

11781210
$this->assertNull($form->getData());
1179-
$this->assertNull($form->getViewData());
1211+
$this->assertEmpty($form->getViewData());
11801212
$this->assertEmpty($form->getExtraData());
11811213
$this->assertTrue($form->isSynchronized());
11821214
}
@@ -1194,7 +1226,7 @@ public function testSubmitSingleExpandedNonRequiredFalse()
11941226
$form->submit(false);
11951227

11961228
$this->assertNull($form->getData());
1197-
$this->assertNull($form->getViewData());
1229+
$this->assertEmpty($form->getViewData());
11981230
$this->assertEmpty($form->getExtraData());
11991231
$this->assertTrue($form->isSynchronized());
12001232

@@ -1228,7 +1260,7 @@ public function testSubmitSingleExpandedNonRequiredFalseNoChoices()
12281260
$form->submit(false);
12291261

12301262
$this->assertNull($form->getData());
1231-
$this->assertNull($form->getViewData());
1263+
$this->assertEmpty($form->getViewData());
12321264
$this->assertEmpty($form->getExtraData());
12331265
$this->assertTrue($form->isSynchronized());
12341266
}
@@ -1247,7 +1279,7 @@ public function testSubmitSingleExpandedWithEmptyChild()
12471279

12481280
$form->submit('');
12491281

1250-
$this->assertNull($form->getData());
1282+
$this->assertEmpty($form->getData());
12511283
$this->assertTrue($form->isSynchronized());
12521284

12531285
$this->assertTrue($form[0]->getData());

0 commit comments

Comments
 (0)
0