Skip to content

Commit

Permalink
[Form] fix choice value "false" in ChoiceType
Browse files Browse the repository at this point in the history
  • Loading branch information
@HeahDude
HeahDude committed Feb 22, 2016
1 parent ae0b5fa commit 3eac469
Show file tree
Hide file tree
Showing 6 changed files with 230 additions and 17 deletions.
6 changes: 3 additions & 3 deletions src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php
View file
Expand Up @@ -76,7 +76,7 @@ public function __construct($choices, $value = null)

if (null === $value && $this->castableToString($choices)) {
$value = function ($choice) {
return (string) $choice;
return false === $choice ? '0' : (string) $choice;
};
}

Expand Down Expand Up @@ -235,11 +235,11 @@ private function castableToString(array $choices, array &$cache = array())
continue;
} elseif (!is_scalar($choice)) {
return false;
} elseif (isset($cache[(string) $choice])) {
} elseif (isset($cache[$choice])) {
return false;
}

$cache[(string) $choice] = true;
$cache[$choice] = true;
}

return true;
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoiceToValueTransformer.php
View file
Expand Up @@ -39,8 +39,8 @@ public function transform($choice)

public function reverseTransform($value)
{
if (null !== $value && !is_scalar($value)) {
throw new TransformationFailedException('Expected a scalar.');
if (null !== $value && !is_string($value)) {
throw new TransformationFailedException('Expected a string or null.');
}

$choices = $this->choiceList->getChoicesForValues(array((string) $value));
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php
View file
Expand Up @@ -137,4 +137,29 @@ public function testGetChoicesForValuesWithContainingNull()

$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('0')));
}

public function testGetChoicesForValuesWithContainingFalseAndNull()
{
$choiceList = new ArrayChoiceList(array('False' => false, 'Null' => null));

$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
$this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
}

public function testGetChoicesForValuesWithContainingEmptyStringAndNull()
{
$choiceList = new ArrayChoiceList(array('Empty String' => '', 'Null' => null));

$this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('0')));
$this->assertSame(array(0 => null), $choiceList->getChoicesForValues(array('1')));
}

