Plugins screen - The view 'Auto-updates Enabled' also counts not supported plugins

[knutsp]

On plugins.php screen, and network/plugins.php, right above the table, the usual view links, one of the links reads "Auto-updates Enabled" and has an item count.

This count seems to include all plugins that does not explicitly have Auto-updates Disabled. This includes/counts also those plugins with no (auto) update supported.

Expected: The count should reflect only those specifically set to be have Auto-updates Enabled.

In other words: (All) - ( Enabled + Disabled ) = (the number of plugins with no support)

Since Enabled is the default, it probably has to calculate this by rearranging the above formula, based on an actual, internal count of plugins with no support.

For network/themes.php I can't see these views. Is the lack of them an oversight?

[knutsp]

For network/themes.php I can't see these views. Is the lack of them an oversight?

Sorry. My multisites does not run trunk/5.5 yet, just the feature plugin, so I guess it has the same issue.

[audrasjb]

THanks for spotting this @knutsp, working on a patch right now.
Milestoning to 5.5 as this regression introduced in RC1 wasn't here during Beta cycle.

It will be up to the Release Lead to keep it in the milestone or not.

[audrasjb]

Upgrade/Install: Add plugins that doesn’t support auto-updates to filters count

[audrasjb]

Before the patch: plugins that doesn't support auto-updates were counted in the wrong filter

[audrasjb]

After the patch: plugins that doesn't support auto-updates are counted in the good filter

[audrasjb]

After the patch: filters works fine \o/

[audrasjb]

50808.diff fixes the issue.

Pinging @pbiron, @desrosj, @whyisjake, @dd32 for peer review as they are aware of the background of the patch that introduced the issue.

No string changes. Hope to see this regression resolved during the RC cycle.

[pbiron]

