search

sonata-project / form-extensions

Symfony form extensions
https://docs.sonata-project.org/projects/form-extensions
MIT License
100 stars 26 forks source link

TypeError on ResizeFormListener since 1.17.0 #368

Closed willemverspyck closed 2 years ago

willemverspyck commented 2 years ago

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 4.12.0 4.12.0 The missing Symfony Admin Generator sonata-project/block-bundle 4.13.0 4.13.0 Symfony SonataBlockBundle sonata-project/cache 2.2.0 2.2.0 Cache library Package sonata-project/cache is abandoned, you should avoid using it. No replacement was suggested. sonata-project/doctrine-extensions 1.17.0 1.17.0 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 4.3.0 4.3.0 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/exporter 2.13.0 2.13.0 Lightweight Exporter library sonata-project/form-extensions 1.17.0 1.17.0 Symfony form extensions sonata-project/twig-extensions 1.10.0 1.10.0 Sonata twig extensions ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/amqp-messenger v6.1.0 v6.1.0 Symfony AMQP extension Messenger Bridge symfony/apache-pack v1.0.1 v1.0.1 A pack for Apache support in Symfony symfony/asset v6.1.0 v6.1.0 Manages URL generation and versioning of web assets such as CSS stylesheets, JavaScript files and image files symfony/cache v6.1.1 v6.1.1 Provides an extended PSR-6, PSR-16 (and tags) implementation symfony/cache-contracts v3.1.0 v3.1.0 Generic abstractions related to caching symfony/config v6.1.0 v6.1.0 Helps you find, load, combine, autofill and validate configuration values of any kind symfony/console v6.1.1 v6.1.1 Eases the creation of beautiful and testable command line interfaces symfony/css-selector v6.1.0 v6.1.0 Converts CSS selectors to XPath expressions symfony/debug-bundle v6.1.0 v6.1.0 Provides a tight integration of the Symfony VarDumper component and the ServerLogCommand from MonologBridge into the Symfony full-stack fr... symfony/dependency-injection v6.1.0 v6.1.0 Allows you to standardize and centralize the way objects are constructed in your application symfony/deprecation-contracts v3.1.0 v3.1.0 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge v6.1.0 v6.1.0 Provides integration for Doctrine with various Symfony components symfony/doctrine-messenger v6.1.1 v6.1.1 Symfony Doctrine Messenger Bridge symfony/dom-crawler v6.1.0 v6.1.0 Eases DOM navigation for HTML and XML documents symfony/dotenv v6.1.0 v6.1.0 Registers environment variables from a .env file symfony/error-handler v6.1.0 v6.1.0 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher v6.1.0 v6.1.0 Provides tools that allow your application components to communicate with each other by dispatching events and listening to them symfony/event-dispatcher-contracts v3.1.0 v3.1.0 Generic abstractions related to dispatching event symfony/expression-language v6.1.0 v6.1.0 Provides an engine that can compile and evaluate expressions symfony/filesystem v6.1.0 v6.1.0 Provides basic utilities for the filesystem symfony/finder v6.1.0 v6.1.0 Finds files and directories via an intuitive fluent interface symfony/flex v2.2.2 v2.2.2 Composer plugin for Symfony symfony/form v6.1.1 v6.1.1 Allows to easily create, process and reuse HTML forms symfony/framework-bundle v6.1.1 v6.1.1 Provides a tight integration between Symfony components and the Symfony full-stack framework symfony/http-client v6.1.1 v6.1.1 Provides powerful methods to fetch HTTP resources synchronously or asynchronously symfony/http-client-contracts v3.1.0 v3.1.0 Generic abstractions related to HTTP clients symfony/http-foundation v6.1.1 v6.1.1 Defines an object-oriented layer for the HTTP specification symfony/http-kernel v6.1.1 v6.1.1 Provides a structured process for converting a Request into a Response symfony/intl v6.1.0 v6.1.0 Provides a PHP replacement layer for the C intl extension that includes additional data from the ICU library symfony/lock v6.1.1 v6.1.1 Creates and manages locks, a mechanism to provide exclusive access to a shared resource symfony/mailer v6.1.1 v6.1.1 Helps sending emails symfony/maker-bundle v1.43.0 v1.43.0 Symfony Maker helps you create empty commands, controllers, form classes, tests and more so you can forget about writing boilerplate code. symfony/messenger v6.1.0 v6.1.0 Helps applications send and receive messages to/from other applications or via message queues symfony/mime v6.1.1 v6.1.1 Allows manipulating MIME messages symfony/monolog-bridge v6.1.1 v6.1.1 Provides integration for Monolog with various Symfony components symfony/monolog-bundle v3.8.0 v3.8.0 Symfony MonologBundle symfony/options-resolver v6.1.0 v6.1.0 Provides an improved replacement for the array_replace PHP function symfony/password-hasher v6.1.0 v6.1.0 Provides password hashing utilities symfony/phpunit-bridge v6.1.0 v6.1.0 Provides utilities for PHPUnit, especially user deprecation notices management symfony/polyfill-ctype v1.26.0 v1.26.0 Symfony polyfill for ctype functions symfony/polyfill-intl-grapheme v1.26.0 v1.26.0 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu v1.26.0 v1.26.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn v1.26.0 v1.26.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer v1.26.0 v1.26.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.26.0 v1.26.0 Symfony polyfill for the Mbstring extension symfony/polyfill-php72 v1.26.0 v1.26.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions symfony/polyfill-php80 v1.26.0 v1.26.0 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions symfony/process v6.1.0 v6.1.0 Executes commands in sub-processes symfony/property-access v6.1.0 v6.1.0 Provides functions to read and write from/to an object or array using a simple string notation symfony/property-info v6.1.1 v6.1.1 Extracts information about PHP class' properties using metadata of popular sources symfony/proxy-manager-bridge v6.1.0 v6.1.0 Provides integration for ProxyManager with various Symfony components symfony/psr-http-message-bridge v2.1.2 v2.1.2 PSR HTTP message bridge symfony/rate-limiter v6.1.0 v6.1.0 Provides a Token Bucket implementation to rate limit input and output in your application symfony/routing v6.1.1 v6.1.1 Maps an HTTP request to a set of configuration variables symfony/runtime v6.1.1 v6.1.1 Enables decoupling PHP applications from global state symfony/security-acl v3.3.1 v3.3.1 Symfony Security Component - ACL (Access Control List) symfony/security-bundle v6.1.0 v6.1.0 Provides a tight integration of the Security component into the Symfony full-stack framework symfony/security-core v6.1.0 v6.1.0 Symfony Security Component - Core Library symfony/security-csrf v6.1.0 v6.1.0 Symfony Security Component - CSRF Library symfony/security-http v6.1.1 v6.1.1 Symfony Security Component - HTTP Integration symfony/serializer v6.1.1 v6.1.1 Handles serializing and deserializing data structures, including object graphs, into array structures or other formats like XML and JSON. symfony/service-contracts v3.1.0 v3.1.0 Generic abstractions related to writing services symfony/stopwatch v6.1.0 v6.1.0 Provides a way to profile code symfony/string v6.1.0 v6.1.0 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way symfony/templating v6.1.0 v6.1.0 Provides all the tools needed to build any kind of template system symfony/translation v6.1.0 v6.1.0 Provides tools to internationalize your application symfony/translation-contracts v3.1.0 v3.1.0 Generic abstractions related to translation symfony/twig-bridge v6.1.0 v6.1.0 Provides integration for Twig with various Symfony components symfony/twig-bundle v6.1.1 v6.1.1 Provides a tight integration of Twig into the Symfony full-stack framework symfony/validator v6.1.1 v6.1.1 Provides tools to validate values symfony/var-dumper v6.1.0 v6.1.0 Provides mechanisms for walking through any arbitrary PHP variable symfony/var-exporter v6.1.1 v6.1.1 Allows exporting any serializable PHP data structure to plain PHP code symfony/web-link v6.1.0 v6.1.0 Manages links between resources symfony/web-profiler-bundle v6.1.1 v6.1.1 Provides a development tool that gives detailed information about the execution of any request symfony/yaml v6.1.0 v6.1.0 Loads and dumps YAML files ```

