Add preBatchAction to Admin Extension and add a pre_batch_action event

[7ochem]

Subject

This adds a preBatchAction() method to the AbstractAdminExtension and AdminExtensionInterface (in a backwards compatible way). This enables the AdminEventExtension to dispatch a BatchActionEvent of type pre_batch_action. Both the extension and the event provide points to hook into within your own code, just like this is possible for pre/post persist/update/delete.

I am targeting 4.x as this is a feature that is backwards compatible (made in a BC way)

Fixes #7516 "Implement an event for batch actions"
Relates to #6550 "Delete Event isn't triggered in Batch delete"

Changelog

### Added
- Added `AbstractAdminExtension::preBatchAction()` and `AdminExtensionInterface::preBatchAction()` (as annotation for BC) to have an extension point on batch actions.
- Added `AdminEventExtension::preBatchAction()` that dispatches a `sonata.admin.event.batch_action.pre_batch_action` event with a BatchActionEvent object containing the data
- Added BatchActionEvent class as a transport for (newly introduced) batch action events

To do

• Update the documentation;

[VincentLanglet]

Seems nice

[VincentLanglet]

Suggested change

	public function &getIdx(): array
	public function getIdx(): array

?

[7ochem]

The array of IDs selected for the batch actions is passed by reference everywhere. Also to the AbstractAdmin::preBatchAction() for example. This enables you to manipulate the IDs. So here it should also be returned by reference, hence the & in front of the function name.

This enables you to manipulate the array of IDs in your event listener/subscriber:

$idx = &$event->getIdx();
$idx[] = 123;

Above will result in the batch action also being executed on object with ID 123. Of course this is an example and normally you'll have some business logic in there. And you could also remove IDs of course.

This can already be done in an Admin's preBatchAction() method.

[VincentLanglet]

Didn't know this syntax. Thanks for the explanation.

[jordisala1991]

Wouldn't be better to have getters and setters?

[7ochem]

Like:

$idx = $event->getIdx();
$idx[] = 123;
$event->setIdx($idx);

That could be possible. However, I tried to be consistent with the concept of passing by reference that is already in the existing code for preBatchAction()

In my opinion I think it would be better to have an array object so the whole passing by reference is not necessary, but that would be breaking compatibility for the existing code of preBatchAction(). Maybe this is a good idea for 5.x

[VincentLanglet]

Please review again @core23

[core23]

Do we need the default value for allElements? There is no call without that argument.

[VincentLanglet]

I would prefer to keep the consistency with

SonataAdminBundle/src/Admin/AdminInterface.php

Line 254 in ea8c6ac

public function preBatchAction(string $actionName, ProxyQueryInterface $query, array &$idx, bool $allElements = false): void;

[core23]

There is also no call for the AdminInterface::preBatchAction that does not provide the allElements parameter:

SonataAdminBundle/src/Controller/CRUDController.php

Line 471 in 50ef7e1

$this->admin->preBatchAction($action, $query, $idx, $allElements);

It looks more like some kind of bug, that we provide a default value here. We should ignore the current consistency in favor of advertising the correct arguments.

[VincentLanglet]

Can be confusing for the user to have two different method signatures.

What about keeping it like this with a NEXT MAJOR on both method to remove the default argument ?

[core23]

Can be confusing for the user to have two different method signatures.

What about keeping it like this with a NEXT MAJOR on both method to remove the default argument ?

This can be done on the stable branch
https://3v4l.org/V0cUU

[VincentLanglet]

Indeed @7ochem can you do it ?

[7ochem]

Just so I understand this correctly:

I remove the default from the interface?

- * @method void preBatchAction(AdminInterface $admin, string $actionName, ProxyQueryInterface $query, array &$idx, bool $allElements = false)
+ * @method void preBatchAction(AdminInterface $admin, string $actionName, ProxyQueryInterface $query, array &$idx, bool $allElements)

But keep it on the AbstractAdminExtension implementation, here:

SonataAdminBundle/src/Admin/AbstractAdminExtension.php

Line 104 in 7776c37

public function preBatchAction(AdminInterface $admin, string $actionName, ProxyQueryInterface $query, array &$idx, bool $allElements = false): void

Am I correct @core23 ?

[VincentLanglet]

I think he want to remove it in both the Interface and the implementation (and remove it too for the AdminINterface)

[7ochem]

I think he want to remove it in both the Interface and the implementation

👍🏻 Ok, I'll change that and push it

(and remove it too for the AdminINterface)

Removing it inside the existing method in the AdminInterface is a BC break. Also, in my opinion, that would be outside of the scope of this PR. However I would be happy to issue a new PR afterwards for this change.

[VincentLanglet]

Thanks

[7ochem]

Thanks

And thank you all for reviewing and accepting the PR! 🎉