Add onBeforeUnlink and onAfterUnlink method support to the GridFieldDeleteAction Component

[mooror]

Description

I propose that we use invokeWithExtensions() to add onBeforeUnlink() and onAfterUnlink() support to objects that are going to be unlinked using the GridFieldDeleteAction

If this is added to the core, it will mean that all DataObjects (or any other Gridfield managed object) will be able to optionally define these methods to run code before or after that object has been unlinked

Additional context or points of discussion

A few extra thoughts on the matter:

This is something that I have found useful in my own development. In my case I only used it in a few places, so I just subclassed the GridFieldDeleteAction and used my subclass in Gridfields that I needed to support the feature. But I can see this as being a simple (I think?) but useful addition to the core.

Support for non-Extensible objects - With recent updates it is safe to say that going forward not all Gridfields will manage DataObjects and so its plausible that some of the objects may not support the invokeWithExtensions() method that is apart of the Extensible trait. I propose that we add a conditional check around the hooks to ensure that either the method exists, or that the class uses the Extensible trait (perhaps with class_uses)?

Documentation - I would also add some documentation relating to this new feature. I'm thinking adding it to the Extending DataObject models (as invokeWithExtensions() allows for extension support), and Introduction to the data model and ORM pages would be best. But what do you all think?

Validations

• [X] You intend to implement the feature yourself
• [X] You have read the contributing guide
• [X] You strongly believe this feature should be in core, rather than being its own community module
• [X] You have checked for existing issues or pull requests related to this feature (and didn't find any)

[mooror]

Proposed Code for GridFieldDeleteAction.php (extra spacing added for clarity):

public function handleAction(GridField $gridField, $actionName, $arguments, $data)
    {
        //...
        if ($actionName == 'deleterecord' || $actionName == 'unlinkrelation') {
            //...
            if ($actionName == 'deleterecord') {
                //...
            } else {
                //...

                // NEW onBeforeUnlink code
                $hookSupport = $item->hasMethod("invokeWithExtensions");
                if ($hookSupport) {
                    $item->invokeWithExtensions("onBeforeUnlink", $gridField, $this);
                }

                $list->remove($item);  // present in codebase

                // NEW onAfterUnlink code
                if ($hookSupport) {
                    $item->invokeWithExtensions("onAfterUnlink", $gridField, $this);
                }
            }
        }
    }

I am not sure what parameters would be most useful, so I just added the Gridfield and the component itself. But let me know if other parameters should be used instead

[GuySartorelli]

I wouldn't tie this to GridFieldDeleteAction, because you may want the same thing to happen when removing records from a relation list programatically as well.

I've seen similar requests for adding hooks for when adding records to a relation list too, so it's starting to feel like something along these lines is necessary.

The main problem with that would be that adding/removing records to relation lists is a very high traffic piece of code, so before I merge anything new extension hooks in that area I'd want to see some profiling that shows it won't cause performance issues at scale.

[NightJar]

I was thinking the same. Perhaps better suited for (Has|Many)ManyList - the subclass of RelationList (classnames recalled from memory - may differ).

[mooror]

Oh. Well. That makes sense. And I see your point, it would be more useful at a lower level. But alas I don't have any ideas for how that might be done.

I don't suppose it could be a progressive enhancement that's transparent to the end user? Like implementing in the component first, then later removing it and replacing it with the lower level version, but keeping the API the same?

[GuySartorelli]

I don't suppose it could be a progressive enhancement that's transparent to the end user? Like implementing in the component first, then later removing it and replacing it with the lower level version, but keeping the API the same?

It could, but I don't see the value in adding an extension hook we intend to remove. If we're going to do this, IMO it should be done at the lower level from the start.

[NightJar]

You could try Injector override, but I don't think the components are created via Injector to override the GridField Delete button component. Best workaround for now would be to implement your change where you want it, then use a combination of removeByType and addComponent with your GridFieldConfig (or via an Extension if a dependency) before using it.