PHP version

$ php -v
PHP 8.1.4 (cli) (built: Apr  4 2022 13:30:17) (NTS)

Subject

Minimal repository with the bug

Company entity:

declare(strict_types=1);

namespace App\Entity;

use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\Common\Collections\Collection;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
use Stringable;
use Symfony\Component\Validator\Constraints as Validator;

#[ORM\Entity]
#[ORM\Table(name: 'company')]
class Company implements Stringable
{
    /**
     * @var int|null
     */
    #[ORM\Column(name: 'id', type: Types::INTEGER, options: ['unsigned' => true])]
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'IDENTITY')]
    private ?int $id = null;

    /**
     * @var string
     */
    #[ORM\Column(name: 'name', type: Types::STRING, length: 192, unique: true)]
    #[Validator\NotBlank]
    private string $name;

    /**
     * @var Collection<int, Contact>
     */
    #[ORM\OneToMany(mappedBy: 'company', targetEntity: Contact::class, cascade: ['persist'], orphanRemoval: true)]
    #[Validator\Valid]
    private Collection $contacts;

    public function __construct()
    {
        $this->contacts = new ArrayCollection();
    }

    /**
     * @return int|null
     */
    public function getId(): ?int
    {
        return $this->id;
    }

    /**
     * @param string $name
     *
     * @return $this
     */
    public function setName(string $name): self
    {
        $this->name = $name;

        return $this;
    }

    /**
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @param Contact $contact
     *
     * @return $this
     */
    public function addContact(Contact $contact): self
    {
        $contact->setCompany($this);

        $this->contacts->add($contact);

        return $this;
    }

    /**
     * @param Contact $contact
     */
    public function removeContact(Contact $contact): void
    {
        $this->contacts->removeElement($contact);
    }

    public function clearContacts(): void
    {
        $this->contacts->clear();
    }

    /**
     * @return Collection<int, Contact>
     */
    public function getContacts(): Collection
    {
        return $this->contacts;
    }

    /**
     * @return string
     */
    public function __toString(): string
    {
        return $this->getName();
    }
}

