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

Implement an event for batch actions #7516

Closed 7ochem closed 3 years ago

7ochem commented 3 years ago

There are event for sonata.admin.event.persistence.[pre|post]_[persist|update|remove], but no event is dispatched when running a batch action, for instance batch deleting entities. So if I delete entities one by one, then I get them in my event listeners, but when I do a batch delete, then these entities are not passed into any event.

Feature Request

Implement a batch event.

This could be done through the \Sonata\AdminBundle\Event\AdminEventExtension, however also extensions are not called here, so that should also be added and also AdminExtensionInterface should cover a preBatchAction() method (in a BC way). So this solution requires quite some additions.

Another approach is to dispatch an event in the CRUDController. But this way the event would not be dispatched when using a different controller (which handles custom batch actions?).

VincentLanglet commented 3 years ago

I kinda see this like a duplicate of https://github.com/sonata-project/SonataAdminBundle/issues/6550

The issue is that when you're doing a batch delete, the modelManager is doing the job https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/c783e2ed8d1b8e690bf96f2404824a3797816377/src/Model/ModelManager.php#L333 so neither the controller or the admin are knowing the entities deleted.

For the other batch actions, it's kinda the same. You'll start with a ProxyQuery, so as long as you don't execute it, you cannot know the entities to dispatch in the event.

7ochem commented 3 years ago

Ok, so because of the ProxyQuery you don't have entities and don't even have the IDs. But still there's a purpose for a preBatch event. Just like you have prePersist, preUpdate and preRemove and pre_persist, pre_update and pre_remove events.

In a pre_batch event you could also influence the ProxyQuery, just like you could in the admin's preBatch() method

VincentLanglet commented 3 years ago

I never say there was no purpose. I was more talking about the technical issue/limitations. If you have some idea about the implementation you're welcome ; you can also start a draft PR to discuss about the idea

7ochem commented 3 years ago

I should probably do that. Thanks.

One question: This would probably be a new feature, so should I start this from 4.x or is it also possibly to start this in 3.x and once complete it would be forward merged into 4.x?

VincentLanglet commented 3 years ago

The 3.x branch is feature frozen so this should target the 4.x branch indeed. We cannot afford to maintain multiple stable branches.

7ochem commented 3 years ago

@VincentLanglet I have created a draft PR: #7579. I hope you have some time to look at it and discuss it there.