Skip to content

Commit

Permalink
bug #25948 [Form] Fixed empty data on expanded ChoiceType and FileTyp…
Browse files Browse the repository at this point in the history 
…e (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 #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
-------

9722c06 [Form] Fixed empty data on expanded ChoiceType and FileType
  • Loading branch information
@fabpot
fabpot committed Feb 1, 2018
2 parents c61f941 + 9722c06 commit 471c410
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 14 deletions.
9 changes: 4 additions & 5 deletions src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
View file
Expand Up @@ -33,7 +33,6 @@
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer;
use Symfony\Component\Form\Util\FormUtil;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\OptionsResolver\OptionsResolver;

Expand Down Expand Up @@ -91,12 +90,12 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$form = $event->getForm();
$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) {
$emptyData = $form->getConfig()->getEmptyData();

if (false === FormUtil::isEmpty($emptyData) && array() !== $emptyData) {
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
}
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
}

// Convert the submitted data to a string, if scalar, before
Expand Down
15 changes: 6 additions & 9 deletions src/Symfony/Component/Form/Extension/Core/Type/FileType.php
View file
Expand Up @@ -27,10 +27,10 @@ class FileType extends AbstractType
*/
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) {
$form = $event->getForm();
$requestHandler = $form->getConfig()->getRequestHandler();
$data = null;

if ($options['multiple']) {
$data = array();
Expand All @@ -46,19 +46,16 @@ public function buildForm(FormBuilderInterface $builder, array $options)
}
}

// submitted data for an input file (not required) without choosing any file
if (array(null) === $data || array() === $data) {
// Since the array is never considered empty in the view data format
// on submission, we need to evaluate the configured empty data here
if (array() === $data) {
$emptyData = $form->getConfig()->getEmptyData();

$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
}

$event->setData($data);
} elseif (!$requestHandler->isFileUpload($event->getData())) {
$emptyData = $form->getConfig()->getEmptyData();

$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
$event->setData($data);
$event->setData(null);
}
});
}
Expand Down
90 changes: 90 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php
View file
Expand Up @@ -690,6 +690,21 @@ public function testSubmitSingleChoiceWithEmptyData()
$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()
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
Expand All @@ -705,6 +720,36 @@ public function testSubmitMultipleChoiceWithEmptyData()
$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()
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
Expand All @@ -720,6 +765,21 @@ public function testSubmitSingleChoiceExpandedWithEmptyData()
$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()
{
$form = $this->factory->create(static::TESTED_TYPE, null, array(
Expand All @@ -735,6 +795,36 @@ public function testSubmitMultipleChoiceExpandedWithEmptyData()
$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
*/
Expand Down

0 comments on commit 471c410

Please sign in to comment.