Skip to content

Commit 3becf7f

Browse files
fabpotfabpot
committed
bug #17760 [2.7] [Form] fix choice value "false" in ChoiceType (HeahDude)
This PR was merged into the 2.7 branch. Discussion ---------- [2.7] [Form] fix choice value "false" in ChoiceType | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #17292, #14712, #17789 | License | MIT | Doc PR | - - [x] Add tests for choices with `boolean` and `null` values, and with a placeholder - [x] Fix FQCN in 2.8 tests, see #17759 - [x] Remove `choices_as_values` in 3.0 tests, see #17886 Commits ------- 8f918e5 [Form] refactor `RadioListMapper::mapDataToForm()` 3eac469 [Form] fix choice value "false" in ChoiceType
2 parents 6e38118 + f6e39c4 commit 3becf7f

File tree

8 files changed

+240
-21
lines changed

8 files changed

+240
-21
lines changed

‎ChoiceList/ArrayChoiceList.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public function __construct($choices, $value = null)
7676

7777
if (null === $value && $this->castableToString($choices)) {
7878
$value = function ($choice) {
79-
return (string) $choice;
79+
return false === $choice ? '0' : (string) $choice;
8080
};
8181
}
8282

@@ -235,11 +235,11 @@ private function castableToString(array $choices, array &$cache = array())
235235
continue;
236236
} elseif (!is_scalar($choice)) {
237237
return false;
238-
} elseif (isset($cache[(string) $choice])) {
238+
} elseif (isset($cache[$choice])) {
239239
return false;
240240
}
241241

242-
$cache[(string) $choice] = true;
242+
$cache[$choice] = true;
243243
}
244244

245245
return true;

‎Extension/Core/DataMapper/RadioListMapper.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,11 @@ public function __construct(ChoiceListInterface $choiceList)
3838
/**
3939
* {@inheritdoc}
4040
*/
41-
public function mapDataToForms($choice, $radios)
41+
public function mapDataToForms($data, $radios)
4242
{
43-
$valueMap = array_flip($this->choiceList->getValuesForChoices(array($choice)));
44-
4543
foreach ($radios as $radio) {
4644
$value = $radio->getConfig()->getOption('value');
47-
$radio->setData(isset($valueMap[$value]) ? true : false);
45+
$radio->setData($value === $data ? true : false);
4846
}
4947
}
5048

‎Extension/Core/DataTransformer/ChoiceToValueTransformer.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public function transform($choice)
3939

4040
public function reverseTransform($value)
4141
{
42-
if (null !== $value && !is_scalar($value)) {
43-
throw new TransformationFailedException('Expected a scalar.');
42+
if (null !== $value && !is_string($value)) {
43+
throw new TransformationFailedException('Expected a string or null.');
4444
}
4545

4646
$choices = $this->choiceList->getChoicesForValues(array((string) $value));

‎Extension/Core/Type/ChoiceType.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,14 @@ public function buildForm(FormBuilderInterface $builder, array $options)
143143
$event->setData(null);
144144
}
145145
});
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+
});
146154
}
147155
} elseif ($options['multiple']) {
148156
// <select> tag with "multiple" option

‎Tests/ChoiceList/ArrayChoiceListTest.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,29 @@ public function testGetChoicesForValuesWithContainingNull()
137137

138138
$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('0')));
139139
}
140+
141+
public function testGetChoicesForValuesWithContainingFalseAndNull()
142+
{
143+
$choiceList = new ArrayChoiceList(array('False' => false, 'Null' => null));
144+
145+
$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
146+
$this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
147+
}
148+
149+
public function testGetChoicesForValuesWithContainingEmptyStringAndNull()
150+
{
151+
$choiceList = new ArrayChoiceList(array('Empty String' => '', 'Null' => null));
152+
153+
$this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('0')));
154+
$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
155+
}
156+
157+
public function testGetChoicesForValuesWithContainingEmptyStringAndBooleans()
158+
{
159+
$choiceList = new ArrayChoiceList(array('Empty String' => '', 'True' => true, 'False' => false));
160+
161+
$this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('')));
162+
$this->assertSame(array(0 => true), $choiceList->getChoicesForValues(array('1')));
163+
$this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
164+
}
140165
}

