8000 bug #25948 [Form] Fixed empty data on expanded ChoiceType and FileTyp… · pierredup/symfony@471c410 · GitHub
[go: up one dir, main page]
Skip to content

Commit 471c410

Browse files
fabpotfabpot
committed
bug symfony#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 | symfony#25896 | License | MIT | Doc PR | ~ Alternative of symfony#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 symfony#17822. Which then has been replicated in symfony#20418 and symfony#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
2 parents c61f941 + 9722c06 commit 471c410

File tree

3 files changed

+100
-14
lines changed

3 files changed

+100
-14
lines changed

‎src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
use Symfony\Component\Form\Extension\Core\EventListener\MergeCollectionListener;
3434
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoiceToValueTransformer;
3535
use Symfony\Component\Form\Extension\Core\DataTransformer\ChoicesToValuesTransformer;
36-
use Symfony\Component\Form\Util\FormUtil;
3736
use Symfony\Component\OptionsResolver\Options;
3837
use Symfony\Component\OptionsResolver\OptionsResolver;
3938

@@ -91,12 +90,12 @@ public function buildForm(FormBuilderInterface $builder, array $options)
9190
$form = $event->getForm();
9291
$data = $event->getData();
9392

93+
// Since the type always use mapper an empty array will not be
94+
// considered as empty in Form::submit(), we need to evaluate
95+
// empty data here so its value is submitted to sub forms
9496
if (null === $data) {
9597
$emptyData = $form->getConfig()->getEmptyData();
96-
97-
if (false === FormUtil::isEmpty($emptyData) && array() !== $emptyData) {
98-
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
99-
}
98+
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
10099
}
101100

102101
// Convert the submitted data to a string, if scalar, before

‎src/Symfony/Component/Form/Extension/Core/Type/FileType.php

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ class FileType extends AbstractType
2727
*/
2828
public function buildForm(FormBuilderInterface $builder, array $options)
2929
{
30+
// Ensure that submitted data is always an uploaded file or an array of some
3031
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
3132
$form = $event->getForm();
3233
$requestHandler = $form->getConfig()->getRequestHandler();
33-
$data = null;
3434

3535
if ($options['multiple']) {
3636
$data = array();
@@ -46,19 +46,16 @@ public function buildForm(FormBuilderInterface $builder, array $options)
4646
}
4747
}
4848

49-
// submitted data for an input file (not required) without choosing any file
50-
if (array(null) === $data || array() === $data) {
49+
// Since the array is never considered empty in the view data format
50+
// on submission, we need to evaluate the configured empty data here
51+
if (array() === $data) {
5152
$emptyData = $form->getConfig()->getEmptyData();
52-
53-
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
53+
$data = $emptyData instanceof \Closure ? $emptyData($form, $data) : $emptyData;
5454
}
5555

5656
$event->setData($data);
5757
} elseif (!$requestHandler->isFileUpload($event->getData())) {
58-
$emptyData = $form->getConfig()->getEmptyData();
59-
60-
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
61-
$event->setData($data);
58+
$event->setData(null);
6259
}
6360
});
6461
}

‎src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,21 @@ public function testSubmitSingleChoiceWithEmptyData()
690690
$this->assertSame('test', $form->getData());
691691
}
692692

693+
public function testSubmitSingleChoiceWithEmptyDataAndInitialData()
694+
{
695+
$form = $this->factory->create(static::TESTED_TYPE, 'initial', array(
696+
'multiple' => false,
697+
'expanded' => false,
698+
'choices' => array('initial', 'test'),
699+
'choices_as_values' => true,
700+
'empty_data' => 'test',
701+
));
702+
703+
$form->submit(null);
704+
705+
$this->assertSame('test', $form->getData());
706+
}
707+
693708
public function testSubmitMultipleChoiceWithEmptyData()
694709
{
695710
$form = $this->factory->create(static::TESTED_TYPE, null, array(
@@ -705,6 +720,36 @@ public function testSubmitMultipleChoiceWithEmptyData()
705720
$this->assertSame(array('test'), $form->getData());
706721
}
707722

723+
public function testSubmitMultipleChoiceWithEmptyDataAndInitialEmptyArray()
724+
{
725+
$form = $this->factory->create(static::TESTED_TYPE, array(), array(
726+
'multiple' => true,
727+
'expanded' => false,
728+
'choices' => array('test'),
729+
'choices_as_values' => true,
730+
'empty_data' => array('test'),
731+
));
732+
733+
$form->submit(null);
734+
735+
$this->assertSame(array('test'), $form->getData());
736+
}
737+
738+
public function testSubmitMultipleChoiceWithEmptyDataAndInitialData()
739+
{
740+
$form = $this->factory->create(static::TESTED_TYPE, array('initial'), array(
741+
'multiple' => true,
742+
'expanded' => false,
743+
'choices' => array('initial', 'test'),
744+
'choices_as_values' => true,
745+
'empty_data' => array('test'),
746+
));
747+
748+
$form->submit(null);
749+
750+
$this->assertSame(array('test'), $form->getData());
751+
}
752+
708753
public function testSubmitSingleChoiceExpandedWithEmptyData()
709754
{
710755
$form = $this->factory->create(static::TESTED_TYPE, null, array(
@@ -720,6 +765,21 @@ public function testSubmitSingleChoiceExpandedWithEmptyData()
720765
$this->assertSame('test', $form->getData());
721766
}
722767

768+
public function testSubmitSingleChoiceExpandedWithEmptyDataAndInitialData()
769+
{
770+
$form = $this->factory->create(static::TESTED_TYPE, 'initial', array(
771+
'multiple' => false,
772+
'expanded' => true,
773+
'choices' => array('initial', 'test'),
774+
'choices_as_values' => true,
775+
'empty_data' => 'test',
776+
));
777+
778+
$form->submit(null);
779+
780+
$this->assertSame('test', $form->getData());
781+
}
782+
723783
public function testSubmitMultipleChoiceExpandedWithEmptyData()
724784
{
725785
$form = $this->factory->create(static::TESTED_TYPE, null, array(
@@ -735,6 +795,36 @@ public function testSubmitMultipleChoiceExpandedWithEmptyData()
735795
$this->assertSame(array('test'), $form->getData());
736796
}
737797

798+
public function testSubmitMultipleChoiceExpandedWithEmptyDataAndInitialEmptyArray()
799+
{
800+
$form = $this->factory->create(static::TESTED_TYPE, array(), array(
801+
'multiple' => true,
802+
'expanded' => true,
803+
'choices' => array('test'),
804+
'choices_as_values' => true,
805+
'empty_data' => array('test'),
806+
));
807+
808+
$form->submit(null);
809+
810+
$this->assertSame(array('test'), $form->getData());
811+
}
812+
813+
public function testSubmitMultipleChoiceExpandedWithEmptyDataAndInitialData()
814+
{
815+
$form = $this->factory->create(static::TESTED_TYPE, array('init'), array(
816+
'multiple' => true,
817+
'expanded' => true,
818+
'choices' => array('init', 'test'),
819+
'choices_as_values' => true,
820+
'empty_data' => array('test'),
821+
));
822+
823+
$form->submit(null);
824+
825+
$this->assertSame(array('test'), $form->getData());
826+
}
827+
738828
/**
739829
* @group legacy
740830
*/

0 commit comments

Comments
 (0)
0