sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Hooks not firing on embedded admins (e.g. using sonata_type_admin to embed another Admin) #1502

Closed caponica closed 2 years ago

caponica commented 11 years ago

For example, I have a Document class and an Image class. In ImageAdmin I have prePersist() and preUpdate() methods to handle the saving of the uploaded files.

This works fine when I'm editing Images directly via SonataAdmin. The save hooks fire and all is well.

In DocumentAdmin I use sonata_type_admin as the field type to embed the ImageAdmin form:

    class DocumentAdmin extends Admin
    {
        $formMapper
            ...
            ->add('imageHeader', 'sonata_type_admin')
            ...
        ;
    }

But when I upload a new image file from the Document edit page in SonataAdmin it does not fire the save hooks for the embedded ImageAdmin.

Now, I know that I could probably put some code into DocumentAdmin::prePersist() and DocumentAdmin::preUpdate() but this doesn't seem very DRY (and feels like something that the SonataAdminBundle should be doing for me when I embed another Admin class using sonata_type_admin).

Have I just misconfigured something or is the non-propagation of save hooks something that we need to look into coding into this bundle in future?

Edvinas9 commented 10 years ago

Did you find any other way of working this out without messing with DocumentAdmin::prePersist() and DocumentAdmin::preUpdate()?

oopsFrogs commented 9 years ago

I have a similar problem here, I have an embedded form using 'sonata_type_collection':

class BranchAdmin extends Admin
{
    protected function configureFormFields(FormMapper $formMapper)
        {
            $formMapper
                ...
        ->add('regularImages',  'sonata_type_collection', ...)
               ...
        }
}

There are checkboxes inside the dynamically created collection, but I cannot catch click event of the checkboxes, I think maybe the event propagation is disabled, but I just don't know how to re-enable it, and whether it would cause any problem if I do.

bobemoe commented 9 years ago

Exact same issue here as the Document and Image example.. Has this been solved yet? Thanks.

bobemoe commented 9 years ago

OK, i found another discussion: https://github.com/sonata-project/SonataAdminBundle/issues/1400 which leads to some documentation and an example: https://sonata-project.org/bundles/admin/master/doc/cookbook/recipe_file_uploads.html#advanced-example-works-with-embedded-admins

ibasaw commented 8 years ago

how to solve this ? neeed prePersist() and preUpdate() in my collections sub forms

chalasr commented 8 years ago

:+1: This issue still exists, hooks of embedded admin classes are not fired.

There is a quick-and-dirty alternative which consists in calling the child admin's hooks from the parent class.

For instance, if you have a Post admin that has a Comment field as embedded admin class and you need to hook into CommentAdmin::prePersist():

// PostAdmin (parent)
public function prePersist($post)
{
    $commentAdmin = $this
        ->getConfigurationPool()
        ->getAdminByAdminCode('comment_admin_service_id');

    foreach ($post->getComments as $comment) {
        $commentAdmin->prePersist($comment); 
    }
}
quisse commented 4 years ago

This issue still seems to exist. My solution is as follows in the parent admin:

public function prePersist($object)
{
    parent::prePersist($object);

    foreach ($this->getFormFieldDescriptions() as $fieldName => $fieldDescription) {
        // detect embedded Admins
        if ($fieldDescription->getType() === AdminType::class) {
            /** @var AbstractAdmin $associationAdmin */
            $associationAdmin = $fieldDescription->getAssociationAdmin();
            $associationObjectGetter = 'get'.ucfirst($fieldName);
            $associationObject = $object->$associationObjectGetter();
            $associationAdmin->preUpdate($associationObject);
        }
    }
}

I am willing to create a PR to get this in the AbstractAdmin class if there's no reason this shouldn't be there.

RaphaelBuquet commented 4 years ago

preRemove() is not called either. I'm assuming that it would be a bit trickier to implement.

icedevelopment commented 4 years ago

This bug is still happening on 3.57.0 and the workaround proposed by @chalasr doesn't seem to work with preRemove.

VincentLanglet commented 2 years ago

I understand the issue but I also feel like this is somehow a mis-understanding of the AdminType. The AdminType is not really an admin but a FormType before everything else.

This is a simple way to generate a form,

I assume none of you used a custom ModelManager, but if you override the ModelManager::create/update method instead of adding code inside prePersist/preUpdate you'll see, the modelManager method of the "embedded admin" is not called. All the logic is let to the form component

$form = $this->admin->getForm();
$form->setData($newObject);
$form->handleRequest($request);
$newObject = $this->admin->create($submittedObject);

For prePersist/preUpdate/preRemove hooks related to an admin, you may rely on a persistence parameter in the url ; for instance /admin/image/create?fullWidth=1 or /admin/document/42/image/create?fullWidth=1. If you're on a document creation, /admin/document/create you won't have the persistence parameter (since it's only on imageAdmin) and the related prePersist/preUpdate/preRemove hooks could ends up with weird behavior. So I won't consider it's should be automatically called. If it should be, you should use Doctrine entityListeners instead.

Moreover since Sonata 4, calling pre* hooks makes less sens and are less possible now that those methods are protected.

I think we should close this @jordisala1991 wdyt ?

jordisala1991 commented 2 years ago

I agree, lets close. An issue 9 years old is not really useful anyway.