WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 2 weeks ago

#50870 closed defect (bug) (fixed)

Auto-update counts do not update when bulk deleting

Reported by: desrosj Owned by: audrasjb
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.5
Component: Plugins Keywords: has-patch has-screenshots commit
Focuses: administration Cc:

Description

When selecting multiple plugins (assuming this will be true for the Themes page in the network admin as well since it is also a WP List Table), selecting "Delete" from the dropdown, and clicking "Apply", the Auto-update filters at the top do not update their counts like the other filters do.

Attachments (6)

50870.diff (1.2 KB) - added by noisysocks 3 weeks ago.
a550e602ab67f905a984e9c2faef1da8.mp4 (3.5 MB) - added by audrasjb 3 weeks ago.
Patch solves the issue
50870.2.diff (8.4 KB) - added by pbiron 3 weeks ago.
50870.3.diff (8.4 KB) - added by pbiron 2 weeks ago.
50870.3.gif (9.0 MB) - added by hellofromTonya 2 weeks ago.
Count updates correctly with 50870.3.diff patch. Ran on Mac OS in Chrome.
50870.4.diff (8.4 KB) - added by pbiron 2 weeks ago.

Change History (30)

#1 @desrosj
4 months ago

  • Keywords needs-patch added
  • Version changed from trunk to 5.5

#2 @audrasjb
4 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

Good catch, thanks!
Self assigning as I'm currently working on a patch for this issue.

#3 @pbiron
3 months ago

Some background about this:

At one point, the feature plugin was hooking into deleted_plugin and removing plugins from the site option. See https://github.com/audrasjb/wp-autoupdates/issues/29.

We then talked about doing the same for themes, but there is currently no equivalent hook. See #14955. As part of that discussion about themes, it was decided that removing things from the site options on delete was not a good idea, see https://github.com/WordPress/wp-autoupdates/issues/30#issuecomment-622608171.

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


3 months ago

#5 @audrasjb
3 months ago

  • Milestone changed from 5.5.1 to 5.6

Moving to 5.6 as per today's bug scrub

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


3 months ago

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


3 months ago

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


5 weeks ago

@noisysocks
3 weeks ago

#9 @noisysocks
3 weeks ago

  • Keywords has-patch added; needs-patch removed

#10 @pbiron
3 weeks ago

@noisysocks thanx for the patch!

It's nearing the end of a long, hectic day here (in the US)...but I'll give it a test first thing in the morning.

#11 @pbiron
3 weeks ago

  • Keywords needs-testing added

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


3 weeks ago

@audrasjb
3 weeks ago

Patch solves the issue

#13 @audrasjb
3 weeks ago

  • Keywords has-screenshots added; needs-testing removed

I tested the patch and it fixes the issue on Plugins screen (see video above).
It can be backported to handle the issue on multisites themes screen.

#14 @pbiron
3 weeks ago

Revised patch coming shortly that fixes things on the multisite themes screen. Just triple-checking that it doesn't have any unintended consequences.

@pbiron
3 weeks ago

#15 @pbiron
3 weeks ago

  • Keywords needs-testing added

50870.2.diff incorporates 50870.diff and extends the solution there so that it works on the multisite themes screen.

Please test this patch very carefully because in order to get it to work on the themes screen I had to change the data that is passed to the JS (via wp_localize_script() in WP_MS_Themes_List_Table::prepare_items()).

In 5.5, only the counts of themes in each status (e.g., disabled, upgrade, etc) was passed to the JS; whereas WP_Plugins_List_Table::prepare_items() passes an array of plugins with each status. This patch does the same for themes...and the JS in wp.updates.deleteThemeSuccess() has been adjusted accordingly.

The patch also updates the DocBlock at the head of the JS to mention settings.{plugins,themes}['auto-update-{enabled,disabled}'] which was never done in 5.5.

Last edited 2 weeks ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core-auto-updates by pbiron. View the logs.


2 weeks ago

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


2 weeks ago

#18 @hellofromTonya
2 weeks ago

Testing 50870.2.diff on multisite throws the following warning:

Warning: Invalid argument supplied for foreach() in ...wp-admin/includes/class-wp-ms-themes-list-table.php on line 369

Question: What's the relationship between the new $js_themes and the global $totals?

@pbiron
2 weeks ago

#19 @pbiron
2 weeks ago

50870.3.diff fixes the problem identified by @hellofromTonya above.

When putting 50870.2.diff together I didn't realize that $totals was a global...I thought it was only used in the call to wp_localize_script().

The difference between $totals and the new $js_themes is that $totals contains counts of themes in each status, whereas $js_themes contains the theme slugs in each status. The later is needed in the modified JS (wp-admin/js/updates.js) to know which counts should be updated in the views when a theme with a specific slug is deleted.

Last edited 2 weeks ago by pbiron (previous) (diff)

@hellofromTonya
2 weeks ago

Count updates correctly with 50870.3.diff patch. Ran on Mac OS in Chrome.

#20 @hellofromTonya
2 weeks ago

  • Keywords needs-testing removed

@pbiron
2 weeks ago

#21 @pbiron
2 weeks ago

  • Keywords commit added

50870.4.diff is the same as 50870.3.gif but removes an unnecessary cast to array as mentioned in slack.

For whoever commit's this, the cast was to mimic the equivalent code in WP_Plugins_List_Table::prepare_items() which was added in [37714]. As best I can tell, the case isn't needed there, but I don't want to include that change in this ticket...but feel free to make the change if you thing it's warranted.

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


2 weeks ago

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


2 weeks ago

#24 @SergeyBiryukov
2 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 49582:

Administration: Make sure auto-update counts properly update when bulk deleting plugins or themes.

Props pbiron, noisysocks, desrosj, audrasjb, hellofromTonya.
Fixes #50870.

Note: See TracTickets for help on using tickets.