sonata-project / SonataFormatterBundle

Symfony SonataFormatterBundle
https://docs.sonata-project.org/projects/SonataFormatterBundle
MIT License
81 stars 117 forks source link

[Branch 5.x] FormatterListener not called when using FormatterType #715

Closed shiroko closed 2 years ago

shiroko commented 2 years ago

Environment

PHP 8.1.9 Symfony 5.4.11

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' Direct dependencies: sonata-project/admin-bundle 4.16.0 4.18.0 The missing Symfony Admin Generator sonata-project/block-bundle 4.16.1 4.16.2 Symfony SonataBlockBundle sonata-project/classification-bundle 4.3.0 4.3.0 Symfony SonataClassificationBundle sonata-project/doctrine-orm-admin-bundle 4.4.0 4.6.0 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/entity-audit-bundle 1.8.0 1.8.0 Audit for Doctrine Entities sonata-project/formatter-bundle 5.0.0 5.0.0 Symfony SonataFormatterBundle sonata-project/media-bundle 4.5.0 4.5.0 Symfony SonataMediaBundle sonata-project/seo-bundle 3.2.0 3.2.0 Symfony SonataSeoBundle sonata-project/user-bundle 5.3.2 5.3.3 Symfony SonataUserBundle Transitive dependencies: 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 2.0.1 Doctrine2 behavioral extensions sonata-project/exporter 3.0.0 3.0.0 Lightweight Exporter library sonata-project/form-extensions 1.18.0 1.18.0 Symfony form extensions sonata-project/twig-extensions 1.12.0 2.0.0 Sonata twig extensions ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' Direct dependencies: symfony/asset v5.4.7 v6.1.0 Manages URL generation and versioning of web assets such as CSS stylesheets, JavaScript files and image files symfony/browser-kit v5.4.11 v6.1.3 Simulates the behavior of a web browser, allowing you to make requests, click on links and submit forms programmatically symfony/console v5.4.11 v6.1.3 Eases the creation of beautiful and testable command line interfaces symfony/css-selector v5.4.11 v6.1.3 Converts CSS selectors to XPath expressions symfony/debug-bundle v5.4.11 v6.1.3 Provides a tight integration of the Symfony VarDumper component and the ServerLogCommand from MonologBridge into the Symfony full-stack ... symfony/dotenv v5.4.5 v6.1.0 Registers environment variables from a .env file symfony/expression-language v5.4.11 v6.1.3 Provides an engine that can compile and evaluate expressions symfony/flex v1.19.2 v2.2.3 Composer plugin for Symfony symfony/form v5.4.11 v6.1.3 Allows to easily create, process and reuse HTML forms symfony/framework-bundle v5.4.11 v6.1.3 Provides a tight integration between Symfony components and the Symfony full-stack framework symfony/http-client v5.4.11 v6.1.3 Provides powerful methods to fetch HTTP resources synchronously or asynchronously symfony/intl v5.4.11 v6.1.0 Provides a PHP replacement layer for the C intl extension that includes additional data from the ICU library symfony/mailer v5.4.11 v6.1.3 Helps sending emails symfony/maker-bundle v1.45.0 v1.45.0 Symfony Maker helps you create empty commands, controllers, form classes, tests and more so you can forget about writing boilerplate code. symfony/messenger v5.4.11 v6.1.3 Helps applications send and receive messages to/from other applications or via message queues symfony/monolog-bundle v3.8.0 v3.8.0 Symfony MonologBundle symfony/phpunit-bridge v6.1.3 v6.1.3 Provides utilities for PHPUnit, especially user deprecation notices management symfony/process v5.4.11 v6.1.3 Executes commands in sub-processes symfony/property-access v5.4.11 v6.1.3 Provides functions to read and write from/to an object or array using a simple string notation symfony/property-info v5.4.11 v6.1.3 Extracts information about PHP class' properties using metadata of popular sources symfony/proxy-manager-bridge v5.4.6 v6.1.0 Provides integration for ProxyManager with various Symfony components symfony/runtime v5.4.11 v6.1.3 Enables decoupling PHP applications from global state symfony/security-bundle v5.4.11 v6.1.3 Provides a tight integration of the Security component into the Symfony full-stack framework symfony/sendgrid-mailer v5.4.7 v6.1.0 Symfony Sendgrid Mailer Bridge symfony/serializer v5.4.11 v6.1.3 Handles serializing and deserializing data structures, including object graphs, into array structures or other formats like XML and JSON. symfony/stopwatch v5.4.5 v6.1.0 Provides a way to profile code symfony/translation v5.4.11 v6.1.3 Provides tools to internationalize your application symfony/twig-bundle v5.4.8 v6.1.1 Provides a tight integration of Twig into the Symfony full-stack framework symfony/uid v5.4.11 v6.1.3 Provides an object-oriented API to generate and represent UIDs symfony/validator v5.4.11 v6.1.3 Provides tools to validate values symfony/var-dumper v5.4.11 v6.1.3 Provides mechanisms for walking through any arbitrary PHP variable symfony/web-link v5.4.3 v6.1.0 Manages links between resources symfony/web-profiler-bundle v5.4.10 v6.1.2 Provides a development tool that gives detailed information about the execution of any request symfony/webpack-encore-bundle v1.15.1 v1.15.1 Integration with your Symfony app & Webpack Encore! symfony/yaml v5.4.11 v6.1.3 Loads and dumps YAML files Transitive dependencies: symfony/amqp-messenger v5.4.11 v5.4.11 Symfony AMQP extension Messenger Bridge symfony/cache v5.4.11 v6.1.3 Provides an extended PSR-6, PSR-16 (and tags) implementation symfony/cache-contracts v2.5.2 v3.1.1 Generic abstractions related to caching symfony/config v5.4.11 v6.1.3 Helps you find, load, combine, autofill and validate configuration values of any kind symfony/dependency-injection v5.4.11 v6.1.3 Allows you to standardize and centralize the way objects are constructed in your application symfony/deprecation-contracts v3.1.1 v3.1.1 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge v5.4.11 v6.1.3 Provides integration for Doctrine with various Symfony components symfony/doctrine-messenger v5.4.11 v6.1.3 Symfony Doctrine Messenger Bridge symfony/dom-crawler v5.4.11 v6.1.3 Eases DOM navigation for HTML and XML documents symfony/error-handler v5.4.11 v6.1.3 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher v5.4.9 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.1 v3.1.1 Generic abstractions related to dispatching event symfony/filesystem v5.4.11 v6.1.3 Provides basic utilities for the filesystem symfony/finder v5.4.11 v6.1.3 Finds files and directories via an intuitive fluent interface symfony/http-client-contracts v2.5.2 v3.1.1 Generic abstractions related to HTTP clients symfony/http-foundation v5.4.11 v6.1.3 Defines an object-oriented layer for the HTTP specification symfony/http-kernel v5.4.11 v6.1.3 Provides a structured process for converting a Request into a Response symfony/mime v5.4.11 v6.1.3 Allows manipulating MIME messages symfony/monolog-bridge v5.4.10 v6.1.2 Provides integration for Monolog with various Symfony components symfony/options-resolver v5.4.11 v6.1.0 Provides an improved replacement for the array_replace PHP function symfony/password-hasher v5.4.11 v6.1.3 Provides password hashing utilities 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-php73 v1.26.0 v1.26.0 Symfony polyfill backporting some PHP 7.3+ 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/polyfill-php81 v1.26.0 v1.26.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions symfony/polyfill-uuid v1.26.0 v1.26.0 Symfony polyfill for uuid functions symfony/redis-messenger v5.4.6 v6.1.3 Symfony Redis extension Messenger Bridge symfony/routing v5.4.11 v6.1.3 Maps an HTTP request to a set of configuration variables symfony/security-acl v3.3.1 v3.3.1 Symfony Security Component - ACL (Access Control List) symfony/security-core v5.4.11 v6.1.3 Symfony Security Component - Core Library symfony/security-csrf v5.4.11 v6.1.0 Symfony Security Component - CSRF Library symfony/security-guard v5.4.9 v5.4.9 Symfony Security Component - Guard symfony/security-http v5.4.11 v6.1.3 Symfony Security Component - HTTP Integration symfony/service-contracts v2.5.2 v3.1.1 Generic abstractions related to writing services symfony/string v5.4.11 v6.1.3 Provides an object-oriented API to strings and deals with bytes, UTF-8 code points and grapheme clusters in a unified way symfony/translation-contracts v2.5.2 v3.1.1 Generic abstractions related to translation symfony/twig-bridge v5.4.11 v6.1.3 Provides integration for Twig with various Symfony components symfony/var-exporter v5.4.10 v6.1.3 Allows exporting any serializable PHP data structure to plain PHP code ```

