Count constraint 'min'=>1 applied against file field type with 'multiple'=>true is ignored #12547
Labels
Comments
|
Have you already checked what the CountValidator gets as |
|
@nurikabe I'v 8000 e tested your form and the Count Constraint / Validator behaves correctly. As form data we get the following values for an empty file field: array (size=2)
'documents' =>
array (size=1)
0 => string '' (length=0)Like in #1400 mentioned, you can use the |
|
@SenseException Thanks, I'll use |
|
@fabpot Could this be a [DX] labled issue? |
|
Can this issue be closed? |
|
This ticket doesn't make sense to me. It's an optional field. The count constraint will make the field pass when it's not set. So whatever you set as |
|
@Tobion That's counter-intuitive to me. I would assume |
|
All constraints allow |
|
see #10221 (comment) and #11956 |
|
But if this the comments in #12547 (comment) and
are true, then this seem strange to me. When no file is uploaded then the value should be null and not an array with an empty string. |
It's a bug then? IMHO this requires a view data transformation "if public function reverseTransform($array)
{
// multiple files uploaded must be an array always
// even when no file is uploaded (in this case: array(null))
if (!is_array($array)) {
throw new TransformationFailedException('Expected an array.');
}
// remove empty items (e.g. when no file is uploaded: array(null) --> array())
$array = array_filter($array);
// ensures that each item must be a File object
foreach ($array as $value) {
if (!$value instanceof File) {
throw new TransformationFailedException('Expected an array of File.');
}
}
// returns empty array or array of File objects
return $array;
}As well as works other form types (e.g. |
fabpot
added a commit
that referenced
this issue
Dec 3, 2016
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 | #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
symfony-splitter
pushed a commit
to symfony/form
that referenced
this issue
Dec 3, 2016
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 | symfony/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 th 7580 e 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For example:
This validates true even if no file is selected. If min is greater than one, however, the constraint behaves as expected and the form fails with "This collection should contain # elements or more."
Experiencing this in both 2.5.7 as well as 2.6.0-BETA1.
The text was updated successfully, but these errors were encountered: