a2lix / TranslationFormBundle

Ease translations with some dedicated Symfony form types
https://a2lix.fr/bundles/translation-form
MIT License
330 stars 140 forks source link

Runtime exception "Warning: Undefined variable $unknowsFields" #375

Closed Thibibi closed 1 year ago

Thibibi commented 2 years ago

Composer packages

$ composer show
a2lix/auto-form-bundle              0.4.2   Automate form building
a2lix/translation-form-bundle       3.2.1   Translate your doctrine objects easily with some helpers

PHP version

$ php -v
PHP 8.1.3 (cli) (built: Feb 16 2022 08:26:12) (ZTS Visual C++ 2019 x64)
Copyright (c) The PHP Group
Zend Engine v4.1.3, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.3, Copyright (c), by Zend Technologies
    with Xdebug v3.1.3, Copyright (c) 2002-2022, by Derick Rethans

Subject

I get the runtime Exception "Warning: Undefined variable $unknowsFields" when I try to add options to my translated fields. In fact I have 2 problems:

  1. First problem I can't find a way to give options to my translated fields, trying to do so, by option 'fields', results in an error
  2. Second problem, there is an error in the code that raises the exception... XD

My class:

// Truc.php
#[ORM\Entity(repositoryClass: TrucRepository::class)]
class Truc implements TranslatableInterface
{
    use TranslatableTrait;

    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    private $id;

    #[ORM\Column(type: 'string', length: 255)]
    private $Code;

    // + getters, seters, blah blah blah
}

My translation class:

// TrucTranslation.php
#[ORM\Entity(repositoryClass: TrucTranslationRepository::class)]
class TrucTranslation implements TranslationInterface
{
    use TranslationTrait;

    #[ORM\Id]
    #[ORM\GeneratedValue]
    #[ORM\Column(type: 'integer')]
    private $id;

    #[ORM\Column(name: 'label', type: 'string', length: 255)]
    private $Labelo;

    // + getters, seters, blah blah blah
}

My formtype:

// TrucType.php
class TrucType extends AbstractType
{
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        // Spécial polymorphisme
        parent::buildForm($builder, $options);

        $builder
            ->add('translations', TranslationsType::class, [
                ////////////////////////// Remove this part and it works fine /////////////////////
                'fields' => [
                    'labelo' => [
                        'field_type' => 'textarea',
                    ],
                 ],
                ///////////////////////////////////////////////////////////////////////////////////
            ])
            ->add('code', TextType::class, [
                'label' => 'code',
            ])
            ->add('save', SubmitType::class, [
                'label' => 'action.save',
            ]);
        ;
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'data_class' => Truc::class,
        ]);
    }
}

Steps to reproduce

If you remove the part between the two ////// lines, it works. If you let it, an error occurs saying "Warning: Undefined variable $unknowsFields", and as a matter of fact, there is a piece of code in the bundle refering to this non-existing variable:

// vendor\a2lix\auto-form-bundle\src\Form\Manipulator\DoctrineORMManipulator.php

(...)

    private function checkFieldIsValid($formFieldName, $formFieldConfig, $validObjectFieldsConfig): void
    {
        if (isset($validObjectFieldsConfig[$formFieldName])) {
            return;
        }
        if (false === ($formFieldConfig['mapped'] ?? true)) {
            return;
        }
        // READ AT THE END OF THE LINE HERE, $unknownFields variable does not exist!
        throw new \RuntimeException(sprintf("Field(s) '%s' doesn't exist in %s", implode(', ', $unknowsFields), $class));
    }

Expected results

  1. I would like to have my 'fields' => [ 'labelo' ] option to work
  2. We could also fix the bug of the non-existing variable $unknowsFields
Thibibi commented 2 years ago

About my post above:

  1. Point 1 is solved, the problem comes from the field name given in the "fields" form option: case is important. Labello works, labello don't (normal, see the field definition in Entity)
  2. Point 2 is still there, throwing "Warning: Undefined variable $unknowsFields" instead of "Field 'labelo' in unknown on the entity, please check orthograph carefully" XD
scuben commented 2 years ago

I encounter the same Problem. This commit introduces the error on the exception: https://github.com/a2lix/AutoFormBundle/commit/d1299090c547f429c642b543de717b34f20ba8e3

wucherpfennig commented 2 years ago

I am facing the same issue.

I am trying to use a file field (vichuploader) in a translation and I cannot make it work

wucherpfennig commented 2 years ago

Update in order to explain my issue:

In order to perform some crud operations I am using easyadmin.

I created a new easyadmin field:

<?php

declare(strict_types=1);

namespace App\Field;

use A2lix\TranslationFormBundle\Form\Type\TranslationsType;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Field\FieldInterface;
use EasyCorp\Bundle\EasyAdminBundle\Field\FieldTrait;

final class TranslationField implements FieldInterface
{

    use FieldTrait;

    public static function new(string $propertyName, ?string $label = NULL, array $fieldsConfig = []): self
    {
        return (new self())
            ->setProperty($propertyName)
            ->setLabel($label)
            ->setFormType(TranslationsType::class)
            ->setFormTypeOptions([
                'default_locale' => 'de',
                'excluded_fields' => [
                    'createdAt', 'updatedAt'
                ],
                'fields' => $fieldsConfig,
            ]);
    }

}

In my crud controller I am using something like:

      $translate = TranslationField::new('translations', 'BLBLA', [
            'title' => [
                'row_attr' => ['class' => 'full-width'],
                'label' => 'Title',
                'label_attr' => ['style' => 'font-weight:bold;padding-bottom:0.5rem;'],
                'required' => TRUE,
                'help' => 'Aussagekräftiger Titel des Assets',
            ],
            'localResourceFile' => [
                'row_attr' => ['class' => 'full-width'],
                'label' => 'Datei Upload',
                'label_attr' => ['style' => 'font-weight:bold;padding-bottom:0.5rem;'],
                'help' => 'Optional: Wahlweise kann eine Datei hinterlegt werden',
                'field_type' => 'Vich\UploaderBundle\Form\Type\VichFileType',
                'required' => FALSE,
                'allow_delete' => TRUE,
                'delete_label' => 'Löschen?',
            ],
        ]);

I do get the error from above if localResourceFile is used. If I localResourceFile to mapped=FALSE the form etc. looks good but no data will be submitted...

wucherpfennig commented 2 years ago

Update 2:

If I uncomment the line with the $unknowsFields in the file vendor\a2lix\auto-form-bundle\src\Form\Manipulator\DoctrineORMManipulator.php my example from above works just fine...

izhddm commented 1 year ago

Update 2:

If I uncomment the line with the $unknowsFields in the file vendor\a2lix\auto-form-bundle\src\Form\Manipulator\DoctrineORMManipulator.php my example from above works just fine...

I confirm the solution of the problem

DennisdeBest commented 1 year ago

Update 2:

If I uncomment the line with the $unknowsFields in the file vendor\a2lix\auto-form-bundle\src\Form\Manipulator\DoctrineORMManipulator.php my example from above works just fine...

Work for me to when adding translations in easyAdmin

DennisdeBest commented 1 year ago

The error disappears when the field has mapped set to false. However this prevents Vich of uploading the file and setting the filename.

To get this to work on my end I had to override some parts of the bundle.

First I extend the FormType to add valid_extra_fields

<?php

namespace App\Form\Type;

use A2lix\TranslationFormBundle\Form\Type\TranslationsType;
use Symfony\Component\Form\AbstractTypeExtension;
use Symfony\Component\OptionsResolver\OptionsResolver;

class TranslationsTypeExtension extends AbstractTypeExtension
{
    public static function getExtendedTypes(): iterable
    {
        return [TranslationsType::class];
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'excluded_fields' => ['id', 'locale', 'updatedAt', 'createdAt'],
            'valid_extra_fields' => [],
        ]);
    }
}

Then I add the values of my extra VichUploader fields in the TranslationField

<?php

declare(strict_types=1);

namespace App\Form\Field;

use A2lix\TranslationFormBundle\Form\Type\TranslationsType;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Field\FieldInterface;
use EasyCorp\Bundle\EasyAdminBundle\Field\FieldTrait;

final class TranslationField implements FieldInterface
{
    use FieldTrait;

    public static function new(string $propertyName, ?string $label = null, array $fieldsConfig = []): self
    {
        return (new self())
            ->setProperty($propertyName)
            ->setLabel($label)
            ->setFormType(TranslationsType::class)
            ->setFormTypeOptions([
                'default_locale' => 'nl',
                'fields' => $fieldsConfig,
                'excluded_fields' => ['createdAt', 'updatedAt', 'document', 'id', 'locale'],
                'valid_extra_fields' => ['documentFile' => []]
            ]);
    }
}

