Make WordPress Core

Opened 5 years ago

Last modified 9 days ago

#53255 reopened defect (bug)

WordPress multisite allows you to delete plugins which are active on some subdomains

Reported by: denisflorin197's profile denisflorin197 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: critical Version: 4.6
Component: Plugins Keywords: has-patch has-unit-tests has-screenshots
Focuses: administration, multisite Cc:

Description

Hello,

I remember in the past, WordPress was not allowing you to delete a plugin that is active at least on one website.

Attachments (5)

53255.diff (13.0 KB) - added by tippl 9 days ago.
Fix subsite active check and UI warnings
just-delete.png (1.1 MB) - added by tippl 9 days ago.
first solution
with-site-amount.png (1.1 MB) - added by tippl 9 days ago.
2nd solution
53255.2.diff (13.6 KB) - added by tippl 9 days ago.
53255.3.diff (14.2 KB) - added by tippl 9 days ago.
Added a warning to the delete confirmation screen in the Network Admin as suggested

Change History (14)

#1 @SergeyBiryukov
5 years ago

  • Component changed from General to Plugins
  • Focuses administration multisite added
  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

Hi there, welcome to WordPress Trac! Thanks for the report.

As far as I can tell, this has always been the case, we're already tracking this issue in #31673.

#2 @denisflorin197
5 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Hello,

But, I don't see any warnings. The only thing I get is that if I want to remove the plugin. See video: https://www.loom.com/share/d69900d4098e4bebbefc80b307bbdfb8

Thanks,

#3 @SergeyBiryukov
5 years ago

  • Keywords needs-patch added
  • Milestone set to Awaiting Review
  • Version changed from 5.7.2 to 4.6

Thanks for the additional details! Indeed, I was able to reproduce the issue.

The warning is still in core, but it looks like it's no longer displayed, apparently due to the "Shiny Updates" changes introduced in [37714] for WordPress 4.6.

@tippl
9 days ago

Fix subsite active check and UI warnings

@tippl
9 days ago

first solution

@tippl
9 days ago

2nd solution

#4 @tippl
9 days ago

  • Keywords has-patch 2nd-opinion has-unit-tests has-screenshots added; needs-patch removed

Added patch that:

  • Blocks deletion when plugin is active on subsites
  • Shows "Active on X sites" instead of delete link
  • Adds note when plugin has dependents AND is active on subsites
  • Counts main site + subsites correctly

Please give feedback:

  • first solution disabled "delete"
  • 2nd solution with amount of sites

#5 @Kau-Boy
9 days ago

I've checked the code and run the test on my laptop and all pass.

BTW: This was Manja's first code contribution to core that was done at the WCEU 2026 Contributor Day at the Multisite table.

#6 follow-up: @realloc
9 days ago

You may use a filter to work on a defined number of sites, to guard against performance problems on larger multisites.

<?php
$site_ids = get_sites(
    array(
       'fields' => 'ids',
       'number' => 100, // filter here
    )
);

#7 follow-up: @desrosj
9 days ago

Thank you @tippl for this contribution!

I discussed with @bernhard-reiter a bit today at WordCamp Europe Contributor Day. A few bullet points.

  • The notice should be updated to always warn the user in the confirmation message that the plugin may have been activated for individual sites on the network.
  • I am a bit concerned with scanning every site to count the number of subsites that have activated the plugin. This will not scale, especially every time the Plugins page on the Network Admin is loaded.
  • There is not a good way to query across multiple tables at the same time, so that's out.

I'm not convinced yet, but I think the only way this works is if there a network option is added that contains a multidimensional array of site IDs keyed by plugin slug:

array(
	'hello-dolly/hello.php' => array(
		1,
		2,
		3,
	),
	'ai-by-jonathan' => array(
		4,
		5,
		6,
	),
);

Each time a sub site activates or deactivates a plugin, the option is updated accordingly. However, this would require the option to be hydrated the next time the network upgrade is run.

#8 in reply to: ↑ 6 @tippl
9 days ago

  • Keywords 2nd-opinion removed

Replying to realloc:

You may use a filter to work on a defined number of sites, to guard against performance problems on larger multisites.

<?php
$site_ids = get_sites(
    array(
       'fields' => 'ids',
       'number' => 100, // filter here
    )
);

Good point! I've added the filter. This way, a network admin can adjust the limit themselves, and the default of 100 is a sensible compromise.

@tippl
9 days ago

#9 in reply to: ↑ 7 @tippl
9 days ago

Replying to desrosj:

Thank you @tippl for this contribution!

I discussed with @bernhard-reiter a bit today at WordCamp Europe Contributor Day. A few bullet points.

  • The notice should be updated to always warn the user in the confirmation message that the plugin may have been activated for individual sites on the network.
  • I am a bit concerned with scanning every site to count the number of subsites that have activated the plugin. This will not scale, especially every time the Plugins page on the Network Admin is loaded.
  • There is not a good way to query across multiple tables at the same time, so that's out.

I'm not convinced yet, but I think the only way this works is if there a network option is added that contains a multidimensional array of site IDs keyed by plugin slug:

array(
	'hello-dolly/hello.php' => array(
		1,
		2,
		3,
	),
	'ai-by-jonathan' => array(
		4,
		5,
		6,
	),
);

Each time a sub site activates or deactivates a plugin, the option is updated accordingly. However, this would require the option to be hydrated the next time the network upgrade is run.

Thank you for the detailed feedback! I understand the performance concerns with large networks. The solution was a first step, but I agree it doesn't scale well.

The network option approach makes sense to me. I'm not sure I have the experience yet to implement the full migration/hydration part correctly.

@tippl
9 days ago

Added a warning to the delete confirmation screen in the Network Admin as suggested

Note: See TracTickets for help on using tickets.