Skip to content

[2.7] [Form] fix choice value "false" in ChoiceType #17760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 26, 2016
Merged

[2.7] [Form] fix choice value "false" in ChoiceType #17760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3eac469
[Form] fix choice value "false" in ChoiceType
HeahDude Feb 11, 2016
8f918e5
[Form] refactor `RadioListMapper::mapDataToForm()`
HeahDude Feb 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Symfony/Component/Form/ChoiceList/ArrayChoiceList.php
View file
Open in desktop
Original file line number Diff line number Diff line change
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
6 changes: 2 additions & 4 deletions src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,11 @@ public function __construct(ChoiceListInterface $choiceList)
/**
* {@inheritdoc}
*/
public function mapDataToForms($choice, $radios)
public function mapDataToForms($data, $radios)
{
$valueMap = array_flip($this->choiceList->getValuesForChoices(array($choice)));

foreach ($radios as $radio) {
$value = $radio->getConfig()->getOption('value');
$radio->setData(isset($valueMap[$value]) ? true : false);
$radio->setData($value === $data ? true : false);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/DataTransformer/ChoiceToValueTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
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
8 changes: 8 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,14 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$event->setData(null);
}
});
// For radio lists, pre selection of the choice needs to pre set data
// with the string value so it can be matched in
// {@link \Symfony\Component\Form\Extension\Core\DataMapper\RadioListMapper::mapDataToForms()}
$builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
$choiceList = $event->getForm()->getConfig()->getOption('choice_list');
$value = current($choiceList->getValuesForChoices(array($event->getData())));
$event->setData((string) $value);
});
}
} elseif ($options['multiple']) {
// <select> tag with "multiple" option
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/Form/Tests/ChoiceList/ArrayChoiceListTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Open in desktop
Original file line number Diff line number Diff line change
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
Loading