PHP version

$ php -v
PHP 8.1.9 (cli) (built: Aug  4 2022 21:09:01) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.9, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.9, Copyright (c), by Zend Technologies
    with Xdebug v3.1.5, Copyright (c) 2002-2022, by Derick Rethans

Subject

When a FormatterType widget is used, the transformed content (target_field) is not set because FormatterListener is not called when the form is submitted.

configureFormFields() as follows:

                ->add('content', FormatterType::class, [
                    'label' => 'content',
                    'required' => true,
                    'format_field' => 'contentFormatter',
                    'format_field_options' => [
                        'choices' => [
                            'Rich HTML' => 'richhtml',
                            'Raw HTML' => 'rawhtml',
                        ],
                        'empty_data' => 'richhtml',
                    ],
                    'source_field' => 'rawContent',
                    'ckeditor_context' => 'news',
                    'ckeditor_image_format' => 'reference',
                    'target_field' => 'content',
                    'listener' => true,
                ])

form as follows: formatter-form

A FormatterListener is registered for the "content" form's form.submit listeners. The "content" form is set with inherit_data: true, so the "content" form's submit process does not call the FormEvent::SUBMIT event.

Prior to 5.x, FormatListener was called by the 'event_dispatcher' option, but in 5.x the 'event_dispatcher' option has been removed.

Minimal repository with the bug

https://github.com/sonata-project/SonataFormatterBundle/tree/7ee53e723c3c4c3c576d771954a18920127307f6

Steps to reproduce

Install the Sonata stack with the version I use Install Ckeditor Configure a Sonata Admin with a Ckeditor field Edit the Ckeditor field on Admin create page Submit the form ( by clicking the btn_create_and_edit_again button )

Expected results

transformed content (target_field) is set and created successfully.

Actual results

transformed content (target_field) is not set and NotBlank form error occurs.

VincentLanglet commented 2 years ago

cc @jordisala1991

Call was here: https://github.com/sonata-project/SonataFormatterBundle/blob/4.x/src/Form/Type/FormatterType.php#L167-L174 And now it's: https://github.com/sonata-project/SonataFormatterBundle/blame/5.x/src/Form/Type/FormatterType.php#L70

Seems like the behavior is not the same

haivala commented 2 years ago

Seems like this the same problem why this block does not work with SonataPageBundle 4.x

jordisala1991 commented 2 years ago

Did the last commit on 5.x fixes your issue @shiroko ?

haivala commented 2 years ago

I'm guessing it did not because I did need to change the FormatterType in any way.

When you define the settings to that FormatterType do you need to set all of those settings? Does this work?

->add('content', FormatterType::class, [
                    'label' => 'content',
                    'required' => true,
                    'ckeditor_context' => 'news',
                    'ckeditor_image_format' => 'reference',
                ])
shiroko commented 2 years ago

It was not fixed even in the latest 5.x branch source.

It worked fine in FormatterType with the event_dispatcher option added.

*** vendor/sonata-project/formatter-bundle/src/Form/Type/FormatterType.php      2022-09-26 00:39:25.000000000 +0900
--- src/Form/Type/FormatterType.php     2022-09-26 18:51:38.835046000 +0900
***************
*** 11,21 ****
   * file that was distributed with this source code.
   */

! namespace Sonata\FormatterBundle\Form\Type;

  use FOS\CKEditorBundle\Config\CKEditorConfigurationInterface;
  use Sonata\FormatterBundle\Form\EventListener\FormatterListener;
  use Sonata\FormatterBundle\Formatter\PoolInterface;
  use Symfony\Component\Form\AbstractType;
  use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
  use Symfony\Component\Form\Extension\Core\Type\HiddenType;
--- 11,22 ----
   * file that was distributed with this source code.
   */

! namespace App\Form\Type;

  use FOS\CKEditorBundle\Config\CKEditorConfigurationInterface;
  use Sonata\FormatterBundle\Form\EventListener\FormatterListener;
  use Sonata\FormatterBundle\Formatter\PoolInterface;
+ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
  use Symfony\Component\Form\AbstractType;
  use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
  use Symfony\Component\Form\Extension\Core\Type\HiddenType;
***************
*** 28,43 ****

  final class FormatterType extends AbstractType
  {
-     private PoolInterface $pool;
- 
-     private CKEditorConfigurationInterface $ckEditorConfiguration;
- 
      public function __construct(
!         PoolInterface $pool,
!         CKEditorConfigurationInterface $ckEditorConfiguration
      ) {
-         $this->pool = $pool;
-         $this->ckEditorConfiguration = $ckEditorConfiguration;
      }

      public function buildForm(FormBuilderInterface $builder, array $options): void
--- 29,38 ----

  final class FormatterType extends AbstractType
  {
      public function __construct(
!         private readonly PoolInterface $pool,
!         private readonly CKEditorConfigurationInterface $ckEditorConfiguration
      ) {
      }

      public function buildForm(FormBuilderInterface $builder, array $options): void
***************
*** 68,73 ****
--- 63,72 ----
           * The listener option only work if the source field is after the current field
           */
          if (true === $options['listener']) {
+             if (!$options['event_dispatcher'] instanceof EventDispatcherInterface) {
+                 throw new \RuntimeException('The event_dispatcher option must be an instance of EventDispatcherInterface');
+             }
+ 
              $listener = new FormatterListener(
                  $this->pool,
                  $formatOptions['property_path'] ?? $formatField,
***************
*** 75,81 ****
                  $options['target_field']
              );

!             $builder->addEventListener(FormEvents::SUBMIT, [$listener, 'postSubmit']);
          }
      }

--- 74,80 ----
                  $options['target_field']
              );

!             $options['event_dispatcher']->addListener(FormEvents::SUBMIT, $listener->postSubmit(...));
          }
      }

***************
*** 116,121 ****
--- 115,121 ----
      {
          $resolver->setDefaults([
              'inherit_data' => true,
+             'event_dispatcher' => null,
              'ckeditor_toolbar_icons' => [
                  [
                      'Bold', 'Italic', 'Underline',

@haivala

                ->add('content', FormatterType::class, [
                    'label' => 'content',
                    'required' => true,
                    'ckeditor_context' => 'news',
                    'ckeditor_image_format' => 'reference',
                ])

The following error occurred in this setup:

An error has occurred resolving the options of the form "Sonata\FormatterBundle\Form\Type\FormatterType": The required options "format_field", "source_field" are missing.

jordisala1991 commented 2 years ago

What I dont understand about your setup is what is the difference between @haivala one, and with the functional tests already present on this bundle. Can you share a reproducer of the issue?

shiroko commented 2 years ago

I created a reproducer of this issue. https://github.com/shiroko/sonata_formatter_bundle_issue_715

Sonata media is not included for minimization.

If you use App\Form\Type\FormatterType and set the event_dispatcher option in PostAdmin, it will work fine.

jordisala1991 commented 2 years ago

I does not work indeed, I need to figure out how to change test to properly see this issue. Also, what I don't really understand is that we are using this behavior of adding submit events to the builder on other places like: https://github.com/sonata-project/SonataMediaBundle/blob/4.x/src/Form/Type/MediaType.php#L64-L68 and it is working there.

why does not work here? Do you have any idea?

shiroko commented 2 years ago

In FormatterType, the inherit_data option is set to true by default, and in Symfony Form if inherit_data is true, the FormEvents::SUBMIT event is not called. https://github.com/symfony/form/blob/6.1/Form.php#L578-L622

When I set inherit_data to false, I get the following error in FormatterListener: Cannot read property "contentFormatter" from an array. Maybe you intended to write the property path as "[contentFormatter]" instead.

Registering a FormatterListener with FormEvents::POST_SUBMIT instead of FormEvents::SUBMIT resulted in the following error: Symfony\Component\PropertyAccess\PropertyAccessor::getValue(): Argument #1 ($objectOrArray) must be of type object|array, null given, called in /var/www/bug_app/vendor/sonata-project/formatter-bundle/src/Form/EventListener/FormatterListener.php on line 43

How about adding an event_dispatcher option as in 4.x branch?