Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form] do not accept array input when a form is not multiple (regression) #46519

Open
raziel057 opened this issue May 31, 2022 · 10 comments
Open

Comments

@raziel057
Copy link
Contributor

raziel057 commented May 31, 2022

Symfony version(s) affected

6.1.0

Description

Hi,

From the following commit 6bffdbc I got a regression because in a specific FileType component I can enable the support for single or multiple upload. But when using a single upload I always post an array that contains metadata of a single element (file). So the field is set as multiple=false, and compound=false but I submit an array that I transform to a file entity.

image

Now, I got a TransformationFailedException with message: Submitted data was expected to be text or number, array given.

image

For me it's important to provide at least the id of file (if it exists) and the file name to support the update of file. I tried to set the field as compound but it leads to other issues, as I always receive an empty File entity as data in my listener (on submit).

I don't understand why the the submitted data corresponding to a field (not multiple or compound) should not be an array.

BlueimpFileType:

<?php

namespace PTC\CoreBundle\Form\Type;

...

class BlueimpFileType extends AbstractType
{
    public function __construct(
        private readonly string $uploadDir,
        private readonly EntityManagerInterface $em,
        private readonly LoggerInterface $logger
    ) {
    }

    /**
     * {@inheritdoc}
     */
    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $uploadDir = $this->uploadDir;

        if ($options['module'] !== null) {
            $uploadDir .= '/'.$options['module'];
        }

        if ($options['userId'] !== null) {
            $uploadDir .= '/'.$options['userId'];
        }

        if (!$options['multiple']) {
            $options['max_number_of_files'] = 1;
        }

        $builder
            ->addEventSubscriber(new BlueimpFileListener($uploadDir, $options['type'], $this->em, $this->logger))
            //->addViewTransformer(new CollectionToArrayTransformer())
            ->setAttribute('btnlabel', $options['btnlabel'])
            ->setAttribute('edit_title', $options['edit_title'])
            ->setAttribute('attr_title', $options['attr_title'])
            ->setAttribute('multiple', $options['multiple'])
            ->setAttribute('max_number_of_files', $options['max_number_of_files'])
            ->setAttribute('module', $options['module'])
            ->setAttribute('userId', $options['userId'])
            ->setAttribute('type', $options['type'])
            ->setAttribute('download_route', $options['download_route'])
            ->setAttribute('download_route_params', $options['download_route_params']);

        if ($options['multiple']) {
            $builder->addViewTransformer(new CollectionToArrayTransformer());
        }
    }

    /**
     * {@inheritdoc}
     */
    public function buildView(FormView $view, FormInterface $form, array $options)
    {
        $data = $form->getViewData();
        $config = $form->getConfig();

        $files = array();

        $this->logger->info('buildView');
        //$this->logger->info(print_r($data, true));

        if (false === empty($data)) {
            if ($data instanceof EntityFile) {
                $this->logger->info('data is an EntityFile');
                $files[] = $data;
            } elseif (\is_array($data)) {
                $this->logger->info('data is a array');
            //} elseif ($data instanceof PersistentCollection) {
                //$this->logger->info('data is a PersistentCollection');
                foreach($data as $dataFile) {
                    if ($dataFile instanceof EntityFile) {
                        $this->logger->info('EntityFile');
                        $files[] = $dataFile;
                    }
                }
            }
        }

        $view->vars = \array_replace($view->vars, array(
            'files' => $files,
            'btnlabel' => $config->getAttribute('btnlabel'),
            'edit_title' => $config->getAttribute('edit_title'),
            'attr_title' => $config->getAttribute('attr_title'),
            'max_number_of_files' => $config->getAttribute('max_number_of_files'),
            'module' => $config->getAttribute('module'),
            'userId' => $config->getAttribute('userId'),
            'download_route' => $config->getAttribute('download_route'),
            'download_route_params' => $config->getAttribute('download_route_params'),
        ));
    }

    /**
     * {@inheritdoc}
     */
    public function configureOptions(OptionsResolver $resolver)
    {
        // data_class must be mapped as :
        // - PTC\CoreBundle\Entity\File if the field is not multiple
        // - null if the field is not multiple because the collection can be:
        //      - Doctrine\ORM\PersistentCollection: if the entity containing the files exists
        //      - Doctrine\Common\Collections\ArrayCollection: if the entity containing the files doesn't exist

        $dataClass = function (Options $options) {
            return (!$options['multiple'])
                ? File::class
                //: 'Doctrine\ORM\PersistentCollection';
                : null;
        };

        $resolver->setDefaults(array(
            'data_class' => $dataClass,
            'compound' => false,
            'files' => array(),
            'btnlabel' => 'Add File',
            'edit_title' => false,
            'attr_title' => array(),
            'multiple' => false,
            'max_number_of_files' => 5,
            'module' => 'FILE',
            'userId' => null,
            'type' => 'FILE',
            'download_route' => 'get_file',
            'download_route_params' => array(),
        ));
    }

}

