Multisite theme update not showing new version number

[afragen]

When updating plugins via the ajax updater the new version number displays correctly in the plugin row when the update is completed.

In multisite, when a theme is updated via the ajax updater the new version number is not displayed in the theme row.

[afragen]

Adjusted the conditional check. I think it makes more sense.

[swissspidy]

A few notes on the first patches:

• The old version needs to be fetched before any update happens, otherwise it's just the new version
• The upgrader object holds the theme info, not the upgrader skin ($upgrader->theme_info() method)

40764.2.diff​ addresses this.

[afragen]

$upgrader->skin->theme_info already exists and it really does contain the current theme_info.

If you want to explicitly get the current theme, that works too and might be easier to understand in code. I was simply trying to work with the data that currently existed.

Thanks for the feedback. I appreciate it.

[afragen]

If we are going to explicitly obtain the current WP_Theme object, for consistency, shouldn’t we use the same conditional check as in https://core.trac.wordpress.org/attachment/ticket/40764/40764.diff

if ( $theme->get( ‘Version’ ) ) {

Sorry not really good at WikiFormat.

[swissspidy]

Replying to afragen:

$upgrader->skin->theme_info already exists and it really does contain the current theme_info.

If you want to explicitly get the current theme, that works too and might be easier to understand in code. I was simply trying to work with the data that currently existed.

Thanks for the feedback. I appreciate it.

You're right. I see now that there's a $this->skin->theme_info = $this->theme_info($theme); line in Theme_Upgrader::bulk_upgrade(). I got suspicious because theme_info isn't an explicit property of the skin. Plus, that property is set within a loop and could theoretically get overridden by accident in a future change. Thus, it's not really straightforward. It would be safer to use Theme_Upgrader::theme_info() directly within the Ajax callback. However, this is a) just a wrapper for wp_get_theme() and b) it was called too late anyway and never returned the old version.

Regarding the conditional check in 40764.diff​: that doesn't work reliably. $upgrader->skin->theme_info can be false. Not checking for that can lead to warnings.

! empty( $upgrader->skin->theme_info ) is better, but still not an explicit check for false or WP_Theme. Also, wp_get_theme() recommends checking the exists() method, which is what I did in 40764.2.diff​

[afragen]

Sounds great thanks for the explanation. I also didn't know if the exists() method. I learned something, thanks.

[afragen]

Based on core-multisite Slack chat ​https://wordpress.slack.com/archives/C03BVB47S/p1503336138000172 a patch refresh is attached.

I also changed the conditional for newVersion to match so both are similar.

$theme = wp_get_theme( $stylesheet );
    if ( $theme->exists() ) {
...

[pmbaldha]

I have tested 40764.2.diff and it is working fine. I have tested without patch, Network theme update is showing version number. After apply the 40764.2.diff patch, it works fine. There was some coding strandard related issue in 40764.2.diff, I have given it to @afragen.

However, I am not able to apply patch 40764.3.diff in my new checkout.

[afragen]

Replying to pmbaldha:

I have tested 40764.2.diff and it is working fine. I have tested without patch, Network theme update is showing version number. After apply the 40764.2.diff patch, it works fine. There was some coding strandard related issue in 40764.2.diff, I have given it to @afragen.

However, I am not able to apply patch 40764.3.diff in my new checkout.

I think the issue was my local setup. See if the latest patch doesn't install cleanly.

[desrosj]

Tested this on a subdomain and subdirectory install. Both fixed the issue for me. I think this is good to go.

[swissspidy]

In 41611:

Upgrade/Install: Show new version number when updating a theme on Multisite.

Props afragen.
Fixes #40764.

[swissspidy]

In 41619:

Upgrade/Install: Update unit tests after [41611].

See #40764.