public function testGetChoicesForValuesWithContainingEmptyStringAndBooleans()
{
$choiceList = new ArrayChoiceList(array('Empty String' => '', 'True' => true, 'False' => false));

$this->assertSame(array(0 => ''), $choiceList->getChoicesForValues(array('')));
$this->assertSame(array(0 => true), $choiceList->getChoicesForValues(array('1')));
$this->assertSame(array(0 => false), $choiceList->getChoicesForValues(array('0')));
}
}
40 changes: 30 additions & 10 deletions ...fony/Component/Form/Tests/Extension/Core/DataTransformer/ChoiceToValueTransformerTest.php
View file
Expand Up @@ -17,60 +17,80 @@
class ChoiceToValueTransformerTest extends \PHPUnit_Framework_TestCase
{
protected $transformer;
protected $transformerWithNull;

protected function setUp()
{
$list = new ArrayChoiceList(array('', false, 'X'));
$list = new ArrayChoiceList(array('', false, 'X', true));
$listWithNull = new ArrayChoiceList(array('', false, 'X', null));

$this->transformer = new ChoiceToValueTransformer($list);
$this->transformerWithNull = new ChoiceToValueTransformer($listWithNull);
}

protected function tearDown()
{
$this->transformer = null;
$this->transformerWithNull = null;
}

public function transformProvider()
{
return array(
// more extensive test set can be found in FormUtilTest
array('', '0'),
array(false, '1'),
array('', '', '', '0'),
array(false, '0', false, '1'),
array('X', 'X', 'X', '2'),
array(true, '1', null, '3'),
);
}

/**
* @dataProvider transformProvider
*/
public function testTransform($in, $out)
public function testTransform($in, $out, $inWithNull, $outWithNull)
{
$this->assertSame($out, $this->transformer->transform($in));
$this->assertSame($outWithNull, $this->transformerWithNull->transform($inWithNull));
}

public function reverseTransformProvider()
{
return array(
// values are expected to be valid choice keys already and stay
// the same
array('0', ''),
array('1', false),
array('2', 'X'),
array('', '', '0', ''),
array('0', false, '1', false),
array('X', 'X', '2', 'X'),
array('1', true, '3', null),
);
}

/**
* @dataProvider reverseTransformProvider
*/
public function testReverseTransform($in, $out)
public function testReverseTransform($in, $out, $inWithNull, $outWithNull)
{
$this->assertSame($out, $this->transformer->reverseTransform($in));
$this->assertSame($outWithNull, $this->transformerWithNull->reverseTransform($inWithNull));
}

public function reverseTransformExpectsStringOrNullProvider()
{
return array(
array(0),
array(true),
array(false),
array(array()),
);
}

/**
* @dataProvider reverseTransformExpectsStringOrNullProvider
* @expectedException \Symfony\Component\Form\Exception\TransformationFailedException
*/
public function testReverseTransformExpectsScalar()
public function testReverseTransformExpectsStringOrNull($value)
{
$this->transformer->reverseTransform(array());
$this->transformer->reverseTransform($value);
}
}
20 changes: 18 additions & 2 deletions ...ny/Component/Form/Tests/Extension/Core/DataTransformer/ChoicesToValuesTransformerTest.php
View file
Expand Up @@ -17,24 +17,34 @@
class ChoicesToValuesTransformerTest extends \PHPUnit_Framework_TestCase
{
protected $transformer;
protected $transformerWithNull;

protected function setUp()
{
$list = new ArrayChoiceList(array('', false, 'X'));
$listWithNull = new ArrayChoiceList(array('', false, 'X', null));

$this->transformer = new ChoicesToValuesTransformer($list);
$this->transformerWithNull = new ChoicesToValuesTransformer($listWithNull);
}

protected function tearDown()
{
$this->transformer = null;
$this->transformerWithNull = null;
}

public function testTransform()
{
$in = array('', false, 'X');
$out = array('0', '1', '2');
$out = array('', '0', 'X');

$this->assertSame($out, $this->transformer->transform($in));

$in[] = null;
$outWithNull = array('0', '1', '2', '3');

$this->assertSame($outWithNull, $this->transformerWithNull->transform($in));
}

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

$this->assertSame($out, $this->transformer->reverseTransform($in));
// values are expected to be valid choices and stay the same
$inWithNull = array('0','1','2','3');
$out[] = null;

$this->assertSame($out, $this->transformerWithNull->reverseTransform($inWithNull));
}

public function testReverseTransformNull()
{
$this->assertSame(array(), $this->transformer->reverseTransform(null));
$this->assertSame(array(), $this->transformerWithNull->reverseTransform(null));
}