BlueimpFileListener:

<?php

namespace PTC\CoreBundle\Form\EventListener;

...

class BlueimpFileListener implements EventSubscriberInterface
{
    public function __construct(
        private readonly string $uploadDir,
        private readonly string $type,
        private readonly EntityManagerInterface $em,
        private readonly LoggerInterface $logger
    ) {
    }

    public function onSubmit(FormEvent $event)
    {
        $data = $event->getData();
        $config = $event->getForm()->getConfig();

        $this->logger->info('onSubmit');

        if (true === empty($data)) {
            return;
        }

        if (!$config->getAttribute('multiple')) {

            $this->logger->info('SINGLE FILES');
            //$this->logger->info(print_r($data, true));

            if ($data instanceof EntityFile) {
                $event->setData(null);
                return;
            }

            $file = $this->createEntityWithFormData($event, $data[0]);
            $event->setData($file);

        } else {

            $this->logger->info('MULTI FILES');
            //$this->logger->info(print_r($data, true));

            $files = array();
            foreach ($data as $formDataFile) {
                $files[] = $this->createEntityWithFormData($event, $formDataFile);
            }

            $event->setData($files);
        }
    }

    /**
     * Data posted from the form
     */
    private function createEntityWithFormData(FormEvent $event, EntityFile|array $data): ?EntityFile
    {
        $entity = null;

        if (!$data instanceof EntityFile) {

            if ($data['id'] !== null && trim($data['id']) !== '') {

                $entity = $this->em->getRepository(EntityFile::class)->findOneBy(array('id' => $data['id'], 'type' => $this->type));

            } elseif (true === \is_file($this->uploadDir.'/'.$data['name'])) {

                $handle = new File($this->uploadDir.'/'.$data['name']);

                $entity = new EntityFile();
                $entity->setType($this->type);
                $entity->setMimetype($handle->getMimeType());
                $entity->setName($handle->getFilename());
                $entity->setContent(file_get_contents($handle->getPathname()));
                $entity->setSize($handle->getSize());

                if (!$event->getForm()->isRoot() && \is_object($event->getForm()->getRoot()->getData())) {
                    // Used to link the File to the root entity (Loggable)
                    $entity->setParent($event->getForm()->getRoot()->getData());
                }

                if (isset($data['title'])) {
                    $entity->setTitle($data['title']);
                }
            }
        }

        return $entity;
    }

    /**
     * {@inheritdoc}
     */
    public static function getSubscribedEvents(): array
    {
        return array(FormEvents::SUBMIT => 'onSubmit');
    }
}

view:

{%- macro widget_blueimp_file_prototype(key, full_name, edit_title, attr_title, file, module, userId, download_route, download_route_params, readonly, disabled) -%}
    {% if file.id is not defined %}
        {% set download_url = "__download_url__" %}
    {% elseif file.id is null and file.name is not null %}
        {% set download_url = path('get_file_from_cache', { 'module': module, 'name': file.name, 'userId': userId }) %}
    {% elseif download_route == 'get_file' %}
        {% set download_url = path(download_route, { 'module': module, 'id': file.id|default('__file_id__') }) %}
    {% else %}
        {% set download_route_params = download_route_params|merge({ 'id': file.id|default('__file_id__') }) %}
        {% set download_url = path(download_route, download_route_params) %}
    {% endif %}
    <li class="uploader-queue-item">
        {% if not readonly and not disabled %}
            {% if edit_title %}
                <input type="text" name="{{ full_name }}[{{ key }}][title]"
                {% for attrname,attrvalue in attr_title %}{{attrname}}="{{attrvalue}}"{% endfor %} class="file-title" value="{{ file.title|default('') }}"/>
            {% endif %}
            <div class="cancel"><a href="#" onclick="deleteFile($(this).parents('.uploader-queue-item'));return false;"></a></div>
            <span class="fileName">
                <a href="{{ download_url }}" title="{{ 'download.file'|trans }}" target="_blank">
                    {{ file.name|default('__file_name__') }}
                </a> - {{ file.size|default('__file_size__')|bytesFormat }}
            </span>
            <div class="uploader-progress"><div class="uploader-progress-bar" style="width: 100%;"><!--Progress Bar--></div></div>
            <input type="hidden" name="{{ full_name }}[{{ key }}][id]" value="{{ file.id|default('') }}">
            <input type="hidden" name="{{ full_name }}[{{ key }}][name]" value="{{ file.name|default('__file_name__') }}">
            <input type="hidden" name="{{ full_name }}[{{ key }}][size]" value="{{ file.size|default('__file_size__')|bytesFormat }}">
        {% else %}
            <span class="fileName">
                <a href="{{ download_url }}" title="{{ 'download.file'|trans }}" target="_blank">
                    {{ file.name|default('__file_name__') }}
                </a> - {{ file.size|default('__file_size__')|bytesFormat }}
            </span>
        {% endif %}
    </li>
{%- endmacro -%}