‎Tests/Extension/Core/DataTransformer/ChoiceToValueTransformerTest.php‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,60 +17,80 @@
1717
class ChoiceToValueTransformerTest extends \PHPUnit_Framework_TestCase
1818
{
1919
protected $transformer;
20+
protected $transformerWithNull;
2021

2122
protected function setUp()
2223
{
23-
$list = new ArrayChoiceList(array('', false, 'X'));
24+
$list = new ArrayChoiceList(array('', false, 'X', true));
25+
$listWithNull = new ArrayChoiceList(array('', false, 'X', null));
2426

2527
$this->transformer = new ChoiceToValueTransformer($list);
28+
$this->transformerWithNull = new ChoiceToValueTransformer($listWithNull);
2629
}
2730

2831
protected function tearDown()
2932
{
3033
$this->transformer = null;
34+
$this->transformerWithNull = null;
3135
}
3236

3337
public function transformProvider()
3438
{
3539
return array(
3640
// more extensive test set can be found in FormUtilTest
37-
array('', '0'),
38-
array(false, '1'),
41+
array('', '', '', '0'),
42+
array(false, '0', false, '1'),
43+
array('X', 'X', 'X', '2'),
44+
array(true, '1', null, '3'),
3945
);
4046
}
4147

4248
/**
4349
* @dataProvider transformProvider
4450
*/
45-
public function testTransform($in, $out)
51+
public function testTransform($in, $out, $inWithNull, $outWithNull)
4652
{
4753
$this->assertSame($out, $this->transformer->transform($in));
54+
$this->assertSame($outWithNull, $this->transformerWithNull->transform($inWithNull));
4855
}
4956

5057
public function reverseTransformProvider()
5158
{
5259
return array(
5360
// values are expected to be valid choice keys already and stay
5461
// the same
55-
array('0', ''),
56-
array('1', false),
57-
array('2', 'X'),
62+
array('', '', '0', ''),
63+
array('0', false, '1', false),
64+
array('X', 'X', '2', 'X'),
65+
array('1', true, '3', null),
5866
);
5967
}
6068

6169
/**
6270
* @dataProvider reverseTransformProvider
6371
*/
64-
public function testReverseTransform($in, $out)
72+
public function testReverseTransform($in, $out, $inWithNull, $outWithNull)
6573
{
6674
$this->assertSame($out, $this->transformer->reverseTransform($in));
75+
$this->assertSame($outWithNull, $this->transformerWithNull->reverseTransform($inWithNull));
76+
}
77+
78+
public function reverseTransformExpectsStringOrNullProvider()
79+
{
80+
return array(
81+
array(0),
82+
array(true),
83+
array(false),
84+
array(array()),
85+
);
6786
}
6887

6988
/**
89+
* @dataProvider reverseTransformExpectsStringOrNullProvider
7090
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
7191
*/
72-
public function testReverseTransformExpectsScalar()
92+
public function testReverseTransformExpectsStringOrNull($value)
7393
{
74-
$this->transformer->reverseTransform(array());
94+
$this->transformer->reverseTransform($value);
7595
}
7696
}

‎Tests/Extension/Core/DataTransformer/ChoicesToValuesTransformerTest.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,24 +17,34 @@
1717
class ChoicesToValuesTransformerTest extends \PHPUnit_Framework_TestCase
1818
{
1919
protected $transformer;
20+
protected $transformerWithNull;
2021

2122
protected function setUp()
2223
{
2324
$list = new ArrayChoiceList(array('', false, 'X'));
25+
$listWithNull = new ArrayChoiceList(array('', false, 'X', null));
26+
2427
$this->transformer = new ChoicesToValuesTransformer($list);
28+
$this->transformerWithNull = new ChoicesToValuesTransformer($listWithNull);
2529
}
2630

2731
protected function tearDown()
2832
{
2933
$this->transformer = null;
34+
$this->transformerWithNull = null;
3035
}
3136

3237
public function testTransform()
3338
{
3439
$in = array('', false, 'X');
35-
$out = array('0', '1', '2');
40+
$out = array('', '0', 'X');
3641

3742
$this->assertSame($out, $this->transformer->transform($in));
43+
44+
$in[] = null;
45+
$outWithNull = array('0', '1', '2', '3');
46+
47+
$this->assertSame($outWithNull, $this->transformerWithNull->transform($in));
3848
}
3949

4050
public function testTransformNull()
@@ -53,15 +63,21 @@ public function testTransformExpectsArray()
5363
public function testReverseTransform()
5464
{
5565
// values are expected to be valid choices and stay the same
56-
$in = array('0', '1', '2');
66+
$in = array('', '0', 'X');
5767
$out = array('', false, 'X');
5868

5969
$this->assertSame($out, $this->transformer->reverseTransform($in));
70+
// values are expected to be valid choices and stay the same
71+
$inWithNull = array('0','1','2','3');
72+
$out[] = null;
73+
74+
$this->assertSame($out, $this->transformerWithNull->reverseTransform($inWithNull));
6075
}
6176

6277
public function testReverseTransformNull()
6378
{
6479
$this->assertSame(array(), $this->transformer->reverseTransform(null));
80+
$this->assertSame(array(), $this->transformerWithNull->reverseTransform(null));
6581
}
6682

6783
/**

0 commit comments

Comments
 (0)