Fix getSubject method when admin has parentFieldDescription

[EmmanuelVella]

Changelog

### Fixed
- Fixed AbstractAdmin::getSubject on admins with parentFieldDescription

To do

• Update the tests

Subject

When an admin has a parentFieldDescription, the AbstractAdmin::getSubject method should not use the url id parameter to load the object (because the id correspond to the root object only).

Example: I have an issue when submitting a form with a collection which contains an AdminType field. In AdminType, this line loads a (random) collection element by using the url id (which is the root object id) : https://github.com/sonata-project/SonataAdminBundle/blob/3.x/Form/Type/AdminType.php#L57

Should fix #2879

[EmmanuelVella]

PR rebased, tests are now ok !

[jlamur]

Does this fix #4556?

[EmmanuelVella]

@jlamur No sorry, that's not related !

[jlamur]

Nevermind I confused parentFieldDescription with parentAssociationMapping.

[greg0ire]

Thanks @EmmanuelVella !

[davidromani]

This change has broken my app because is firing extra (and unnecessary) calls to configureFormFields method after click on update button in edit form view.

I've put a breakpoint inside a configureFormFields method, in v3.20.1 and when I click on update button, my debugger never stops there. After this change of code, the debugger stops there with the same use case.

This behaviour brokes my app becuase it's executing a "strange mess". I my code, I'm using this kind of relations:

$formMapper
    ->add(
        'auditWindmillBlades',
        CollectionType::class,
        array(
             'label' => ' ',
             'required' => false,
             'btn_add' => false,
             'cascade_validation' => true,
             'error_bubbling' => true,
             'type_options' => array(
                 'delete' => false,
             ),
         ),
         array(
             'edit' => 'inline',
             'inline' => 'table',
         )
     )

Should I open a new issue?

[greg0ire]

Should I open a new issue?

Probably. @EmmanuelVella can you help?

[EmmanuelVella]

Hello @davidromani, I tried with a (native) collection type, submitted the form with success.

Could you please elaborate (more details about what happens) and tell me how to reproduce this behavior ?

[davidromani]

@EmmanuelVella yes, I've a public repository of my app (with fixtures, easy to install and deploy) if you want to try it, or I can describe my entity relations and my Admin form mapper. Plus I have some Sonata hooks defined (pre-persist, per-update). What do you prefer?

Another question what "native collection type" means? My app still broken If I use "sonata_type_collection" insetad of CollectionType::class declarations if you are thinking about it.

[EmmanuelVella]

@davidromani I tried both collection types (sonata_type_collection and sonata_type_collection) with no problem. Could you please give me the url of your repo and tell me where you use the broken sonata_type_collection ?

[greg0ire]

sonata_type_collection and sonata_type_collection

Hmmmmm

[davidromani]

@EmmanuelVella the problem is focused with parent and child admins and how this affect to the getSubject() method call behaviour. Please check if $this->hasParentFieldDescription() call can cause instance “construction” problems or fire some unexpected behaviour.

SonataAdminBundle/Admin/AbstractAdmin.php

Line 1689 in f7534c2

public function getSubject()

[EmmanuelVella]

@greg0ire I meant (sonata_type_native_collection and sonata_type_collection ;)

[davidromani]

@EmmanuelVella you can try with my real app cloning this repo https://github.com/Flexible-User-Experience/fibervent & checkout to this commit https://github.com/Flexible-User-Experience/fibervent/commit/115d0336359fc5bf54cc3aa56a96bec04aad6096

