Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 76e265c

Browse filesBrowse 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 76e265c
Copy full SHA for 76e265c

File tree

Expand file treeCollapse file tree

5 files changed

+76
-78
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+76
-78
lines changed
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
+1-1Lines 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()
Collapse file

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php
+8-32Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@
1111

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

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

1818
/**
1919
* Maps choices to/from checkbox forms.
@@ -26,16 +26,6 @@
2626
*/
2727
class CheckboxListMapper implements DataMapperInterface
2828
{
29-
/**
30-
* @var ChoiceListInterface
31-
*/
32-
private $choiceList;
33-
34-
public function __construct(ChoiceListInterface $choiceList)
35-
{
36-
$this->choiceList = $choiceList;
37-
}
38-
3929
/**
4030
* {@inheritdoc}
4131
*/
@@ -46,22 +36,12 @@ public function mapDataToForms($choices, $checkboxes)
4636
}
4737

4838
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-
);
39+
throw new UnexpectedTypeException($choices, 'array');
6040
}
6141

6242
foreach ($checkboxes as $checkbox) {
6343
$value = $checkbox->getConfig()->getOption('value');
64-
$checkbox->setData(isset($valueMap[$value]) ? true : false);
44+
$checkbox->setData(in_array($value, $choices, true) ? true : false);
6545
}
6646
}
6747

@@ -70,6 +50,10 @@ public function mapDataToForms($choices, $checkboxes)
7050
*/
7151
public function mapFormsToData($checkboxes, &$choices)
7252
{
53+
if (!is_array($choices)) {
54+
throw new UnexpectedTypeException($choices, 'array');
55+
}
56+
7357
$values = array();
7458

7559
foreach ($checkboxes as $checkbox) {
@@ -79,14 +63,6 @@ public function mapFormsToData($checkboxes, &$choices)
7963
}
8064
}
8165

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-
}
66+
$choices = $values;
9167
}
9268
}
Collapse file

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php
+11-16Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
+10-15Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
+46-14Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.