WordPress.org

Make WordPress Core

Opened 4 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#51129 closed defect (bug) (fixed)

Avoid the Enable/Disable auto-updates links to appear for externally hosted themes on the Network Admin > Themes screen

Reported by: audrasjb Owned by: pbiron
Milestone: 5.5.1 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch has-screenshots
Focuses: multisite Cc:

Description (last modified by afragen)

This is a follow-up to #50280

As noted by @pbiron in comment number 66:


The above (Note: see [48688]) commit did not include the requisite changes to WP_MS_Themes_List_Table. The impact is that in multisite, the Enable/Disable auto-updates links will still appear for externally hosted themes on the Network Admin > Themes screen.

I'm looking into what it will take to add the multisite support for this. It's complicated by the fact that plugins use an array (returned by get_plugin_data()) and thus can use array_merge() on the info in the transient, array( 'update-supported => true ) and the plugin_data; whereas themes use an instance of WP_Theme...making the array merge not possible.

I think something can be worked out using the magic __get() and __set() methods of WP_Theme...but still looking into that.

Attachments (6)

51129.diff (3.4 KB) - added by afragen 3 weeks ago.
51129.2.diff (5.1 KB) - added by pbiron 3 weeks ago.
51129.3.diff (4.4 KB) - added by afragen 3 weeks ago.
fix loose comparison
before.png (142.7 KB) - added by audrasjb 3 weeks ago.
Before last patch
after.png (139.5 KB) - added by audrasjb 3 weeks ago.
After last patch
Capture d’écran 2020-08-27 à 23.14.50.png (123.7 KB) - added by audrasjb 3 weeks ago.
The theme without auto-update available shows in the disable tab

Download all attachments as: .zip

Change History (22)

#1 @SergeyBiryukov
4 weeks ago

  • Description modified (diff)

#2 @afragen
4 weeks ago

  • Description modified (diff)

So I have a patch that seems to correctly set the auto-update tabs count in prepare_items() and correctly adds/removes the Enable|Disable auto-updates links in column_autoupdates().

Themes are very much different from plugins as we know.

I'm missing why we actually need the theme headers for setting the auto-updates. Can someone explain this a bit for me? I've been working with plugin/theme updates for a while and know I can add the array( 'update-supported => true ) if it's really needed.

#3 @afragen
4 weeks ago

  • Description modified (diff)

#4 @afragen
4 weeks ago

I don't know how I messed up your description edit @SergeyBiryukov, but I think I fixed it.

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


3 weeks ago

@afragen
3 weeks ago

#6 @afragen
3 weeks ago

  • Keywords has-patch dev-feedback needs-testing added; needs-patch removed

Initial pass at a patch. This should correctly do the following.

  1. Count the number of enabled/disabled auto-updates in the network themes tabs.
  2. Correctly display or not display the auto-update link.
  3. Add the auto_update_theme filter to multisite.

I'm certain this isn't complete, but it's a start.

Last edited 3 weeks ago by afragen (previous) (diff)

@pbiron
3 weeks ago

#7 @pbiron
3 weeks ago

  • Keywords dev-feedback removed

Thanx for the patch @afragen

51129.2.diff builds on it and I think brings the multisite themes list table into parity with the plugins list table, as far as how the "Automatic Updates" column is populated.

A few things to note:

  1. it does not relying on the __get() and __set() magic methods of WP_theme (as suggested in my comment on the other ticket). It simply dynamically creates the update_supported and auto_update_forced properties of the WP_Theme objects as needed.
  2. notice that those property names are slightly different than the dynamic array keys used in the plugins list table (which are update-supported and auto-update-forced). That makes it easier to get/set those dynamic properties. We could use $theme->{'update-supported'}, but that syntax of really hard to deal with :-)
  3. it correctly (I think) displays Auto-update enabled or Auto-update disabled as plain text (instead of the normal enable/disable links) when the auto_update_theme filter returns a non-null value (51129.diff didn't do that)

With 5.5.1-RC1 scheduled for tomorrow, this could use some testing ASAP.

#8 @pbiron
3 weeks ago

  • Owner set to pbiron
  • Status changed from new to assigned

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


3 weeks ago

@afragen
3 weeks ago

fix loose comparison

#10 @afragen
3 weeks ago

Fixes a loose comparison that exists in class-wp-plugins-list-table.php line 1127 that was copied. Discussed with @pbiron.

@SergeyBiryukov you might want to fix that  🙃

Last edited 3 weeks ago by afragen (previous) (diff)

@audrasjb
3 weeks ago

Before last patch

@audrasjb
3 weeks ago

After last patch

#11 @audrasjb
3 weeks ago

  • Keywords has-screenshots added; needs-testing removed

Just tested the last patch on a multisite with a custom theme and it works fine.
I also reviewed the code and it looks good on my side.

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


3 weeks ago

#13 @afragen
3 weeks ago

@audrasjb and the theme that doesn’t have an auto-updates link should show in the Disabled auto-updates tab.

@audrasjb
3 weeks ago

The theme without auto-update available shows in the disable tab

#14 @audrasjb
3 weeks ago

@afragen yes, I can confirm

#15 @SergeyBiryukov
3 weeks ago

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

In 48899:

Upgrade/Install: Only display the auto-update links on the Network Admin > Themes screen for themes that support the feature.

Follow-up to [48669], [48688].

Props afragen, pbiron, audrasjb, desrosj, SergeyBiryukov.
Fixes #51129.

#16 @SergeyBiryukov
3 weeks ago

In 48900:

Upgrade/Install: Only display the auto-update links on the Network Admin > Themes screen for themes that support the feature.

Follow-up to [48669], [48688].

Props afragen, pbiron, audrasjb, desrosj, SergeyBiryukov.
Merges [48899] to the 5.5 branch.
Fixes #51129.

Note: See TracTickets for help on using tickets.