Opened 11 years ago
Closed 9 years ago
#26904 closed enhancement (fixed)
General 'delete_plugins' hook needed
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Plugins | Keywords: | has-patch needs-testing 2nd-opinion |
Focuses: | Cc: |
Description
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 );
Attachments (7)
Change History (25)
#1
follow-up:
↓ 3
@
11 years ago
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...
#2
@
11 years ago
- Summary changed from General 'uninstall_plugin' hook needed in uninstall_plugin() to General 'delete_plugins' hook needed
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.
#3
in reply to:
↑ 1
;
follow-up:
↓ 4
@
11 years ago
- Keywords needs-patch added
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.
#4
in reply to:
↑ 3
;
follow-up:
↓ 5
@
11 years ago
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?
#5
in reply to:
↑ 4
@
11 years ago
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...
- Plugin has it's own uninstall file, run it.
- 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"...
- A 'delete_plugins' hook that runs at the end of delete_plugins() (e.g. delete_plugin_hook.diff) OR...
- 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.
@
11 years ago
Option 3: Formal recommendation - 'delete_plugin' hook within delete_plugins() deletion loop
#7
@
11 years ago
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.
#8
@
11 years ago
Any chance we could get this slated for 4.0? My plugin is dead in the water without it.
#9
@
10 years ago
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 ;)
#10
follow-up:
↓ 14
@
10 years ago
- Keywords needs-testing added
- Milestone changed from Awaiting Review to 4.4
- Type changed from feature request to enhancement
26904.diff moves the hook to just after the plugin deletion attempt and adds a hook doc.
@Veraxus: Can you please test?
This ticket was mentioned in Slack in #core by drew. View the logs.
10 years ago
#12
@
10 years ago
26904.diff still applies. I'd like to get some others' eyes on it before we move forward here.
#14
in reply to:
↑ 10
@
10 years ago
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.
#17
@
9 years ago
- Keywords 2nd-opinion added
- Resolution fixed deleted
- Status changed from closed to reopened
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?)
Patch that adds the 'uninstall_plugin' hook