Contact entity (related with ManyToOne to Company)

declare(strict_types=1);

namespace App\Entity;

use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
use Stringable;
use Symfony\Component\Validator\Constraints as Validator;

#[ORM\Entity]
#[ORM\Table(name: 'contact')]
class Contact implements Stringable
{
    /**
     * @var int|null
     */
    #[ORM\Column(name: 'id', type: Types::INTEGER, options: ['unsigned' => true])]
    #[ORM\Id]
    #[ORM\GeneratedValue(strategy: 'IDENTITY')]
    private ?int $id = null;

    /**
     * @var Company
     */
    #[ORM\ManyToOne(targetEntity: Company::class, inversedBy: 'contacts')]
    #[ORM\JoinColumn(name: 'company_id', referencedColumnName: 'id', nullable: false)]
    private Company $company;

    /**
     * @var string
     */
    #[ORM\Column(name: 'name', type: Types::STRING, length: 192, nullable: false)]
    #[Validator\NotBlank]
    private string $name;

    /**
     * @return int|null
     */
    public function getId(): ?int
    {
        return $this->id;
    }

    /**
     * @param Company $company
     *
     * @return $this
     */
    public function setCompany(Company $company): self
    {
        $this->company = $company;

        return $this;
    }

    /**
     * @return Company
     */
    public function getCompany(): Company
    {
        return $this->company;
    }

    /**
     * @param string $name
     *
     * @return $this
     */
    public function setName(string $name): self
    {
        $this->name = $name;

        return $this;
    }

    /**
     * @return string
     */
    public function getName(): string
    {
        return $this->name;
    }

    /**
     * @return string
     */
    public function __toString(): string
    {
        return $this->getName();
    }
}

Contact Admin:

declare(strict_types=1);

namespace App\Admin;

use App\Entity\Contact;
use App\Translator\AdminTranslator;
use Sonata\AdminBundle\Form\FormMapper;
use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;

#[AutoconfigureTag('sonata.admin', [
    'manager_type' => 'orm',
    'model_class' => Contact::class,
    'label_translator_strategy' => AdminTranslator::class,
])]
final class ContactAdmin extends AbstractAdmin
{
    /**
     * @var array
     */
    protected array $removeRoutes = ['delete', 'show'];

    /**
     * @inheritDoc
     */
    protected function configureFormFields(FormMapper $formMapper): void
    {
        $formMapper
            ->add('name', null, [
                'required' => true,
            ]);
    }
}

Company Admin:

<?php

declare(strict_types=1);

namespace App\Admin;

use App\Entity\Company;
use App\Translator\AdminTranslator;
use Sonata\AdminBundle\Datagrid\ListMapper;
use Sonata\AdminBundle\Datagrid\DatagridMapper;
use Sonata\AdminBundle\FieldDescription\FieldDescriptionInterface;
use Sonata\AdminBundle\Form\FormMapper;
use Sonata\AdminBundle\Show\ShowMapper;
use Sonata\DoctrineORMAdminBundle\Filter\CallbackFilter;
use Sonata\DoctrineORMAdminBundle\Filter\ChoiceFilter;
use Sonata\Form\Type\CollectionType;
use Symfony\Component\DependencyInjection\Attribute\AutoconfigureTag;
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;

#[AutoconfigureTag('sonata.admin', [
    'manager_type' => 'orm',
    'model_class' => Company::class,
    'label' => 'Company',
    'label_translator_strategy' => AdminTranslator::class,
])]
final class CompanyAdmin extends AbstractAdmin
{
    /**
     * @var array
     */
    protected array $removeRoutes = ['delete'];

