bug #20418 [Form][DX] FileType "multiple" fixes (yceruto)
This PR was squashed before being merged into the 2.7 branch (closes #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 | https://github.com/symfony/symfony/issues/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
This commit is contained in:
commit
7ef0951daf
@ -12,12 +12,37 @@
|
|||||||
namespace Symfony\Component\Form\Extension\Core\Type;
|
namespace Symfony\Component\Form\Extension\Core\Type;
|
||||||
|
|
||||||
use Symfony\Component\Form\AbstractType;
|
use Symfony\Component\Form\AbstractType;
|
||||||
|
use Symfony\Component\Form\FormBuilderInterface;
|
||||||
|
use Symfony\Component\Form\FormEvent;
|
||||||
|
use Symfony\Component\Form\FormEvents;
|
||||||
use Symfony\Component\Form\FormInterface;
|
use Symfony\Component\Form\FormInterface;
|
||||||
use Symfony\Component\Form\FormView;
|
use Symfony\Component\Form\FormView;
|
||||||
|
use Symfony\Component\OptionsResolver\Options;
|
||||||
use Symfony\Component\OptionsResolver\OptionsResolver;
|
use Symfony\Component\OptionsResolver\OptionsResolver;
|
||||||
|
|
||||||
class FileType extends AbstractType
|
class FileType extends AbstractType
|
||||||
{
|
{
|
||||||
|
/**
|
||||||
|
* {@inheritdoc}
|
||||||
|
*/
|
||||||
|
public function buildForm(FormBuilderInterface $builder, array $options)
|
||||||
|
{
|
||||||
|
if ($options['multiple']) {
|
||||||
|
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
|
||||||
|
$form = $event->getForm();
|
||||||
|
$data = $event->getData();
|
||||||
|
|
||||||
|
// submitted data for an input file (not required) without choosing any file
|
||||||
|
if (array(null) === $data) {
|
||||||
|
$emptyData = $form->getConfig()->getEmptyData();
|
||||||
|
|
||||||
|
$data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
|
||||||
|
$event->setData($data);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* {@inheritdoc}
|
* {@inheritdoc}
|
||||||
*/
|
*/
|
||||||
@ -39,9 +64,7 @@ class FileType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function finishView(FormView $view, FormInterface $form, array $options)
|
public function finishView(FormView $view, FormInterface $form, array $options)
|
||||||
{
|
{
|
||||||
$view
|
$view->vars['multipart'] = true;
|
||||||
->vars['multipart'] = true
|
|
||||||
;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -49,10 +72,18 @@ class FileType extends AbstractType
|
|||||||
*/
|
*/
|
||||||
public function configureOptions(OptionsResolver $resolver)
|
public function configureOptions(OptionsResolver $resolver)
|
||||||
{
|
{
|
||||||
|
$dataClass = function (Options $options) {
|
||||||
|
return $options['multiple'] ? null : 'Symfony\Component\HttpFoundation\File\File';
|
||||||
|
};
|
||||||
|
|
||||||
|
$emptyData = function (Options $options) {
|
||||||
|
return $options['multiple'] ? array() : null;
|
||||||
|
};
|
||||||
|
|
||||||
$resolver->setDefaults(array(
|
$resolver->setDefaults(array(
|
||||||
'compound' => false,
|
'compound' => false,
|
||||||
'data_class' => 'Symfony\Component\HttpFoundation\File\File',
|
'data_class' => $dataClass,
|
||||||
'empty_data' => null,
|
'empty_data' => $emptyData,
|
||||||
'multiple' => false,
|
'multiple' => false,
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
@ -44,6 +44,33 @@ class FileTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
|
|||||||
$this->assertNull($form->getData());
|
$this->assertNull($form->getData());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testSubmitEmptyMultiple()
|
||||||
|
{
|
||||||
|
$form = $this->factory->createBuilder('file', null, array(
|
||||||
|
'multiple' => true,
|
||||||
|
))->getForm();
|
||||||
|
|
||||||
|
// submitted data when an input file is uploaded without choosing any file
|
||||||
|
$form->submit(array(null));
|
||||||
|
|
||||||
|
$this->assertSame(array(), $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testSetDataMultiple()
|
||||||
|
{
|
||||||
|
$form = $this->factory->createBuilder('file', null, array(
|
||||||
|
'multiple' => true,
|
||||||
|
))->getForm();
|
||||||
|
|
||||||
|
$data = array(
|
||||||
|
$this->createUploadedFileMock('abcdef', 'first.jpg', true),
|
||||||
|
$this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
|
||||||
|
);
|
||||||
|
|
||||||
|
$form->setData($data);
|
||||||
|
$this->assertSame($data, $form->getData());
|
||||||
|
}
|
||||||
|
|
||||||
public function testSubmitMultiple()
|
public function testSubmitMultiple()
|
||||||
{
|
{
|
||||||
$form = $this->factory->createBuilder('file', null, array(
|
$form = $this->factory->createBuilder('file', null, array(
|
||||||
|
Reference in New Issue
Block a user