8000 bug #20418 [Form][DX] FileType "multiple" fixes (yceruto) · src-run/symfony@7ef0951 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7ef0951

Browse files
fabpotfabpot
committed
bug symfony#20418 [Form][DX] FileType "multiple" fixes (yceruto)
This PR was squashed before being merged into the 2.7 branch (closes symfony#20418). Discussion ---------- [Form][DX] FileType "multiple" fixes | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#12547 | License | MIT | Doc PR | - # (1st) Derive "data_class" option from passed "multiple" option Information ------------- Following this tutorial ["How to Upload Files"][1] but storing many `brochures` instead of one, i.e.: ```php // src/AppBundle/Entity/Product.php class Product { /** * @var string[] * * @Orm\Column(type="array") */ private $brochures; //... } ``` ```php //src/AppBundle/Form/ProductType.php $builder->add('brochures', FileType::class, array( 'label' => 'Brochures (PDF files)', 'multiple' => true, )); ``` The Problem -------------- I found a pain point here when the form is loaded again after save some brochures (Exception): > The form's view data is expected to be an instance of class Symfony\Component\HttpFoundation\File\File, but is a(n) array. You can avoid this error by setting the "data_class" option to null or by adding a view transformer that transforms a(n) array to an instance of Symfony\Component\HttpFoundation\File\File. The message is very clear, but counter-intuitive in this case, because the form field (`FileType`) was configured with `multiple = true`, so IMHO it shouldn't expect a `File` instance but an array of them at all events. The PR's effect --------------- **Before:** ```php $form = $this->createFormBuilder($product) ->add('brochures', FileType::class, [ 'multiple' => true, 'data_class' => null, // <---- mandatory ]) ->getForm(); ``` **After:** ```php $form = $this->createFormBuilder($product) ->add('brochures', FileType::class, [ 'multiple' => true, ]) ->getForm(); ``` # (2nd) Return empty `array()` at submit no file Information ------------- Based on the same information before, but adding some constraints: ```php // src/AppBundle/Entity/Product.php use Symfony\Component\Validator\Constraints as Assert; class Product { /** * @var string[] * * @Orm\Column(type="array") * * @Assert\Count(min="1") // or @Assert\NotBlank() * @Assert\All({ * @Assert\File(mimeTypes = {"application/pdf", "application/x-pdf"}) * }) * */ private $brochures; } ``` This should require at least one file to be stored. The Problem -------------- But, when no file is uploaded at submit the form, it's valid completely. The submitted data for this field was `array(null)` so the constraints pass without any problem: * `@Assert\Count(min="1")` pass! because contains at least one element (No matter what) * `@Assert\NotBlank()` it could pass! because no `false` and no `empty()` * `@Assert\File()` pass! because the element is `null` Apart from that really we expecting an empty array instead. The PR's effect ---------------- **Before:** ```php // src/AppBundle/Entity/Product.php use Symfony\Component\Validator\Constraints as Assert; class Product { /** * @var string[] * * @Orm\Column(type="array") * * @Assert\All({ * @Assert\NotBlank, * @Assert\File(mimeTypes = {"application/pdf", "application/x-pdf"}) * }) * */ private $brochures; } ``` **After:** ```php // src/AppBundle/Entity/Product.php use Symfony\Component\Validator\Constraints as Assert; class Product { /** * @var string[] * * @Orm\Column(type="array") * * @Assert\Count(min="1") // or @Assert\NotBlank * @Assert\All({ * @Assert\File(mimeTypes = {"application/pdf", "application/x-pdf"}) * }) * */ private $brochures; } ``` [1]: http://symfony.com/doc/current/controller/upload_file.html Commits ------- 36b7ba6 [Form][DX] FileType "multiple" fixes
2 parents fe15381 + 36b7ba6 commit 7ef0951

File tree

2 files changed

+63
-5
lines changed

2 files changed

+63
-5
lines changed

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

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,37 @@
1212
namespace Symfony\Component\Form\Extension\Core\Type;
1313

1414
use Symfony\Component\Form\AbstractType;
15+
use Symfony\Component\Form\FormBuilderInterface;
16+
use Symfony\Component\Form\FormEvent;
17+
use Symfony\Component\Form\FormEvents;
1518
use Symfony\Component\Form\FormInterface;
1619
use Symfony\Component\Form\FormView;
20+
use Symfony\Component\OptionsResolver\Options;
1721
use Symfony\Component\OptionsResolver\OptionsResolver;
1822

1923
class FileType extends AbstractType
2024
{
25+
/**
26+
* {@inheritdoc}
27+
*/
28+
public function buildForm(FormBuilderInterface $builder, array $options)
29+
{
30+
if ($options['multiple']) {
31+
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
32+
$form = $event->getForm();
33+
$data = $event->getData();
34+
35+
// submitted data for an input file (not required) without choosing any file
36+
if (array(null) === $data) {
37+
$emptyData = $form->getConfig()->getEmptyData();
38+
39+
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
40+
$event->setData($data);
41+
}
42+
});
43+
}
44+
}
45+
2146
/**
2247
* {@inheritdoc}
2348
*/
@@ -39,20 +64,26 @@ public function buildView(FormView $view, FormInterface $form, array $options)
3964
*/
4065
public function finishView(FormView $view, FormInterface $form, array $options)
4166
{
42-
$view
43-
->vars['multipart'] = true
44-
;
67+
$view->vars['multipart'] = true;
4568
}
4669

4770
/**
4871
* {@inheritdoc}
4972
*/
5073
public function configureOptions(OptionsResolver $resolver)
5174
{
75+
$dataClass = function (Options $options) {
76+
return $options['multiple'] ? null : 'Symfony\Component\HttpFoundation\File\File';
77+
};
78+
79+
$emptyData = function (Options $options) {
80+
return $options['multiple'] ? array() : null;
81+
};
82+
5283
$resolver->setDefaults(array(
5384
'compound' => false,
54-
'data_class' => 'Symfony\Component\HttpFoundation\File\File',
55-
'empty_data' => null,
85+
'data_class' => $dataClass,
86+
'empty_data' => $emptyData,
5687
'multiple' => false,
5788
));
5889
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,33 @@ public function testSubmitEmpty()
4444
$this->assertNull($form->getData());
4545
}
4646

47+
public function testSubmitEmptyMultiple()
48+
{
49+
$form = $this->factory->createBuilder('file', null, array(
50+
'multiple' => true,
51+
))->getForm();
52+
53+
// submitted data when an input file is uploaded without choosing any file
54+
$form->submit(array(null));
55+
56+
$this->assertSame(array(), $form->getData());
57+
}
58+
59+
public function testSetDataMultiple()
60+
{
61+
$form = $this->factory->createBuilder('file', null, array(
62+
'multiple' => true,
63+
))->getForm();
64+
65+
$data = array(
66+
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
67+
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
68+
);
69+
70+
$form->setData($data);
71+
$this->assertSame($data, $form->getData());
72+
}
73+
4774
public function testSubmitMultiple()
4875
{
4976
$form = $this->factory->createBuilder('file', null, array(

0 commit comments

Comments
 (0)
0