Make WordPress Core

Opened 5 months ago

Last modified 5 months ago

#61610 new defect (bug)

Hidding the dependency notice + prevent the plugin deactivation only on some case.

Reported by: sebastienserre's profile sebastienserre Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.5
Component: Plugins Keywords: reporter-feedback
Focuses: Cc:

Description

WordPress 6.5 introduced th depedency fature for plugins which is a great feature.
For the context, i'm working at the Polylang helpdesk and all the day i'm helping our customers to fix behaviours between pllugins and Polylang.
Most of the plugins for WooCommerce are starting to declare their depedency to WooCommerce nas this is great. In my case, I've several of them, not activated but installed and I've permanently this notice: https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/class-wp-plugin-dependencies.php#L372.
=> I think we should be able to find a solution to hide the notice in some cases.

I also can't deactivate WooCommerce in bulk because these plugins are installed (not activated) and I think there's something not logical because for the really first time on WordPress (I think) that a plugin has an action on another one without being activated.

=> I think we should prevent the deactivation on WooCommerce only if a dependent plugin is activated.

I took WooCommerce as an example because it's a major plugin with lots of plugins in its ecosystem.

Attachments (2)

warning-admin-notice.jpg (120.3 KB) - added by sebastienserre 5 months ago.
wordpress-6.6-notice.jpg (168.4 KB) - added by sebastienserre 5 months ago.

Download all attachments as: .zip

Change History (14)

#1 @audrasjb
5 months ago

  • Milestone changed from Awaiting Review to 6.6.1

Thanks for the report!
While this is really a edge case, let's milestone it to 6.6.1.
A good starting point would be to display a warning when using a development environment.

A nice enhancement would be to disable the dependency feature according to the environment setup. Maybe by using a new environment constant type?

#2 @sebastienserre
5 months ago

I've just updated my WordPress Develop instance (wordpress-6.6-notice.jpg) and unfortunately do not reproduce the same notice as in WordPress 6.5.5 (warning-admin-notice.jpg)

This ticket was mentioned in PR #7002 on WordPress/wordpress-develop by @sebastienserre.


5 months ago
#3

  • Keywords has-patch added; needs-patch removed

This PR will check the current environmement defined by the constant and disable the plugin dependency on non production website.It will allow an easier debug.

#4 @sebastienserre
5 months ago

The PR seems to fix in WordPress 6.6 but (yet) not the notice in 6.5.5

Version 0, edited 5 months ago by sebastienserre (next)

#5 @audrasjb
5 months ago

@sebastienserre thanks for the PR.
I added a commit to it to clean up some unwanted changes.

The PR seems to fix in WordPress 6.6 but not (yet) the notice in 6.5.5

Backporting this future changeset to other branches should be discussed. Quick question: as a plugin maintainer, do you think it is really valuable to backport it to 6.5 during the next minor?

#6 @sebastienserre
5 months ago

I don't know, we do not use (yet) this feature for our plugins.
Maybe it would be great to backport because, there's only one branch 6.5.6 to create (Do I miss something) ?

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


5 months ago

#8 @hellofromTonya
5 months ago

  • Version set to 6.5

Modifying the Version to 6.5.0 which added the feature and code in question.

[57658] added the initialize() method.

As this was introduced in 6.5 and not in 6.6, I'm curious of compelling reasons to fix it in 6.6.1, rather than in 6.7.

#9 @sebastienserre
5 months ago

I think a near version has been set because this notice pollutes the interface, as it is always visible when WooCommerce (for example) is deactivated.

#10 @costdev
5 months ago

  • Keywords reporter-feedback added; 2nd-opinion has-patch removed

Thanks for opening this ticket @sebastienserre!

Milestone

If a bug can be established, this wasn't introduced in 6.6. If no bug can be established, then this is an enhancement. In both instances, this doesn't appear to be appropriate for a 6.6 minor.


Why the notice displays

This notice should only appear when either of the following apply:

  1. Dependents are installed, but their dependencies are not installed.
  2. Dependents are active, but their dependencies are not active, or are not installed.

It would appear that you were experiencing the first case. The solution is to ensure all dependencies are installed. I tested on WordPress Playground with WordPress 6.5.5 and the notice appeared when a WooCommerce dependent was installed when WooCommerce wasn't, and disappeared once WooCommerce was installed, which is the expected behaviour.

If you delete the WooCommerce directory from your current setup, you should see the notice appear again after refreshing the page. Adding reporter-feedback, pending this check.


Conditional display of the notice

I don't currently see a reason to bring conditional display of this notice into Core, as it seems to be doing what it's designed to do. Happy to discuss though.


Bulk Actions

Bulk Actions were intentionally disabled for plugins utilizing the Plugin Dependencies feature at this time. Bulk Action support for such plugins has some complexity that needs to be addressed carefully.

For example, allowing Bulk Deactivation for WooCommerce means also allowing Bulk Deletion of WooCommerce, as the Bulk Actions dropdown options apply to all plugins. Adding a wp_die() or an admin notice when a dependency could not be deleted sounds straightforward, but we need to ensure that we check the plugins in the right order, otherwise alphabetical order will determine the chance of failure. The list of considerations continues.

Enhancement work to introduce Bulk Action support for those utilizing Plugin Dependencies is already planned - aiming to get this in for 6.7 if we can. This ticket should not address this work, as it should have its own ticket that establishes historical context and outlines the planned work. When I get a chance, I'll create the ticket for introducing the Bulk Action support.

Removing 2nd-opinion as both points of the ticket's description have been addressed by the feature team.


Conditional enabling/disabling of Plugin Dependencies

Regarding the separate enhancement suggestion of enabling/disabling the Plugin Dependencies feature based on environment type, that should be another ticket with its own discussion. Since the feature is about dependencies (hard requirements), I'd like to see clear use cases for conditional enabling/disabling outlined for discussion, as I can't currently see why this should be optional, much like PHP/WP hard requirements.

Removing has-patch as any patch should be on that separate ticket, if discussion there leads to consensus on conditional enabling/disabling.

#11 @jorbin
5 months ago

With 6.6.1 being a short cycle small focus release, i think we should bump this to 6.6.2

#12 @costdev
5 months ago

  • Milestone changed from 6.6.1 to Awaiting Review

Moving to Awaiting Review as the code wasn't introduced in 6.6 and no bug has been confirmed yet.

Note: See TracTickets for help on using tickets.