sonata-project / SonataMediaBundle

Symfony SonataMediaBundle
https://docs.sonata-project.org/projects/SonataMediaBundle
MIT License
451 stars 495 forks source link

Remove final on MediaAdminController #2304

Closed mpoiriert closed 2 years ago

mpoiriert commented 2 years ago

Feature Request

I want to add a batch action and it's not possible if I want to use the default behaviour of the controller since there is no way (as I know of) to add batch action without adding the action method directly on the controller.

On a more general point, there is a lot of "final" that have been add to sonata >=4 that make it harder to extend. Not everything can be done with decorator or other patterns. I end up having a command overriding code on install, which is not really a clean solution.

VincentLanglet commented 2 years ago

@franmomu introduce a way to create action without relying on CRUDController, https://github.com/sonata-project/SonataAdminBundle/blob/4.x/docs/cookbook/recipe_decouple_crud_controller.rst#decouple-from-crudcontroller

Can't this be used for batch actions ?

On a more general point, there is a lot of "final" that have been add to sonata >=4 that make it harder to extend. Not everything can be done with decorator or other patterns. I end up having a command overriding code on install, which is not really a clean solution.

For every valid use case, we tried to give another way to solve the issue rather than removing final keyword. If there are non supported use case, it just require new feature requests.

mpoiriert commented 2 years ago

@VincentLanglet I do confirm it's not working for batch actions since the are not a controller but a hook in the CRUDController::batchAction https://github.com/sonata-project/SonataAdminBundle/blob/7ab7cbd88f8662a298c14b10d29565762e49874d/src/Controller/CRUDController.php#L381

You can see the logic exactly here:

https://github.com/sonata-project/SonataAdminBundle/blob/7ab7cbd88f8662a298c14b10d29565762e49874d/src/Controller/CRUDController.php#L498

To allow batch action to work we would need to add a to configure a full _controller (or service) and forward the request to it or call the service.

But this is something that need to be added to Sonata Main project.

VincentLanglet commented 2 years ago

@VincentLanglet I do confirm it's not working for batch actions since the are not a controller but a hook in the CRUDController::batchAction https://github.com/sonata-project/SonataAdminBundle/blob/7ab7cbd88f8662a298c14b10d29565762e49874d/src/Controller/CRUDController.php#L381

Indeed we use

To allow batch action to work we would need to add a to configure a full _controller (or service) and forward the request to it or call the service.

But this is something that need to be added to Sonata Main project.

This would be a great feature to add and this would solve the issue. Do you want to work on it ? Maybe we could create a BatchActionInterface with some methods ?

cc @jordisala1991 @franmomu

mpoiriert commented 2 years ago

@VincentLanglet I can work on this yes

jordisala1991 commented 2 years ago

Closing, as it was solved on AdminBundle instead.