{%- block blueimp_file_row -%}
    {% set readonly = (attr.readonly is defined and attr.readonly != 'readonly') %}
    <div class="form_row">
        {{ block('form_label') }}
        <div class="bloc_file">
            {% if not readonly and not disabled %}
                <span class="btn btn-bloc fileinput-button">
                    <span class="icon-upload"></span>&nbsp;<span class="upload-btn-label">{{ btnlabel }}</span>
                    <input type="file" id="{{ id }}" class="fileUploadField" name="uploadFile" data-url="{{ path('upload_file', { 'module': module, 'userId': userId }) }}"/>
                </span>
                {% if help_attr is not empty %}
                    {{ block('form_help') }}
                {% endif %}
            {% endif %}
            {% if description is defined and description is not null %}
                <div class="description">{{ description|raw }}</div>
            {% endif %}
            <div class="fieldError">{{ form_errors(form) }}</div>
            <ul class="fileList" data-field-name="{{ full_name }}"
                {% import _self as forms %}
                data-prototype="{{ forms.widget_blueimp_file_prototype('__key__', full_name, edit_title, attr_title, null, module, userId, download_route, download_route_params)|escape }}">
            {% if files is not empty %}
                {% for key, file in files %}
                    {{ forms.widget_blueimp_file_prototype(key, full_name, edit_title, attr_title, file, module, userId, download_route, download_route_params, readonly, disabled) }}
                {% endfor %}
            {% endif %}
            </ul>
        </div>
        <script type="text/javascript">
        $(document).ready(function() {
            initUploadField('{{ id }}', {{ max_number_of_files }}, '{{ module }}', {{ userId|default('null') }});
        });
        </script>
    </div>
{%- endblock -%}

How to reproduce

Send data as array (metadata of single filed) on non multiple / compound field.

Possible Solution

Revert the previous commit?

Additional Context

No response

@raziel057
Copy link
Contributor Author

@xabbuh Not sure if it's the best way to fix the issue but I could resolve it by setting compound => true and adding 'allow_extra_fields' => true and then replacing in BlueimpFileListener::onSubmit

$data = $event->getData();

with

$data = $event->getForm()->getExtraData();

@NothingWeAre
Copy link
Contributor

This is interesting change, we used same functionality to accept entities selection, from external web hook,
Which sends json objects like:

{
  item: {id: 111, description: "XXXXXX"},
  category: {id: 2, name: "YYYYY"}
}

While we can reduce objects to ID in PRE_SUBMIT it will require change to tens if not hundreds of forms.
Is there a better way to accept such an objects in symfony forms ?

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@NothingWeAre
Copy link
Contributor

For me this is still relevant.
As of now I have implemented really hacky workaround, by forcing ''multiple" to true and using another property to actually specify whether field accepts multiple selection while using custom transformer.

It would be really helpful if there was a way to use transformers before this check is made.

@carsonbot carsonbot removed the Stalled label Jan 5, 2023
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Friendly ping? Should this still be open? I will close if I don't hear anything.

@NothingWeAre
Copy link
Contributor

Hello.
While I have been using workaround, that was mentioned earlier, I'm pretty sure that it is not an intended way of using "multiple" option.

@carsonbot carsonbot removed the Stalled label Jul 30, 2023
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3

@NothingWeAre
Copy link
Contributor

Hello.
Would be good to at least get the answer if this is intended behaviour

@carsonbot carsonbot removed the Stalled label Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants