Skip to content

Commit f17c483

Browse files
HeahDudeHeahDude
committed
[Form] fix choice value "false" in ChoiceType
1 parent 458abee commit f17c483

File tree

6 files changed

+230
-17
lines changed

6 files changed

+230
-17
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/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));

‎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
/**

‎Tests/Extension/Core/Type/ChoiceTypeTest.php‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
2626
'Roman' => 'e',
2727
);
2828

29+
private $scalarChoices = array(
30+
'Yes' => true,
31+
'No' => false,
32+
'n/a' => '',
33+
);
34+
2935
private $numericChoicesFlipped = array(
3036
0 => 'Bernhard',
3137
1 => 'Fabien',
@@ -139,6 +145,58 @@ public function testExpandedFlippedChoicesOptionsTurnIntoChildren()
139145
$this->assertCount(count($this->choices), $form, 'Each choice should become a new field');
140146
}
141147

148+
public function testChoiceListWithScalarValues()
149+
{
150+
$view = $this->factory->create('choice', null, array(
151+
'choices' => $this->scalarChoices,
152+
'choices_as_values' => true,
153+
))->createView();
154+
155+
$this->assertSame('1', $view->vars['choices'][0]->value);
156+
$this->assertSame('0', $view->vars['choices'][1]->value);
157+
$this->assertSame('', $view->vars['choices'][2]->value);
158+
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][0], $view->vars['value']), 'True value should not be pre selected');
159+
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][1], $view->vars['value']), 'False value should not be pre selected');
160+
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][2], $view->vars['value']), 'Empty value should not be pre selected');
161+
}
162+
163+
public function testChoiceListWithScalarValuesAndFalseAsPreSetData()
164+
{
165+
$view = $this->factory->create('choice', false, array(
166+
'choices' => $this->scalarChoices,
167+
'choices_as_values' => true,
168+
))->createView();
169+
170+
$this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'False value should be pre selected');
171+
}
172+
173+
public function testExpandedChoiceListWithScalarValues()
174+
{
175+
$view = $this->factory->create('choice', null, array(
176+
'choices' => $this->scalarChoices,
177+
'choices_as_values' => true,
178+
'expanded' => true,
179+
))->createView();
180+
181+
$this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
182+
$this->assertFalse($view->children[1]->vars['checked'], 'False value should not be pre selected');
183+
$this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
184+
}
185+
186+
public function testExpandedChoiceListWithScalarValuesAndFalseAsPreSetData()
187+
{
188+
$view = $this->factory->create('choice', false, array(
189+
'choices' => $this->scalarChoices,
190+
'choices_as_values' => true,
191+
'expanded' => true,
192+
))->createView();
193+
194+
$this->assertSame('1', $view->vars['choices'][0]->value);
195+
$this->assertSame('0', $view->vars['choices'][1]->value);
196+
$this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
197+
$this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
198+
}
199+
142200
public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
143201
{
144202
$form = $this->factory->create('choice', null, array(
@@ -198,6 +256,100 @@ public function testPlaceholderNotPresentIfEmptyChoice()
198256
$this->assertCount(2, $form, 'Each choice should become a new field');
199257
}
200258

259+
public function testPlaceholderWithBooleanChoices()
260+
{
261+
$form = $this->factory->create('choice', null, array(
262+
'multiple' => false,
263+
'expanded' => false,
264+
'required' => false,
265+
'choices' => array(
266+
'Yes' => true,
267+
'No' => false,
268+
),
269+
'placeholder' => 'Select an option',
270+
'choices_as_values' => true,
271+
));
272+
273+
$view = $form->createView();
274+
275+
$this->assertSame('', $view->vars['value'], 'Value should be empty');
276+
$this->assertSame('1', $view->vars['choices'][0]->value);
277+
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
278+
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should not be selected');
279+
}
280+
281+
public function testPlaceholderWithBooleanChoicesWithFalseAsPreSetData()
282+
{
283+
$form = $this->factory->create('choice', false, array(
284+
'multiple' => false,
285+
'expanded' => false,
286+
'required' => false,
287+
'choices' => array(
288+
'Yes' => true,
289+
'No' => false,
290+
),
291+
'placeholder' => 'Select an option',
292+
'choices_as_values' => true,
293+
));
294+
295+
$view = $form->createView();
296+
297+
$this->assertSame('0', $view->vars['value'], 'Value should be "0"');
298+
$this->assertSame('1', $view->vars['choices'][0]->value);
299+
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
300+
$this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should be selected');
301+
}
302+
303+
public function testPlaceholderWithExpandedBooleanChoices()
304+
{
305+
$form = $this->factory->create('choice', null, array(
306+
'multiple' => false,
307+
'expanded' => true,
308+
'required' => false,
309+
'choices' => array(
310+
'Yes' => true,
311+
'No' => false,
312+
),
313+
'placeholder' => 'Select an option',
314+
'choices_as_values' => true,
315+
));
316+
317+
$this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
318+
$this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');
319+
320+
$view = $form->createView();
321+
322+
$this->assertSame('', $view->vars['value'], 'Value should be empty');
323+
$this->assertSame('1', $view->vars['choices'][0]->value);
324+
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
325+
$this->assertFalse($view->children[1]->vars['checked'], 'Choice "false" should not be selected');
326+
}
327+
328+
public function testPlaceholderWithExpandedBooleanChoicesAndWithFalseAsPreSetData()
329+
{
330+
$form = $this->factory->create('choice', false, array(
331+
'multiple' => false,
332+
'expanded' => true,
333+
'required' => false,
334+
'choices' => array(
335+
'Yes' => true,
336+
'No' => false,
337+
),
338+
'placeholder' => 'Select an option',
339+
'choices_as_values' => true,
340+
));
341+
342+
$this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
343+
$this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');
344+
345+
$view = $form->createView();
346+
347+
$this->assertSame('0', $view->vars['value'], 'Value should be "0"');
348+
$this->assertSame('1', $view->vars['choices'][0]->value);
349+
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
350+
$this->assertTrue($view->children[1]->vars['checked'], 'Choice "false" should be selected');
351+
}
352+
201353
public function testExpandedChoicesOptionsAreFlattened()
202354
{
203355
$form = $this->factory->create('choice', null, array(

0 commit comments

Comments
 (0)