bug #25948 [Form] Fixed empty data on expanded ChoiceType and FileType (HeahDude)
This PR was merged into the 2.7 branch.
Discussion
----------
[Form] Fixed empty data on expanded ChoiceType and FileType
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #25896
| License | MIT
| Doc PR | ~
Alternative of https://github.com/symfony/symfony/pull/25924.
I don't think we have to do this by adding overhead in master and getting it properly fixed in 2 years.
This is bug I've introduced while fixing another bug #17822. Which then has been replicated in #20418 and #24993.
I propose instead to clean up the code for all LTS, since this is a wrong behaviour that has never been documented, and most of all unreliable.
The `empty_data` can by anything in the view format as long as the reverse view transformation supports it. Even an object derived from the data class could be invokable.
I think we should remain consistent with https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Form/Form.php#L615.
Commits
-------
9722c063fb
[Form] Fixed empty data on expanded ChoiceType and FileType
This commit is contained in:
commit
471c410ce6
@ -33,7 +33,6 @@ use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface as Lega
|
|||||||
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
|
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
|
||||||
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
|
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
|
||||||
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer;
|
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer;
|
||||||
use Symfony\Component\Form\Util\FormUtil;
|
|
||||||
use Symfony\Component\OptionsResolver\Options;
|
use Symfony\Component\OptionsResolver\Options;
|
||||||
use Symfony\Component\OptionsResolver\OptionsResolver;
|
use Symfony\Component\OptionsResolver\OptionsResolver;
|
||||||
|
|
||||||
@ -91,12 +90,12 @@ class ChoiceType extends AbstractType
|
|||||||
$form = $event->getForm();
|
$form = $event->getForm();
|
||||||
$data = $event->getData();
|
$data = $event->getData();
|
||||||
|
|
||||||
|
// Since the type always use mapper an empty array will not be
|
||||||
|
// considered as empty in Form::submit(), we need to evaluate
|
||||||
|
// empty data here so its value is submitted to sub forms
|
||||||
if (null === $data) {
|
if (null === $data) {
|
||||||
$emptyData = $form->getConfig()->getEmptyData();
|
$emptyData = $form->getConfig()->getEmptyData();
|
||||||
|
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
|
||||||
if (false === FormUtil::isEmpty($emptyData) && array() !== $emptyData) {
|
|
||||||
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Convert the submitted data to a string, if scalar, before
|
// Convert the submitted data to a string, if scalar, before
|
||||||
|
@ -27,10 +27,10 @@ class FileType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function buildForm(FormBuilderInterface $builder, array $options)
|
public function buildForm(FormBuilderInterface $builder, array $options)
|
||||||
{
|
{
|
||||||
|
// Ensure that submitted data is always an uploaded file or an array of some
|
||||||
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
|
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
|
||||||
$form = $event->getForm();
|
$form = $event->getForm();
|
||||||
$requestHandler = $form->getConfig()->getRequestHandler();
|
$requestHandler = $form->getConfig()->getRequestHandler();
|
||||||
$data = null;
|
|
||||||
|
|
||||||
if ($options['multiple']) {
|
if ($options['multiple']) {
|
||||||
$data = array();
|
$data = array();
|
||||||
@ -46,19 +46,16 @@ class FileType extends AbstractType
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// submitted data for an input file (not required) without choosing any file
|
// Since the array is never considered empty in the view data format
|
||||||
if (array(null) === $data || array() === $data) {
|
// on submission, we need to evaluate the configured empty data here
|
||||||
|
if (array() === $data) {
|
||||||
$emptyData = $form->getConfig()->getEmptyData();
|
$emptyData = $form->getConfig()->getEmptyData();
|
||||||
|
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
|
||||||
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
$event->setData($data);
|
$event->setData($data);
|
||||||
} elseif (!$requestHandler->isFileUpload($event->getData())) {
|
} elseif (!$requestHandler->isFileUpload($event->getData())) {
|
||||||
$emptyData = $form->getConfig()->getEmptyData();
|
$event->setData(null);
|
||||||
|
|
||||||
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
|
|
||||||
$event->setData($data);
|
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
@ -690,6 +690,21 @@ class ChoiceTypeTest extends BaseTypeTest
|
|||||||
$this->assertSame('test', $form->getData());
|
$this->assertSame('test', $form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSubmitSingleChoiceWithEmptyDataAndInitialData()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, 'initial', array(
|
||||||
|
'multiple' => false,
|
||||||
|
'expanded' => false,
|
||||||
|
'choices' => array('initial', 'test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => 'test',
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame('test', $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
public function testSubmitMultipleChoiceWithEmptyData()
|
public function testSubmitMultipleChoiceWithEmptyData()
|
||||||
{
|
{
|
||||||
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
||||||
@ -705,6 +720,36 @@ class ChoiceTypeTest extends BaseTypeTest
|
|||||||
$this->assertSame(array('test'), $form->getData());
|
$this->assertSame(array('test'), $form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSubmitMultipleChoiceWithEmptyDataAndInitialEmptyArray()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, array(), array(
|
||||||
|
'multiple' => true,
|
||||||
|
'expanded' => false,
|
||||||
|
'choices' => array('test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => array('test'),
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame(array('test'), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSubmitMultipleChoiceWithEmptyDataAndInitialData()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, array('initial'), array(
|
||||||
|
'multiple' => true,
|
||||||
|
'expanded' => false,
|
||||||
|
'choices' => array('initial', 'test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => array('test'),
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame(array('test'), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
public function testSubmitSingleChoiceExpandedWithEmptyData()
|
public function testSubmitSingleChoiceExpandedWithEmptyData()
|
||||||
{
|
{
|
||||||
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
||||||
@ -720,6 +765,21 @@ class ChoiceTypeTest extends BaseTypeTest
|
|||||||
$this->assertSame('test', $form->getData());
|
$this->assertSame('test', $form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSubmitSingleChoiceExpandedWithEmptyDataAndInitialData()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, 'initial', array(
|
||||||
|
'multiple' => false,
|
||||||
|
'expanded' => true,
|
||||||
|
'choices' => array('initial', 'test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => 'test',
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame('test', $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
public function testSubmitMultipleChoiceExpandedWithEmptyData()
|
public function testSubmitMultipleChoiceExpandedWithEmptyData()
|
||||||
{
|
{
|
||||||
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
$form = $this->factory->create(static::TESTED_TYPE, null, array(
|
||||||
@ -735,6 +795,36 @@ class ChoiceTypeTest extends BaseTypeTest
|
|||||||
$this->assertSame(array('test'), $form->getData());
|
$this->assertSame(array('test'), $form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSubmitMultipleChoiceExpandedWithEmptyDataAndInitialEmptyArray()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, array(), array(
|
||||||
|
'multiple' => true,
|
||||||
|
'expanded' => true,
|
||||||
|
'choices' => array('test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => array('test'),
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame(array('test'), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSubmitMultipleChoiceExpandedWithEmptyDataAndInitialData()
|
||||||
|
{
|
||||||
|
$form = $this->factory->create(static::TESTED_TYPE, array('init'), array(
|
||||||
|
'multiple' => true,
|
||||||
|
'expanded' => true,
|
||||||
|
'choices' => array('init', 'test'),
|
||||||
|
'choices_as_values' => true,
|
||||||
|
'empty_data' => array('test'),
|
||||||
|
));
|
||||||
|
|
||||||
|
$form->submit(null);
|
||||||
|
|
||||||
|
$this->assertSame(array('test'), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @group legacy
|
* @group legacy
|
||||||
*/
|
*/
|
||||||
|
Reference in New Issue
Block a user