    /**
     * @inheritDoc
     */
    protected function configureFormFields(FormMapper $formMapper): void
    {
        $formMapper
            ->tab('tab1')
                ->with('tab1_block1')
                    ->add('name', null, [
                        'required' => true,
                    ])
                    ->add('contacts', CollectionType::class, [
                        'required' => false,
                    ], ['edit' => 'inline', 'inline' => 'table'])
                ->end()
            ->end();
    }

    /**
     * @inheritDoc
     */
    protected function configureListFields(ListMapper $listMapper): void
    {
        $listMapper
            ->add('name')
            ->add(ListMapper::NAME_ACTIONS, null, [
                'actions' => [
                    'show' => [],
                    'edit' => [],
                    'delete' => [],
                ],
            ]);
    }
}

Steps to reproduce

If I click "Edit" or "Create new" company and press "Add new", to add a related contact entity, I get the error "Symfony\Component\Form\Form::remove(): Argument #1 ($name) must be of type string, int given, called in /data/vendor/sonata-project/form-extensions/src/EventListener/ResizeFormListener.php on line 92"

The problem started in https://github.com/sonata-project/form-extensions/commit/0898fa766627a5a5a61f3ecf7a75b3748314f523, where the type cast is removed.

Expected results

Allow to insert new "Contact".

Actual results

I get a PHP TypeError.

jordisala1991 commented 2 years ago

Can you provide a PR with a revert of those 2 lines (the type cast) + a comment to not forget it again?

bonus points if you can provide a test that ensures we don't remove that typecast again.

willemverspyck commented 2 years ago

Hi @jordisala1991. Reverted those 2 lines, added comments and include test for both "remove" and "has" method.

Looking in the Symfony Form code, I think this is a problem at Symfony, because the form creates integer keys in some situations (like expanded as option at "ChoiceType") and the interface only allows "string".

VincentLanglet commented 2 years ago

HI @willemverspyck, do you know in the Sonata code where is coming from the int key ? We might consider changing add('0') to something like add('prefix_0').

mpoiriert commented 2 years ago

I saw that symfony 4.4 was not type hinting method to string. I believe myself that it should allowed string and int since Collection type also use int.

What is weird is behaviour of php about array key

$array = ['test1', 'test2', '2' => 'test3'];

var_dump(isset($array[0])); // true
var_dump(isset($array['0'])); // true

var_dump(isset($array[1])); // true
var_dump(isset($array['1'])); // true

var_dump(isset($array[2])); // true
var_dump(isset($array['2'])); // true

var_export($array[2]); // test3

So base on that probably they made the mistake of casting it to string thinking it will have not impact :/

toooni commented 1 year ago

Is it possible that we have the exact same issue here right now: https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Util/FormBuilderIterator.php#L67

I've got a \Sonata\Form\Type\CollectionType with FieldDescriptionOptions [ 'edit' => 'inline', 'inline' => 'table', 'sortable' => 'id', ]

and am getting the same error. If I add casting to string like this return $this->formBuilder->get((string) $this->iterator->current());, everything works perfectly!

VincentLanglet commented 1 year ago

Can you provide a PR @toooni with a unit test ?

willemverspyck commented 1 year ago

@toooni : When do you get the error? When you press the "Add new" button in the CollectionType? If yes, you probably have a ChoiceType field like this:

  ->add('type', ChoiceType::class, [
      'choices' => Company::getCompanyTypes(true),
      /* @temporary Until fixed in Symfony/form */
      'choice_name' => function (string $choice, string $key, string $value): string {
          return sprintf('value_%s', $value);
      },
      'expanded' => true,
      'required' => true,
  ])
  ->add('contacts', CollectionType::class, [
      'required' => false,
  ], ['edit' => 'inline', 'inline' => 'table'])

I added this "choice_name" to temporary fix the problem, but I think the problem is related to Symfony/Form, because ChoiceType generates integer key, but the interface of Symfony Form only allows string names.

toooni commented 1 year ago

@VincentLanglet I'll try. @willemverspyck You are right. I've found the culprit. Funnily, the ChoiceType is on the parent, on the same level as the property which has the CollectionType.

toooni commented 1 year ago

@VincentLanglet You can check this PR: https://github.com/sonata-project/SonataAdminBundle/pull/7983/files

Please see my comment on why I've changed the expected result from an unrelated test. I've also removed the class property factory and replaced it with a var $factory inside setUp since it's not used anywhere else.