General 'delete_plugins' hook needed

[Veraxus]

I'm working on an activity logging plugin and managed to use the 'upgrader_process_complete' hook to detect when plugins are installed, but while digging around core for a way to detect when plugins are deleted, I realized that there was no existing, suitable hook.

Now, there is an 'uninstall_'.$file hook - but that only allows targeting of a specific plugin... while I need to detect and identify any and all plugin deletions.

I'm proposing the addition of the following hook in /wp-admin/includes/plugin.php, immediately after the 'uninstall_'.$file hook in uninstall_plugin()...

do_action( 'uninstall_plugin', $plugin );

[Veraxus]

Patch that adds the 'uninstall_plugin' hook

[TobiasBg]

This would be very unreliable as it would not catch the manual uninstallation of a plugin by directly deleting it on the server, e.g. via FTP or SSH...

[Veraxus]

The previous solution I posted was inadequate as it misses certain uninstall scenarios. A more reliable hook location would be at the end of the delete_plugins() function.

[Veraxus]

Replying to TobiasBg:

This would be very unreliable as it would not catch the manual uninstallation of a plugin by directly deleting it on the server, e.g. via FTP or SSH...

Right now I'm only trying to log actions taken through the admin. If a file changes on the server, I wouldn't be able to assign "blame", which is the primary reason I'm making this.

That said, my Wish List includes using WordPress's ability to detect remote deletions to log those as well... but right now it's just a "would be nice to have" feature, while this is a blocker.

I've been experimenting with a 'delete_plugins' hook instead ( in delete_plugins() ), and that is working very reliably for detecting when plugins are deleted from the admin. Once I've done some more testing (bulk deletes, custom and automatic uninstalls, etc) I'll post that patch.

[nacin]

Replying to Veraxus:

That said, my Wish List includes using WordPress's ability to detect remote deletions to log those as well... but right now it's just a "would be nice to have" feature

That sounds more like plugin territory.

Could you explain the "certain uninstall scenarios" that a hook in uninstall_plugin() misses?

[Veraxus]

'delete_plugins' hook - A much better, more comprehensive hook. Tested and working.

[Veraxus]

Replying to nacin:

That sounds more like plugin territory.

Could you explain the "certain uninstall scenarios" that a hook in uninstall_plugin() misses?

My first patch was inadequate as there are two scenarios handled by that function...

1. Plugin has it's own uninstall file, run it.
2. Plugin has no uninstall file, run "uninstall_{$file}" hook instead.

By default, the first scenario has no hooks at all, and my first patch did not add one... so I was unable to handle it at all. And without a patch, the second scenario can't be handled either.

That means there are two viable options for adding the appropriate hooks for "delete any plugin"...

1. A 'delete_plugins' hook that runs at the end of delete_plugins() (e.g. delete_plugin_hook.diff) OR...
2. TWO separate hooks in uninstall_plugins(), one that handles custom uninstalls, and then another that handles everything else.

I've tried both options and they both work well (I'll upload a fresh patch momentarily that properly addresses point 2, as the original patch neglects scenarios where a custom uninstall is run).

'delete_plugins' seems to address the need in the simplest way, but would only be run after the files have been deleted (meaning you wouldn't be able to pull metadata like name or author out of the plugin before deletion occurs).

Honestly, I'd be fine with either solution, or both if there is a need.

[Veraxus]

Adds the 'uninstall_plugin' hook to both uninstall_plugin() handlers

[Veraxus]

Option 3: Formal recommendation - 'delete_plugin' hook within delete_plugins() deletion loop

[Veraxus]

So, I've spent the day testing a wide variety of plugins with a wide variety of potential hooks, and it's clear that everything I've proposed so far has serious drawbacks.

1. uninstall_plugin() does not always trigger, so placing any hooks in that function is unhelpful.

2. Likewise, placing a hook at the very end of delete_plugins() means that you can't fetch/log plugin metadata as the plugins have already been deleted.

However, after spending the day testing a massive number of plugins and hook options, I've found that placing a new 'delete_plugin' hook right in the middle of delete_plugins()'s deletion loop (just before $wp_filesystem->delete() is called) works perfectly.

I've uploaded my final recommendation as a patch: delete_plugin_hook_FINAL.diff​

One hook, called 'delete_plugin', that runs on each loop inside delete_plugins() just before each plugin is deleted. Bingo.

[Veraxus]

Any chance we could get this slated for 4.0? My plugin is dead in the water without it.

[juliobox]

Hello

@nacin : This is absolutly not a plugin territory since plugin A could hook on plugin B. So when someone deletes plugin B, the hook from plugin A will trigger.

Real example:
The plugin WP Rocket add a hook on "uninstall_woocommerce/woocommerce.php" because when Wooc is deleted, WP Rocket needs to remove the "cart" page from the expluded pages from cache.

But this works only if Wooc has use the uninstall hook and not the uninstall file, not logical.

@TobiasBG : This argument is not valid, you can't refuse a hook because of a possible missuse from a user. Using this way of thinking, why do we have a hook on user_delete if someone can delete it directly from PHPMYADMIN? So, it's not relevant, sorry.

edit : i just thought this: why not just remove the first "return true"? So when uninstall files are included, this is not a blocking situation, then all hooks (added by register_uninstall) will also be triggered!

Thanks to add this simple hook quickly to restore the balance into the force ;)

[DrewAPicture]

+ hook doc

[DrewAPicture]

26904.diff​ moves the hook to just after the plugin deletion attempt and adds a hook doc.

@Veraxus: Can you please test?

[DrewAPicture]

26904.diff​ still applies. I'd like to get some others' eyes on it before we move forward here.

[Veraxus]

Replying to DrewAPicture:

26904.diff​ moves the hook to just after the plugin deletion attempt and adds a hook doc.

@Veraxus: Can you please test?

@DrewAPicture : This version of the hook seems to be working swimmingly for my use-cases.

[DrewAPicture]

@Veraxus Thanks for getting back to us!

[DrewAPicture]

In 35094:

Plugins: Introduce the delete_plugin transactional hook, which fires immediately after a plugin deletion attempt occurs.

Props Veraxus for the initial patch.
Fixes #26904.

[johnjamesjacoby]

Introduce deleted_plugin hook, and move delete_plugin to before

[johnjamesjacoby]

Hey everyone! Switching this up just a bit to match the other activate/deactivate/install/uninstall hooks. This new patch adds before & after hooks, which enables plugins to get information about the plugin (like file headers) being deleted before it happens.

---

For additional context, the way I stumbled into this omission is from ​a forum topic for my WP User Activity plugin, where a user noticed no activity was logged when a plugin was deleted from the file system. This was originally because no delete_plugin action existed, but in digging deeper I found the action was added here, but that it's impossible to get the name of the plugin being deleted after it's no longer in the file system (go figure, right?)

[johnjamesjacoby]

Same as 2.patch, but with corrected filter documentation

[DrewAPicture]

In 35669:

Plugins: Rename the delete_plugin action hook (introduced in [35094]) to deleted_plugin as it fires following a plugin deletion attempt.

Further, introduce a new delete_plugin action hook, to be fired before a plugin deletion attempt. Both changes bring parity with other such transactional hooks in core that fire before and after certain actions, including on plugin activation/deactivation and install/uninstall, among others.

Props johnjamesjacoby.
Fixes #26904.