Just finishing up my "report" and the "raw material" for the new dev-note. When I finish that (and post it to #core-auto-updates) I'll test this patch.

[pbiron]

after some investigation, it appears that the problem as reported and demonstrated in the screenshots JB attached here, only happens if a site has auto-updates enabled for some plugins that do not hook into the Updates API before updating to RC1 and/or [48669] was committed, e.g., a site that had been running beta4 or 5.4 w/ the feature plugin.

Both @audrasjb and I have tested that hypothesis and it seems to be the case. Would love to have someone else check whether it is true. To do that:

• install RC1 on a fresh site, with at least 1 plugin that doesn't hook into the API
• enable auto-updates for 1 or more plugins that do hook into the API
• confirm that the plugins that don't hook into the API are included in the count for the Auto-update disabled view and displayed in the list table when that view is selected

or, on a site that had auto-updates enabled for some plugins that don't hook into the API before it was updated to RC1:

• delete the auto_update_plugins from the DB
• enable auto-updates for 1 or more plugins that do hook into the API
• confirm that the plugins that don't hook into the API are included in the count for the Auto-update disabled view (and displayed in the list table when that view is selected)

While the reported behavior is technically a regression, I suspect it will affect very few sites.

But this also raises another question: should plugins that don't hook into the API be counted as having auto-updates disabled?

My initial thought is that they should not be included in either the enabled nor disabled view. If others agree, then 2 more questions come up:

1. does that need to be fixed for 5.5?
2. should there be a Auto-updates not supported view?

I think number 2 can certainly be considered a possible enhancement for 5.5.1. Not sure about number 1.

Thoughts?

[dd32]

My initial thought is that they should not be included in either the enabled nor disabled view. If others agree, then 2 more questions come up:

My assumption was that plugins that do not support auto-update toggling would show up in the Auto-updates disabled view.
I think of that view as a "What plugins do NOT get automatically updated?" the inverse of "What plugins WILL get automatically updated".

50808.diff​ gets a +1 from me, logically it looks correct and the screenshots show what I would expect.

[apedog]

I'm thinking not-supported is its own category/view.
And any time in the future the view is added is fine. I wouldn't even miss it if it wasn't there at all. It's not a particularly useful view (as opposed to Enabled/Disabled views) but I get how completionists might want it there. In my mind it's just clutter.

[afragen]

I'm not sure that unsupported plugins should show up in either Auto-updates enabled or Auto-updates disabled. Using those tabs provides for a methods of toggling the auto-update switch. Including a non-functional plugin in either of those tabs seems confusing.

I think of Auto-updates disabled as those plugins that CAN be auto-updated, but the user chooses not to. In this case the auto-update isn't simply disabled, it's not present and not capable of being enabled.

[dd32]

I think of Auto-updates disabled as those plugins that CAN be auto-updated, but the user chooses not to. In this case the auto-update isn't simply disabled, it's not present and not capable of being enabled.

I guess it depends highly upon the way you think of the filters. But to use the existing format of the plugins page, we've got Active/Inactive, Recently Active, Update Available, and now Auto-update Enabled/Disabled.

Given the language constructs in use, it doesn't make sense (to me) to split the latter into Auto-Update Enabled/Disabled/Unsupported when all Unsupported are by design Disabled. It should also be pointed out that at this point it's splitting hairs, as the vast majority of users will most likely rarely have an unsupported plugin listed.

Additionally, to use the Active/Inactive case, we count plugins that can't be activated as Inactive, such as those who use Network: true, to say that enabled/disabled is any different from active/inactive when they're both boolean terms seems like it'll introduce more confusion than it'll solve, even if it goes against some purist notions.

[afragen]

Objection withdrawn. Let’s move forward.

Replying to dd32:

I think of Auto-updates disabled as those plugins that CAN be auto-updated, but the user chooses not to. In this case the auto-update isn't simply disabled, it's not present and not capable of being enabled.

I guess it depends highly upon the way you think of the filters. But to use the existing format of the plugins page, we've got Active/Inactive, Recently Active, Update Available, and now Auto-update Enabled/Disabled.

Given the language constructs in use, it doesn't make sense (to me) to split the latter into Auto-Update Enabled/Disabled/Unsupported when all Unsupported are by design Disabled. It should also be pointed out that at this point it's splitting hairs, as the vast majority of users will most likely rarely have an unsupported plugin listed.

Additionally, to use the Active/Inactive case, we count plugins that can't be activated as Inactive, such as those who use Network: true, to say that enabled/disabled is any different from active/inactive when they're both boolean terms seems like it'll introduce more confusion than it'll solve, even if it goes against some purist notions.

[pbiron]

Thanx for the feedback everyone.

@afragen:

I think of Auto-updates disabled as those plugins that CAN be auto-updated, but the user chooses not to. In this case the auto-update isn't simply disabled, it's not present and not capable of being enabled.

That is my initial thought as well.

@dd32:

Additionally, to use the Active/Inactive case, we count plugins that can't be activated as Inactive

But that point convinces me that the unsupported view is not necessary (at least for 5.5).

I've also thought of another situation where a plugin could end up in get_site_option( 'auto_update_plugins', array() ) even though updates aren't supported: a plugin that is in the .org repo (and hence supports updates) is has auto-updates enabled in the UI and then gets pulled from the repo. so the situation I described as only impacting the few sites that had auto-updates enabled for non-updatable plugins prior to RC1 could actually happen to a larger group of sites.

So, yeah, I think 50808.diff​ is actually necessary (and works as expected).

Trac ticket: https://core.trac.wordpress.org/ticket/50808

Plugins that filter auto_update_plugin and returns true should be listed in Auto-updates Enabled view instead of Auto-updates Disabled view.

[chriscct7]

#50825 was marked as a duplicate.

[pbiron]

Following up, there has been a ​discussion on Slack about this.

It looks to me that the change in ​Dion's PR correctly handles this and the code is easier to read/follow than 50808.diff​.

Trac ticket: https://core.trac.wordpress.org/ticket/50808

[whyisjake]

I'm also in favor of the approach found in Dion's ​PR. Would love to get this landed.

[SergeyBiryukov]

Replying to pbiron:

It looks to me that the change in ​Dion's PR correctly handles this and the code is easier to read/follow than 50808.diff​.

Same here, the PR looks good to me.

[whyisjake]

(I neglected to add the #, so this commit doesn't show on the ticket)

WordPress commit r48703

Upgrade/Install: Ensure the proper count of plugins that have auto-update enabled.

As certain plugins may not support updates, this count needs to accurately reflect that in the navbar.

Fixes #50808.
Props knutsp, audrasjb, pbiron, dd32, apedog, afragen, chriscct7, garyc40, whyisjake, SergeyBiryukov.

[whyisjake]

WordPress commit r48704

Upgrade/Install: Ensure the proper count of plugins that have auto-update enabled.

As certain plugins may not support updates, this count needs to accurately reflect that in the navbar.

This brings the changes from [48703] to the WordPress 5.5 branch.

Fixes #50808.
Props knutsp, audrasjb, pbiron, dd32, apedog, afragen, chriscct7, garyc40, whyisjake, SergeyBiryukov.

Fixed in r48703

Merged into WP Core in https://core.trac.wordpress.org/changeset/48704