diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[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.
This commit is contained in:
Jules Pietri 2016-03-15 17:34:45 +01:00
parent 71841c737c
commit ea5375c7b7
6 changed files with 88 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php
View File

@ -261,7 +261,7 @@ class EntityTypeTest extends TypeTestCase
$field->submit(null);
$this->assertNull($field->getData());
$this->assertNull($field->getViewData());
$this->assertSame('', $field->getViewData(), 'View data is always a string');
}
public function testSubmitSingleNonExpandedNull()

2
src/Symfony/Bridge/Doctrine/composer.json
View File

@ -22,7 +22,7 @@
"require-dev": {
"symfony/stopwatch": "~2.2",
"symfony/dependency-injection": "~2.2",
"symfony/form": "~2.7,>=2.7.1",
"symfony/form": "~2.7.12|~2.8.5",
"symfony/http-kernel": "~2.2",
"symfony/property-access": "~2.3",
"symfony/security": "~2.2",

41
src/Symfony/Component/Form/Extension/Core/DataMapper/CheckboxListMapper.php
View File

@ -11,9 +11,8 @@
namespace Symfony\Component\Form\Extension\Core\DataMapper;
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\DataMapperInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Exception\UnexpectedTypeException;
/**
* Maps choices to/from checkbox forms.
@ -26,16 +25,6 @@ use Symfony\Component\Form\Exception\TransformationFailedException;
*/
class CheckboxListMapper implements DataMapperInterface
{
/**
* @var ChoiceListInterface
*/
private $choiceList;
public function __construct(ChoiceListInterface $choiceList)
{
$this->choiceList = $choiceList;
}
/**
* {@inheritdoc}
*/
@ -46,22 +35,12 @@ class CheckboxListMapper implements DataMapperInterface
}
if (!is_array($choices)) {
throw new TransformationFailedException('Expected an array.');
}
try {
$valueMap = array_flip($this->choiceList->getValuesForChoices($choices));
} catch (\Exception $e) {
throw new TransformationFailedException(
'Can not read the choices from the choice list.',
$e->getCode(),
$e
);
throw new UnexpectedTypeException($choices, 'array');
}
foreach ($checkboxes as $checkbox) {
$value = $checkbox->getConfig()->getOption('value');
$checkbox->setData(isset($valueMap[$value]) ? true : false);
$checkbox->setData(in_array($value, $choices, true));
}
}
@ -70,6 +49,10 @@ class CheckboxListMapper implements DataMapperInterface
*/
public function mapFormsToData($checkboxes, &$choices)
{
if (!is_array($choices)) {
throw new UnexpectedTypeException($choices, 'array');
}
$values = array();
foreach ($checkboxes as $checkbox) {
@ -79,14 +62,6 @@ class CheckboxListMapper implements DataMapperInterface
}
}
try {
$choices = $this->choiceList->getChoicesForValues($values);
} catch (\Exception $e) {
throw new TransformationFailedException(
'Can not read the values from the choice list.',
$e->getCode(),
$e
);
}
$choices = $values;
}
}

25
src/Symfony/Component/Form/Extension/Core/DataMapper/RadioListMapper.php
View File

