Conversation
Previously, when a -dev version is not active, it was always being hidden from the plugin page. This caused a few confusing interactions if there wasn't a non-dev version of the plugin also installed: you can't see it to delete it if you want to clean up your filesystem, and Core's plugin dependency feature might show a dependency from the hidden plugin. This PR changes it to only hide the -dev version if the non-dev version is also present. That causes another odd interaction: if you delete the non-dev version, then the dev version unexpectedly shows up. The sensible thing to do in that case is to delete the dev version when the non-dev version gets deleted; this PR hooks 'deleted_plugin' to do just that.
github-actions
Bot
added
the
[Plugin] Beta
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Beta plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
tbradsha
left a comment
tbradsha
left a comment
There was a problem hiding this comment.
I believe it's mostly working as intended.
- When I deactivated a dev version of the plugin, the Plugins page then shows the release version of the plugin.
- Activating again keeps the release version of the plugin.
- Deleting it does indeed delete the dev version as well.
- When I delete (via the filesystem) the release version, the dev version continues to show in the Plugins page. Deactivating it and reactivating it preserves its state in the Beta plugin.
I did see one weird issue. If I have a dev version of Beta active and delete the release version of Beta in the filesystem, then deactivate Beta from the plugins page, the dev version is removed from the filesystem. I'm guessing that's unrelated to this PR.
Out of scope, but it'd be nice if when there's ONLY the dev version installed, I could install the release version.
| if ( ! $wp_filesystem ) { | ||
| return; | ||
| } | ||
| $working_dir = dirname( $plugin->dev_plugin_path() ); |
There was a problem hiding this comment.
But what if the $wp_filesystem is non-functional now?
| if ( ! $deleted ) { | ||
| return; | ||
| } | ||
| $plugin = Plugin::get_plugin( dirname( $plugin_file ) ); |
There was a problem hiding this comment.
This confused me: the plugin is deleted, but then we're going to grab the plugin info? Turns out it's pulling from a cache that persists after the plugin itself is deleted.
|
|
||
| add_filter( 'all_plugins', array( $this, 'update_all_plugins' ) ); | ||
|
|
||
| add_filter( 'deleted_plugin', array( $this, 'maybe_delete_dev_plugin_too' ), 10, 2 ); |
There was a problem hiding this comment.
Semantics comment...it's an action, not a filter:
https://developer.wordpress.org/reference/hooks/deleted_plugin/
| add_filter( 'deleted_plugin', array( $this, 'maybe_delete_dev_plugin_too' ), 10, 2 ); | |
| add_action( 'deleted_plugin', array( $this, 'maybe_delete_dev_plugin_too' ), 10, 2 ); |
2 participants
Proposed changes
Previously, when a -dev version is not active, it was always being hidden from the plugin page. This caused a few confusing interactions if there wasn't a non-dev version of the plugin also installed: you can't see it to delete it if you want to clean up your filesystem, and Core's plugin dependency feature might show a dependency from the hidden plugin.
This PR changes it to only hide the -dev version if the non-dev version is also present.
That causes another odd interaction: if you delete the non-dev version, then the dev version unexpectedly shows up. The sensible thing to do in that case is to delete the dev version when the non-dev version gets deleted; this PR hooks 'deleted_plugin' to do just that.
Related product discussion/links
p1777903294786299-slack-C05Q5HSS013
Does this pull request change what data or activity we track or use?
No
Testing instructions