/**
Expand Down
152 changes: 152 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
View file
Expand Up @@ -26,6 +26,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
'Roman' => 'e',
);

private $scalarChoices = array(
'Yes' => true,
'No' => false,
'n/a' => '',
);

private $numericChoicesFlipped = array(
0 => 'Bernhard',
1 => 'Fabien',
Expand Down Expand Up @@ -139,6 +145,58 @@ public function testExpandedFlippedChoicesOptionsTurnIntoChildren()
$this->assertCount(count($this->choices), $form, 'Each choice should become a new field');
}

public function testChoiceListWithScalarValues()
{
$view = $this->factory->create('choice', null, array(
'choices' => $this->scalarChoices,
'choices_as_values' => true,
))->createView();

$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value);
$this->assertSame('', $view->vars['choices'][2]->value);
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][0], $view->vars['value']), 'True value should not be pre selected');
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][1], $view->vars['value']), 'False value should not be pre selected');
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][2], $view->vars['value']), 'Empty value should not be pre selected');
}

public function testChoiceListWithScalarValuesAndFalseAsPreSetData()
{
$view = $this->factory->create('choice', false, array(
'choices' => $this->scalarChoices,
'choices_as_values' => true,
))->createView();

$this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'False value should be pre selected');
}

public function testExpandedChoiceListWithScalarValues()
{
$view = $this->factory->create('choice', null, array(
'choices' => $this->scalarChoices,
'choices_as_values' => true,
'expanded' => true,
))->createView();

$this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
$this->assertFalse($view->children[1]->vars['checked'], 'False value should not be pre selected');
$this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
}

public function testExpandedChoiceListWithScalarValuesAndFalseAsPreSetData()
{
$view = $this->factory->create('choice', false, array(
'choices' => $this->scalarChoices,
'choices_as_values' => true,
'expanded' => true,
))->createView();

$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value);
$this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
$this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
}

public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
{
$form = $this->factory->create('choice', null, array(
Expand Down Expand Up @@ -198,6 +256,100 @@ public function testPlaceholderNotPresentIfEmptyChoice()
$this->assertCount(2, $form, 'Each choice should become a new field');
}

public function testPlaceholderWithBooleanChoices()
{
$form = $this->factory->create('choice', null, array(
'multiple' => false,
'expanded' => false,
'required' => false,
'choices' => array(
'Yes' => true,
'No' => false,
),
'placeholder' => 'Select an option',
'choices_as_values' => true,
));

$view = $form->createView();

$this->assertSame('', $view->vars['value'], 'Value should be empty');
$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
$this->assertFalse($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should not be selected');
}

public function testPlaceholderWithBooleanChoicesWithFalseAsPreSetData()
{
$form = $this->factory->create('choice', false, array(
'multiple' => false,
'expanded' => false,
'required' => false,
'choices' => array(
'Yes' => true,
'No' => false,
),
'placeholder' => 'Select an option',
'choices_as_values' => true,
));

$view = $form->createView();

$this->assertSame('0', $view->vars['value'], 'Value should be "0"');
$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
$this->assertTrue($view->vars['is_selected']($view->vars['choices'][1]->value, $view->vars['value']), 'Choice "false" should be selected');
}

public function testPlaceholderWithExpandedBooleanChoices()
{
$form = $this->factory->create('choice', null, array(
'multiple' => false,
'expanded' => true,
'required' => false,
'choices' => array(
'Yes' => true,
'No' => false,
),
'placeholder' => 'Select an option',
'choices_as_values' => true,
));

$this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
$this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');

$view = $form->createView();

$this->assertSame('', $view->vars['value'], 'Value should be empty');
$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
$this->assertFalse($view->children[1]->vars['checked'], 'Choice "false" should not be selected');
}

public function testPlaceholderWithExpandedBooleanChoicesAndWithFalseAsPreSetData()
{
$form = $this->factory->create('choice', false, array(
'multiple' => false,
'expanded' => true,
'required' => false,
'choices' => array(
'Yes' => true,
'No' => false,
),
'placeholder' => 'Select an option',
'choices_as_values' => true,
));

$this->assertTrue(isset($form['placeholder']), 'Placeholder should be set');
$this->assertCount(3, $form, 'Each choice should become a new field, placeholder included');

$view = $form->createView();

$this->assertSame('0', $view->vars['value'], 'Value should be "0"');
$this->assertSame('1', $view->vars['choices'][0]->value);
$this->assertSame('0', $view->vars['choices'][1]->value, 'Choice "false" should have "0" as value');
$this->assertTrue($view->children[1]->vars['checked'], 'Choice "false" should be selected');
}

public function testExpandedChoicesOptionsAreFlattened()
{
$form = $this->factory->create('choice', null, array(
Expand Down

1 comment on commit 3eac469

@yaosoft
Copy link

@yaosoft yaosoft commented on 3eac469 Sep 13, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works fine, thank you.

Please sign in to comment.