I then replace the DoctrineORMManipulator to take the new option into account.

<?php

namespace App\Form\Manipulator;

use A2lix\AutoFormBundle\Form\Manipulator\FormManipulatorInterface;
use A2lix\AutoFormBundle\ObjectInfo\DoctrineORMInfo;
use Symfony\Component\Form\FormInterface;

class DoctrineORMManipulator implements FormManipulatorInterface
{
    /** @var DoctrineORMInfo */
    private $doctrineORMInfo;
    /** @var array */
    private $globalExcludedFields;

    public function __construct(DoctrineORMInfo $doctrineORMInfo, array $globalExcludedFields = [])
    {
        $this->doctrineORMInfo = $doctrineORMInfo;
        $this->globalExcludedFields = $globalExcludedFields;
    }

    public function getFieldsConfig(FormInterface $form): array
    {
        $class = $this->getDataClass($form);
        $formOptions = $form->getConfig()->getOptions();

                //Get the parent options for the extra fields
        $parentOptions = $form->getParent()?->getConfig()->getOptions();

                //Get the values for the extra fields if set on this form or the parent
        $extraFields = $formOptions['valid_extra_fields'] ?? $parentOptions['valid_extra_fields'] ?? [];

        // Filtering to remove excludedFields
        $objectFieldsConfig = $this->doctrineORMInfo->getFieldsConfig($class);

        //merge the extra fields with the form fields
        $validObjectFieldsConfig = $this->filteringValidObjectFields(array_merge($objectFieldsConfig, $extraFields), $formOptions['excluded_fields']);

        if (empty($formOptions['fields'])) {
            return $validObjectFieldsConfig;
        }

        $fields = [];

        foreach ($formOptions['fields'] as $formFieldName => $formFieldConfig) {
            $this->checkFieldIsValid($formFieldName, $formFieldConfig, $validObjectFieldsConfig);

            if (null === $formFieldConfig) {
                continue;
            }

            // If display undesired, remove
            if (false === ($formFieldConfig['display'] ?? true)) {
                continue;
            }

            // Override with formFieldsConfig priority
            $fields[$formFieldName] = $formFieldConfig;

            if (isset($validObjectFieldsConfig[$formFieldName])) {
                $fields[$formFieldName] += $validObjectFieldsConfig[$formFieldName];
            }
        }

        return $fields + $validObjectFieldsConfig;
    }

    private function getDataClass(FormInterface $form): string
    {
               ...
    }

    private function filteringValidObjectFields(array $objectFieldsConfig, array $formExcludedFields): array
    {
        ...
    }

    private function checkFieldIsValid($formFieldName, $formFieldConfig, $validObjectFieldsConfig): void
    {
        if (isset($validObjectFieldsConfig[$formFieldName])) {
            return;
        }

        if (false === ($formFieldConfig['mapped'] ?? true)) {
            return;
        }

//Here set a valid error

        throw new \RuntimeException(sprintf("Field '%s' doesn't exist in %s", $formFieldName, $formFieldConfig['field_type']));
    }
}

I then register this class as a service :


services:
  a2lix_auto_form.manipulator.default:
    class: App\Form\Manipulator\DoctrineORMManipulator
    arguments:
      $doctrineORMInfo: '@a2lix_auto_form.object_info.doctrine_orm_info'

I can now add the field in my EasyAdmin controller, it renders correctly and works even when uploading multiple files.

        $translations = TranslationField::new('translations', 'translations', [
            'documentFile' => [
                'field_type' => VichFileType::class
            ],
        ]);

I will have a closer look to see if it is worth making a pull request and adding this to the bundle.

webda2l commented 1 year ago

https://github.com/a2lix/AutoFormBundle/releases/tag/0.4.4 should fix the main issue

DennisdeBest commented 1 year ago

Thanks @webda2l

wucherpfennig commented 1 year ago

@DennisdeBest @webda2l Thank you for your inputs. The udpate "fixes" the original error indeed. But my main issue persists:

No (with 0.4.4) no exception will be thrown but my "override" does not work anymore ;-) hence I had to downgrade to keep vich working...

so a PR with the solution from @DennisdeBest is very much appreciated ;-)