@ -11,8 +11,8 @@
namespace Symfony\Component\Form\Extension\Core\DataMapper;
use Symfony\Component\Form\ChoiceList\ChoiceListInterface;
use Symfony\Component\Form\DataMapperInterface;
use Symfony\Component\Form\Exception\UnexpectedTypeException;
/**
* Maps choices to/from radio forms.
@ -25,26 +25,18 @@ use Symfony\Component\Form\DataMapperInterface;
*/
class RadioListMapper implements DataMapperInterface
{
/**
* @var ChoiceListInterface
*/
private $choiceList;
public function __construct(ChoiceListInterface $choiceList)
{
$this->choiceList = $choiceList;
}
/**
* {@inheritdoc}
*/
public function mapDataToForms($choice, $radios)
{
$valueMap = array_flip($this->choiceList->getValuesForChoices(array($choice)));
if (!is_string($choice)) {
throw new UnexpectedTypeException($choice, 'string');
}
foreach ($radios as $radio) {
$value = $radio->getConfig()->getOption('value');
$radio->setData(isset($valueMap[$value]) ? true : false);
$radio->setData($choice === $value);
}
}
@ -53,6 +45,10 @@ class RadioListMapper implements DataMapperInterface
*/
public function mapFormsToData($radios, &$choice)
{
if (null !== $choice && !is_string($choice)) {
throw new UnexpectedTypeException($choice, 'null or string');
}
$choice = null;
foreach ($radios as $radio) {
@ -61,8 +57,7 @@ class RadioListMapper implements DataMapperInterface
return;
}
$value = $radio->getConfig()->getOption('value');
$choice = current($this->choiceList->getChoicesForValues(array($value)));
$choice = $radio->getConfig()->getOption('value');
return;
}

27
src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
View File

@ -60,9 +60,7 @@ class ChoiceType extends AbstractType
public function buildForm(FormBuilderInterface $builder, array $options)
{
if ($options['expanded']) {
$builder->setDataMapper($options['multiple']
? new CheckboxListMapper($options['choice_list'])
: new RadioListMapper($options['choice_list']));
$builder->setDataMapper($options['multiple'] ? new CheckboxListMapper() : new RadioListMapper());
// Initialize all choices before doing the index check below.
// This helps in cases where index checks are optimized for non
@ -133,22 +131,13 @@ class ChoiceType extends AbstractType
$event->setData($data);
});
}
if (!$options['multiple']) {
// For radio lists, transform empty arrays to null
// This is kind of a hack necessary because the RadioListMapper
// is not invoked for forms without choices
$builder->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) {
if (array() === $event->getData()) {
$event->setData(null);
}
});
}
} elseif ($options['multiple']) {
// <select> tag with "multiple" option
if ($options['multiple']) {
// <select> tag with "multiple" option or list of checkbox inputs
$builder->addViewTransformer(new ChoicesToValuesTransformer($options['choice_list']));
} else {
// <select> tag without "multiple" option
// <select> tag without "multiple" option or list of radio inputs
$builder->addViewTransformer(new ChoiceToValueTransformer($options['choice_list']));
}
@ -247,7 +236,11 @@ class ChoiceType extends AbstractType
$choiceListFactory = $this->choiceListFactory;
$emptyData = function (Options $options) {
if ($options['multiple'] || $options['expanded']) {
if ($options['expanded'] && !$options['multiple']) {
return;
}
if ($options['multiple']) {
return array();
}

84
src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
View File

@ -32,6 +32,12 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
'n/a' => '',
);
private $booleanChoicesWithNull = array(
'Yes' => true,
'No' => false,
'n/a' => null,
);
private $numericChoicesFlipped = array(
0 => 'Bernhard',
1 => 'Fabien',
@ -183,6 +189,19 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$this->assertTrue($view->children[2]->vars['checked'], 'Empty value should be pre selected');
}
public function testExpandedChoiceListWithBooleanAndNullValues()
{
$view = $this->factory->create('choice', null, array(
'choices' => $this->booleanChoicesWithNull,
'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(
@ -197,6 +216,19 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$this->assertFalse($view->children[2]->vars['checked'], 'Empty value should not be pre selected');
}
public function testExpandedChoiceListWithBooleanAndNullValuesAndFalseAsPreSetData()
{
$view = $this->factory->create('choice', false, array(
'choices' => $this->booleanChoicesWithNull,
'choices_as_values' => true,
'expanded' => true,
))->createView();
$this->assertFalse($view->children[0]->vars['checked'], 'True value should not be pre selected');
$this->assertTrue($view->children[1]->vars['checked'], 'False value should be pre selected');
$this->assertFalse($view->children[2]->vars['checked'], 'Null value should not be pre selected');
}
public function testPlaceholderPresentOnNonRequiredExpandedSingleChoice()
{
$form = $this->factory->create('choice', null, array(
@ -319,7 +351,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$view = $form->createView();
$this->assertSame('', $view->vars['value'], 'Value should be empty');
$this->assertSame('', $view->vars['value'], 'Value should be an empty string');
$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');
@ -940,8 +972,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(null);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertFalse($form[0]->getData());
@ -972,8 +1004,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(null);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -990,8 +1022,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit('');
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertFalse($form[0]->getData());
@ -1022,8 +1054,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit('');
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -1040,8 +1072,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(false);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertFalse($form[0]->getData());
@ -1072,8 +1104,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(false);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -1090,8 +1122,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(null);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertTrue($form['placeholder']->getData());
@ -1124,8 +1156,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(null);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -1142,8 +1174,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit('');
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertTrue($form['placeholder']->getData());
@ -1176,8 +1208,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit('');
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -1194,8 +1226,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(false);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
$this->assertTrue($form['placeholder']->getData());
@ -1228,8 +1260,8 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit(false);
$this->assertNull($form->getData());
$this->assertNull($form->getViewData());
$this->assertEmpty($form->getExtraData());
$this->assertSame('', $form->getViewData(), 'View data should always be a string');
$this->assertSame(array(), $form->getExtraData(), 'ChoiceType is compound when expanded, extra data should always be an array');
$this->assertTrue($form->isSynchronized());
}
@ -1247,7 +1279,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
$form->submit('');
$this->assertNull($form->getData());
$this->assertSame('', $form->getData());
$this->assertTrue($form->isSynchronized());
$this->assertTrue($form[0]->getData());