Before to downgrade Sonata up to v3.20.1 it fails. Next commit (https://github.com/Flexible-User-Experience/fibervent/commit/f8f33b0c1640087c8f1d90f393bda58a2e625cc9) it works well.

[EmmanuelVella]

@davidromani Yes I get that. I did this PR to fix an issue where the subject was incorrect in case of embedded forms (the ID in the url was used to load the children).

I tried to execute your project but there are too many requirements, it would be too long to install for me. Could you paste me the stack trace please ?

[EmmanuelVella]

BTW I highly suspect this behavior : #2076

You seem to use a sonata collection, with an admin which itself contains another embedded admin. In this case, getSubject() call in the embedded admin used to "work" before the PR but was returning the wrong object.

I don't see any solution, as the subject is set in the embedded admin once the form data is set, so you can't use getSubject() before this happens.

To summarize :

• CollectionType of Admin => getSubject is ok
• CollectionType of Admin, containing an AdminType => getSubject is not ok

Hope that's clear...

[davidromani]

[1] Symfony\Component\Debug\Exception\FatalThrowableError: Call to a member function getWindmillBlade() on null
    at n/a
        in /Users/david/Desenvolupament/Symfony/fibervent/src/AppBundle/Admin/AuditWindmillBladeAdmin.php line 86

    at AppBundle\Admin\AuditWindmillBladeAdmin->configureFormFields(object(FormMapper))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/sonata-project/admin-bundle/Admin/AbstractAdmin.php line 1292

    at Sonata\AdminBundle\Admin\AbstractAdmin->defineFormBuilder(object(FormBuilder))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/sonata-project/admin-bundle/Form/Type/AdminType.php line 86

    at Sonata\AdminBundle\Form\Type\AdminType->buildForm(object(FormBuilder), array('block_name' => null, 'disabled' => false, 'label' => null, 'label_format' => null, 'translation_domain' => null, 'trim' => true, 'required' => true, 'max_length' => null, 'pattern' => null, 'mapped' => true, 'by_reference' => true, 'virtual' => null, 'compound' => true, 'method' => 'POST', 'action' => '', 'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.', 'error_mapping' => array(), 'invalid_message' => 'This value is not valid.', 'invalid_message_parameters' => array(), 'allow_extra_fields' => false, 'extra_fields_message' => 'This form should not contain extra fields.', 'csrf_protection' => true, 'csrf_field_name' => '_token', 'csrf_message' => 'The CSRF token is invalid. Please try to resubmit the form.', 'intention' => null, 'sonata_admin' => null, 'label_render' => true, 'sonata_help' => null, 'horizontal_label_class' => '', 'horizontal_label_offset_class' => '', 'horizontal_input_wrapper_class' => '', 'delete_options' => array('type' => 'checkbox', 'type_options' => array('required' => false, 'mapped' => false)), 'btn_add' => 'link_add', 'btn_list' => 'link_list', 'btn_delete' => 'link_delete', 'btn_catalogue' => 'SonataAdminBundle', 'read_only' => false, 'attr' => array(), 'auto_initialize' => false, 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'empty_data' => object(Closure), 'property_path' => '[0]', 'error_bubbling' => true, 'label_attr' => array(), 'inherit_data' => false, 'upload_max_size_message' => object(Closure), 'validation_groups' => null, 'constraints' => array(), 'cascade_validation' => false, 'csrf_provider' => object(CsrfTokenManager), 'csrf_token_manager' => object(CsrfTokenManager), 'csrf_token_id' => null, 'sonata_field_description' => object(FieldDescription), 'delete' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/ResolvedFormType.php line 187

    at Symfony\Component\Form\ResolvedFormType->buildForm(object(FormBuilder), array('block_name' => null, 'disabled' => false, 'label' => null, 'label_format' => null, 'translation_domain' => null, 'trim' => true, 'required' => true, 'max_length' => null, 'pattern' => null, 'mapped' => true, 'by_reference' => true, 'virtual' => null, 'compound' => true, 'method' => 'POST', 'action' => '', 'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.', 'error_mapping' => array(), 'invalid_message' => 'This value is not valid.', 'invalid_message_parameters' => array(), 'allow_extra_fields' => false, 'extra_fields_message' => 'This form should not contain extra fields.', 'csrf_protection' => true, 'csrf_field_name' => '_token', 'csrf_message' => 'The CSRF token is invalid. Please try to resubmit the form.', 'intention' => null, 'sonata_admin' => null, 'label_render' => true, 'sonata_help' => null, 'horizontal_label_class' => '', 'horizontal_label_offset_class' => '', 'horizontal_input_wrapper_class' => '', 'delete_options' => array('type' => 'checkbox', 'type_options' => array('required' => false, 'mapped' => false)), 'btn_add' => 'link_add', 'btn_list' => 'link_list', 'btn_delete' => 'link_delete', 'btn_catalogue' => 'SonataAdminBundle', 'read_only' => false, 'attr' => array(), 'auto_initialize' => false, 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'empty_data' => object(Closure), 'property_path' => '[0]', 'error_bubbling' => true, 'label_attr' => array(), 'inherit_data' => false, 'upload_max_size_message' => object(Closure), 'validation_groups' => null, 'constraints' => array(), 'cascade_validation' => false, 'csrf_provider' => object(CsrfTokenManager), 'csrf_token_manager' => object(CsrfTokenManager), 'csrf_token_id' => null, 'sonata_field_description' => object(FieldDescription), 'delete' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/DataCollector/Proxy/ResolvedTypeDataCollectorProxy.php line 110

    at Symfony\Component\Form\Extension\DataCollector\Proxy\ResolvedTypeDataCollectorProxy->buildForm(object(FormBuilder), array('block_name' => null, 'disabled' => false, 'label' => null, 'label_format' => null, 'translation_domain' => null, 'trim' => true, 'required' => true, 'max_length' => null, 'pattern' => null, 'mapped' => true, 'by_reference' => true, 'virtual' => null, 'compound' => true, 'method' => 'POST', 'action' => '', 'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.', 'error_mapping' => array(), 'invalid_message' => 'This value is not valid.', 'invalid_message_parameters' => array(), 'allow_extra_fields' => false, 'extra_fields_message' => 'This form should not contain extra fields.', 'csrf_protection' => true, 'csrf_field_name' => '_token', 'csrf_message' => 'The CSRF token is invalid. Please try to resubmit the form.', 'intention' => null, 'sonata_admin' => null, 'label_render' => true, 'sonata_help' => null, 'horizontal_label_class' => '', 'horizontal_label_offset_class' => '', 'horizontal_input_wrapper_class' => '', 'delete_options' => array('type' => 'checkbox', 'type_options' => array('required' => false, 'mapped' => false)), 'btn_add' => 'link_add', 'btn_list' => 'link_list', 'btn_delete' => 'link_delete', 'btn_catalogue' => 'SonataAdminBundle', 'read_only' => false, 'attr' => array(), 'auto_initialize' => false, 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'empty_data' => object(Closure), 'property_path' => '[0]', 'error_bubbling' => true, 'label_attr' => array(), 'inherit_data' => false, 'upload_max_size_message' => object(Closure), 'validation_groups' => null, 'constraints' => array(), 'cascade_validation' => false, 'csrf_provider' => object(CsrfTokenManager), 'csrf_token_manager' => object(CsrfTokenManager), 'csrf_token_id' => null, 'sonata_field_description' => object(FieldDescription), 'delete' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/FormFactory.php line 118

    at Symfony\Component\Form\FormFactory->createNamedBuilder('0', object(ResolvedTypeDataCollectorProxy), null, array('sonata_field_description' => object(FieldDescription), 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'delete' => false, 'property_path' => '[0]', 'auto_initialize' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/FormFactory.php line 48

    at Symfony\Component\Form\FormFactory->createNamed('0', 'sonata_type_admin', null, array('sonata_field_description' => object(FieldDescription), 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'delete' => false, 'property_path' => '[0]', 'auto_initialize' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Form.php line 917

    at Symfony\Component\Form\Form->add('0', 'sonata_type_admin', array('sonata_field_description' => object(FieldDescription), 'data_class' => 'AppBundle\Entity\AuditWindmillBlade', 'delete' => false, 'property_path' => '[0]', 'auto_initialize' => false))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/sonata-project/core-bundle/Form/EventListener/ResizeFormListener.php line 175

    at Sonata\CoreBundle\Form\EventListener\ResizeFormListener->preSubmit(object(FormEvent))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/sonata-project/core-bundle/Form/EventListener/ResizeFormListener.php line 132

    at Sonata\CoreBundle\Form\EventListener\ResizeFormListener->preBind(object(FormEvent), 'form.pre_bind', object(EventDispatcher))
        in  line 

    at call_user_func(array(object(ResizeFormListener), 'preBind'), object(FormEvent), 'form.pre_bind', object(EventDispatcher))
        in /Users/david/Desenvolupament/Symfony/fibervent/app/cache/dev/classes.php line 1871

    at Symfony\Component\EventDispatcher\EventDispatcher->doDispatch(array(array(object(BindRequestListener), 'preBind'), array(object(TrimListener), 'preSubmit'), array(object(CsrfValidationListener), 'preSubmit'), array(object(ResizeFormListener), 'preBind')), 'form.pre_bind', object(FormEvent))
        in /Users/david/Desenvolupament/Symfony/fibervent/app/cache/dev/classes.php line 1786

    at Symfony\Component\EventDispatcher\EventDispatcher->dispatch('form.pre_bind', object(FormEvent))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/EventDispatcher/ImmutableEventDispatcher.php line 43

    at Symfony\Component\EventDispatcher\ImmutableEventDispatcher->dispatch('form.pre_bind', object(FormEvent))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Form.php line 558

    at Symfony\Component\Form\Form->submit(array(array('audit' => '1513'), array('audit' => '1513'), array('audit' => '1513')), true)
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Form.php line 579

    at Symfony\Component\Form\Form->submit(array('auditWindmillBlades' => array(array('audit' => '1513'), array('audit' => '1513'), array('audit' => '1513'))), true)
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php line 113

    at Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler->handleRequest(object(Form), object(Request))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/Form/Form.php line 501

    at Symfony\Component\Form\Form->handleRequest(object(Request))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/sonata-project/admin-bundle/Controller/CRUDController.php line 258

    at Sonata\AdminBundle\Controller\CRUDController->editAction('1513')
        in  line 

    at call_user_func_array(array(object(AuditAdminController), 'editAction'), array('1513'))
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php line 144

    at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), '1')
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/HttpKernel.php line 64

    at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), '1', true)
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/DependencyInjection/ContainerAwareHttpKernel.php line 69

    at Symfony\Component\HttpKernel\DependencyInjection\ContainerAwareHttpKernel->handle(object(Request), '1', true)
        in /Users/david/Desenvolupament/Symfony/fibervent/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Kernel.php line 185

    at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
        in /Users/david/Desenvolupament/Symfony/fibervent/web/app_dev.php line 30

On composer install it's quite easy to apply all of default parameters.yml.dist values (according to your DB). And then execute $php app/console hautelook_alice:doctrine:fixtures:load --env=dev command.

Finally you can go to the app_dev.php/admin/audits/audit/1/edit path and try to change Audit status and click on update button.

[EmmanuelVella]

Ok, that's what I thought. You have an AuditAdmin, containg a collection of AuditWindmillBlade. In this case, getSubject works thanks to this hack :

SonataAdminBundle/Form/Type/AdminType.php

Line 58 in f7534c2

$parentSubject = $admin->getParentFieldDescription()->getAdmin()->getSubject();

However, your AuditWindmillBladeAdmin contains another admin (WindmillBladeAdmin), but in this case the hack won't work.

Before my PR, the subject of your WindmillBladeAdmin was loaded (so your code was not failing), but it was the wrong one.

I think it's not a good idea to call getSubject in the configureFormFields method. In standard symfony form, data is not accessible when you build the form, as it is set only once the form is submitted.

You should event listeners instead : https://symfony.com/doc/current/form/events.html#event-listeners