Core auto-updates UI: disable checkbox when filter or constant takes precedent on the option

[audrasjb]

Core major versions auto-updates option needs to be disabled when WP_AUTO_UPDATE_CORE constant or allow_major_auto_core_updates filter are used.

Related tickets: #50907, #51698.

[audrasjb]

Upgrade/Install: Disable major auto-updates checkbox when WP_AUTO_UPDATE_CORE constant and/or allow_major_auto_core_updates filter are defined

[audrasjb]

Auto-updates set to true using a constant or a filter.

[audrasjb]

Auto-updates set to false using a constant or a filter.

[audrasjb]

Auto-updates option available when no constant or filter takes precedent

[audrasjb]

51742.diff disables the checkbox and append a small information about this in the label, in italics. The goal of this patch is to make sure constants and filters will take precedent on user settings.

Just adding some thoughts:

• We could also hide the whole auto-updates section when filters/constants are used.
• Of course, the disabled state of an input element can always be bypassed, but I think it doesn't matter as the constant/filter will always take precedent.

[mukesh27]

Updated patch will remove extra space and empty p tag

[mukesh27]

Patch 51742.patch​ removes extra space from the disabled tag and checked function.

When auto-update set to false using define('WP_AUTO_UPDATE_CORE', false) then an empty p tag generated that also removed in a patch.

Feel feel to update patch if anything wrong

[azaozz]

Hmm, maybe the text

This setting has been disabled using a constant or a filter.

would be better as

This setting was overridden programmatically. (or something similar)

Thinking majority of users wouldn't know what constant and filter are. Perhaps better to avoid mentioning these in the UI.

[audrasjb]

Thanks @azaozz I agree with you, that's better 👍
Will refresh the patch accordingly.

@mukesh27 I was going to address the <p> issue in another ticket, but let's address this here. I'll update your patch though, as there is a WPCS issue in it :)

[audrasjb]

51742.2.diff

[mukesh27]

@audrasjb Patch 51742.2.diff​ address src/js/_enqueues/vendor/tinymce/tinymce.js related updates

[audrasjb]

Ah thanks, forgot to merge master changes ;)
Patch updated.

[audrasjb]

Upgrade/Install: Disable major auto-updates checkbox and display a message when WP_AUTO_UPDATE_CORE constant and/or allow_major_auto_core_updates filter are defined

[knutsp]

What if AUTOMATIC_UPDATER_DISABLED is true?

[audrasjb]

Move the auto-updates UI ti the top and replace form/checkbox with action links

[audrasjb]

As per today’s weekly core-auto-updates team meeting, this patch implements the following parts of the new scope for major versions auto-updates system:

• Include all previous changes concerning filters and constants
• Move the auto-updates UI to the top and replace form/checkbox with action links

@karmatosed @helen I'll post few screenshots. I think the technical part is ready and we can iterate on design/copy.

Pinging @whyisjake for a quick security review as action links are using GET variables.

[audrasjb]

Running trunk: auto-updates are not enable so we have a link to enable them

[audrasjb]

Running trunk: auto-updates are enabled so we have a link to disable them (PS: I just clicked on the "enable" action link so we also have the notice bar on the top of the screen)

[audrasjb]

Auto-updates settings are already managed using a constant or filter: the option is not available and there is a sentence to explain that.

[azaozz]

Replying to audrasjb:

Looking at 51742.3.diff, see couple of problems and have few (nitpick) suggestions :)

1. core_auto_updates_settings() should use a nonce when saving the options. As it's a link now the core-auto-updates-major query arg and the nonce will be in the $_GET which is not ideal (if the user bookmarks the page while ?core-auto-updates-major=... is in the URL, it will try to set the option every time they use that bookmark). To fix that it should use wp_nonce_url() and then immediately redirect after the option is updated (for example see how "action links" in list tables work).

2. The filter allow_major_auto_core_updates runs two times in a row. Perhaps that's a typo, it's pretty confusing and not sure it is necessary.

[audrasjb]

I'll add the nonce and immediately redirect just like other action links do 👍
For allow_major_auto_core_updates, that's indeed a typo, sorry about that…

Thanks @azaozz for your review!

[azaozz]

Replying to audrasjb:

Suggestions and nitpicks:

1. The $major_optin_disabled = false; var is a "double negative". Perhaps would be easier to read/understand if it was a "positive", i.e. $major_optin_enabled = true;? Also the name could be a bit more descriptive, perhaps, like $upgrade_major_optin_enabled or similar.

2. The $is_major_optin_disabled doesn't seem needed? If there should be a specific filter for disabling major core updates that runs in addition to $upgrade_major = apply_filters( 'allow_major_auto_core_updates', ..., it can perhaps be checked right in the conditional (and expect only boolean true).

3. Perhaps do_action( 'after_core_auto_updates_settings_fields', $auto_update_settings ); can be only after_core_auto_updates_settings as there aren't any "fields" there.

4. The auto_update_core_major and similar site options would perhaps be better as explicitly set to enabled or disabled strings. It's more readable and, (as PHP 8.0 explicitly requires) it would be better to save and then expect a string, not a boolean that gets saved in the DB as a number, etc. Also, these new options should be pre-populated when updating to WP 5.6.

5. The WordPress auto-update settings updated. notice could perhaps be more specific and "tell" whether major core auto-updates were enabled or disabled.

6. Looking at the UI, maybe it would be better to have a "link as a button" for the action link? The rest of the "actions" are all buttons.

[audrasjb]

@azaozz ah, about running the filter two times in a row, that's because it serves two different goals:

$upgrade_major = apply_filters( 'allow_major_auto_core_updates', $upgrade_major );

The idea is to get the value or the filter or to apply $upgrade_major value. This is for the final value of the option.

$is_major_optin_disabled = apply_filters( 'allow_major_auto_core_updates', null );

The idea is to test if the filter should disable the UI. If it returns null, we know that the filter is not applied.

But that's coming from the previous way this section was managed. We can probably adapt that and get rid of this double call.

[audrasjb]

Just saw your last comment. All your points totally make sense to me, thanks 🙏
New patch coming soon :)

Ah. Except item 6: the idea is also to decrease the amount of buttons on this page, as this is something asked by the design team :)

[azaozz]

Replying to audrasjb:

@azaozz ah, about running the filter two times in a row, that's because it serves two different goals
...
The idea is to test if the filter should disable the UI

Ah, I see. Yeah, it is quite confusing (for plugins) now, as the same filter runs twice and filters different values. Perhaps the second run can be another, specific filter? Lets discuss/figure it out on Slack :)

[azaozz]

Replying to audrasjb:

Except item 6: the idea is also to decrease the amount of buttons on this page, as this is something asked by the design team :)

Right, it's just a nitpick. An inline link is less noticeable/less "important" than a button. Best to implement the design team's decision :)

[audrasjb]

Replying to azaozz:

Also, these new options should be pre-populated when updating to WP 5.6.

That's a specific ticket, so it can be committed independently: #51743 :)

[azaozz]

Uh, didn't mean to remove needs-refresh, posting too fast, sorry :)

[karmatosed]

This is already looking a lot better I think from a position and format view. Thanks for the rapid work. I think it's also really important to see this as a step to iterate from, over being set in stone for coming up versions.

The big thing right now for me to work on for this release is copy. There's an opportunity here to avoid the confusion and sea of content that we are adding to with this. I would like therefore some consideration before strings go in to a few areas:

• Concise, exact language: how space can we be, but still clear?
• Clarity for translation: let's be aware this is going to be translated and consider that for understanding.
• Warmth: how can we ease any feeling around this feature in the copy?
• Balance: whilst we want to encourage this feature, we want it to be natural to use and trusted, not feel a big deal. Copy can help there with tone.

By reducing the interface to a link, the weight has been taken from it, so now we can turn to the text being delivered itself.

A few specific things I think need changing are:

• Length of linked text: I see in one screenshot a link that is over 8 words long, let's try and avoid that.
• Consider opening words like inviting, is this too formal? It might not be, but let's consider the tone of the messaging here.
• Potentially use more spacing between messages and group them. I am not sure when we have 4 lines of text in row we can't also just have some extra white space. This could give breathing room in a sea of text as per the mockups around this feature, I'd love to see that come to this a little.

Perhaps as it's important, writing out each potential message strings is useful to then iterate collectively in this ticket?

[audrasjb]

Address Andrew’s feedback and use enabled/disabled for option value

[azaozz]

In 51742.5.diff:

• Building on 51742.4.diff.
• Use action to follow the existing pattern.
• Use has_filter() when determining if the UI has been overridden by a plugin/theme.
• Some cleanup, etc.

[audrasjb]

Updates 51742.5.diff to also take care of AUTOMATIC_UPDATER_DISABLED constant

[audrasjb]

Thanks @azaozz! Just added 51742.6.diff which updates your patch to also give precedence to AUTOMATIC_UPDATER_DISABLED, as noted by @knutsp :)

Also, I wanted to mention that @karmatosed and @hedgefield worked on the UI copy. I'll update the patch with the decided strings later.
​https://docs.google.com/document/d/1mxplJ4BfWLxtbxpFhPTSH1x3FpbIiBfOfzNECh37Mm8/edit?usp=sharing

[azaozz]

Replying to audrasjb:

Just added 51742.6.diff which updates your patch to also give precedence to AUTOMATIC_UPDATER_DISABLED

Thanks, looks good :)

There's some more: we changed the values saved in the DB for the new options (auto_update_core_dev, auto_update_core_minor, and auto_update_core_major) to avoid PHP type juggling, but the filtered values for allow_dev_auto_core_updates, allow_minor_auto_core_updates, and allow_major_auto_core_updates filters would still need to be boolean as these filters have existed since 2013. Patch coming up.

Also all new options have to be added to populate_options() to avoid making extra calls to the DB for non-existing options. Will update the patch on #51743.

Still todo: test in multisite/network in both existing and new installs.

[azaozz]

In 51742.7.diff:

• Update expected option values in Core_Upgrader::should_update_to_version().
• Use boolean values for the (pre-existing) allow_dev_auto_core_updates, allow_minor_auto_core_updates, and allow_major_auto_core_updates filters.
• Fix couple of typos in comments.

[hedgefield]

Alright, out of the doc suggestions these seem the most agreed upon ones we can use for beta 4 this afternoon, with some slight edits to make the phrasings match as much as possible:

Use case 1 - Auto-updates were enabled by clicking the action link and no constant or filter are used:

Automatic updates to new versions of WordPress are enabled. You can <a>turn this off</a>.

Use case 2 - Auto-updates for major versions are disabled and no constant or filter are used:

If you want you can <a>automatically update your WordPress site</a>.</a>

Use case 3 - Auto-updates were enabled via a filter or constant:

Your WordPress site will be kept updated automatically.

Use case 4 - Auto-updates were disabled via a filter or constant:

Automatic WordPress updates have been disabled for this site.

Use case 5 - Core auto-updates are completely disabled:

Automatic WordPress updates are not available for this site.

Admin notices after settings changes, displayed on page reload:

WordPress will auto-update to new major versions.
WordPress will no longer auto-update to new major versions.

[audrasjb]

Implement string changes

[audrasjb]

Thanks for the update @azaozz!
Thanks for the strings @hedgefield!
51742.8.diff implements those string changes 🙂

I believe we are good to go, nice work all!

[audrasjb]

I checked again 51742.8.diff and it looks good to me. Adding commit keyword.

[helen]

Some general notes from me on strings here (which I can also update a patch for):

1. I would like for strings to refer to "this site" intsead of "your site". This mirrors some other string changes that have been made to reflect that many admins have multiple sites and that a given site might not be "theirs".
2. I do not think that it makes sense to refer to "major" or "feature" releases. Even I'm confused about what this means in the context of the WordPress admin - it only makes sense to me as a core developer.
3. For the purposes of these strings, there are 3 states: all core autoupdates on, minor autoupdates on, no autoupdates on. Filter/constant usage means removing the action link, which comes with the additional benefit of reducing the number of translatable strings.
4. I think there are actually two places where a string can show - at the top where it has the check again link under the heading, and within the update available section below. For the upper one, I would like this to be a statement about what is enabled and with a clear action link. For the lower one, I think this could be more of a conversational "would you like to change this to X" type of string.
5. Per the above, these are my suggestions which were in the doc but I think overlooked because there was a lot going on:

Top string, all updates on:

This site is automatically kept up-to-date with each new version of WordPress. <a>Switch to automatic updates for maintenance and security release only</a>

Top string, minor updates on:

This site is automatically kept up-to-date with maintenance and security releases of WordPress. <a>Enable automatic updates for all new versions of WordPress</a>

Top string, no updates on (this can only be done in code so no action link)

This site will not receive automatic updates for new versions of WordPress.

Admin notice after turning on major updates:

Automatic updates for all WordPress versions have been enabled. Thank you!

Admin notice after turning off major updates:

WordPress will only receive automatic security and maintenance releases from now on.

For the lower update section, I think we would only want to show an action link to enable automatic updates going forward as an additional prompt. This could read:

You can also <a>enable automatic updates for all versions of WordPress</a> from now on.

[karmatosed]

@helen I like those iterations. Good point about taking it away from personal to site specfic, alinging it with other strings is important.

My only ponder is the long sentnace link, but it's not a deal breaker for me:

Switch to automatic updates for maintenance and security release only

Potentially I'd iteration to:

This site is automatically kept up-to-date with each new version of WordPress. <a>Switch to automatic updates</a> for maintenance and security release only.

This site is automatically kept up-to-date with maintenance and security releases of WordPress. <a>Enable automatic updates</a> for all new versions of WordPress.

I would +1 those string changes as we can get them in for beta then.

[audrasjb]

As said on Slack and just in case you didn't see the conversation @karmatosed, I'm afraid we can't use "Switch to automatic updates for maintenance and security release only" as disabling major auto-udpates doesn't mean that security updates will be enabled. I know that sounds weird, but technically, minor updates can be disabled while majors are enabled 😅

That's possible because of filters bellow: They were introduced in WP 3.7 and are totally independant each others:

add_filter( 'allow_dev_auto_core_updates', '__return_true' );           // Enable development updates
add_filter( 'allow_minor_auto_core_updates', '__return_true' );         // Enable minor updates
add_filter( 'allow_major_auto_core_updates', '__return_true' );         // Enable major updates

[marybaum]

Replying to karmatosed:

I of course would like to see even something that happens automagically use active verbs.

How about:

This site stays up to date automatically with every new version of WordPress ...
for majors,

and

This site stays up to date automatically with maintenance and security releases ...

PS -- we only hyphenate 'up to date' when it directly modifies another word, becoming an adjective or adverb phrase. When it's the object of a verb that's acting as a standin for the verb 'to be', we don't hyphenate.

So, 'It's up to date!' But, 'I have the up-to-date strings.'

@helen I like those iterations. Good point about taking it away from personal to site specfic, alinging it with other strings is important.

My only ponder is the long sentence link, but it's not a deal breaker for me:

Switch to automatic updates for maintenance and security release only

Potentially I'd iteration to:

This site is automatically kept up-to-date with each new version of WordPress. <a>Switch to automatic updates</a> for maintenance and security release only.

This site is automatically kept up-to-date with maintenance and security releases of WordPress. <a>Enable automatic updates</a> for all new versions of WordPress.

I would +1 those string changes as we can get them in for beta then.

[helen]

In 49587:

Upgrade/Install: Better UI for auto-update settings on update screen.

This adds clearer messages about what your current settings mean for updates, uses a more compact link-based action instead of a checkbox to change the setting, and respects constants and filters.

Props audrasjb, karmatosed, helen, azaozz, hedgefield, marybaum.
Fixes #51742.

[helen]

[49587] gives us something like ​https://share.getcloudapp.com/v1uxXNpm (note that there are a ton of possible UIs for this screen that need screenshot documentation). Reopening because I would like to do the following before RC:

• Add an additional prompt link in the An updated version of WordPress is available. section. Note that I've changed the string there from You can update ... automatically string to say You can update ... manually to reflect that you are clicking the button, not doing something with automatic updates. Per my other comment, I think this could say something like You can also <a>enable automatic updates for all versions of WordPress</a> from now on..
• Potentially abstract the logic around getting the autoupdate settings into its own function, as the above will also need that information, and this whole file is very messy with functions and procedural PHP so it's a bit hard to trace what's going on.
• Consider adding a string for when major auto-updates are on but minor ones are off - IMO this is a strange and non-ideal thing to do, but is technically possible so the informational part should probably reflect that. I really don't like referring to these as "major" or "feature" updates in user-facing strings but we probably can't avoid that entirely here. Because it is only possible to turn off minor updates via code and I consider this to be a non-ideal state, we do not need an action link.

For the last item, this technically boils down to changing if ( $upgrade_major ) to if ( $upgrade_major && $upgrade_minor ) and then adding an additional elseif ( $upgrade_major ) with whatever we think the right string is for only updating to new major/feature versions.

In 5.7, I would like to tackle further UI updates to the updates screen in #51540 (in particular condensing the button overload) and take a look at also adding another way to manage these settings in general or similar. There are some other potential 5.7 items on ​https://make.wordpress.org/core/2020/11/10/wp5-6-auto-update-implementation-change/

[markparnell]

add_filter('automatic_updater_disabled', '__return_true'); should result in the message This site will not receive automatic updates for new versions of WordPress., however it currently has no impact on the UI. Setting the corresponding constant to false does result in the UI updating correctly.

[audrasjb]

Handles automatic_updater_disabled filter as per Mark’s comment

[audrasjb]

@markparnell thanks for spotting this during the release party.
51742.10.diff fixes this issue.

It was not a blocker for beta 4 but that's definitely something we want to fix before RC1 :)

[azaozz]

Replying to helen:

Consider adding a string for when major auto-updates are on but minor ones are off - IMO this is a strange and non-ideal thing to do, but is technically possible...

Yeah, this doesn't make sense even though it's been technically possible. Logically, when turning major updates on, it should override minor updates to always "on".

This is the same case as when beta or rc are set in the WP_AUTO_UPDATE_CORE constant, see: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-core-upgrader.php#L287. So instead of adding a string, lets "fix" the logic there. True, this is a change (a logic enhancement?) to how the various "global" auto-update filters and constants work, but thinking now is the time to fix it.

Plugins can block any update depending on the "offered version" with the auto_update_[core] filter: https://core.trac.wordpress.org/browser/tags/5.5.3/src/wp-admin/includes/class-wp-automatic-updater.php#L178, see the $item param. It is possible to fully control which updates are automatically applied without even looking at the "global settings" in options, constants, and other filters.

[azaozz]

Replying to audrasjb:

@markparnell thanks for spotting this during the release party.
51742.10.diff fixes this issue.

51742.10.diff looks good.

Yeah, it's a pretty "big mess" how the various auto-update constants and filters work. That also affects the UI...

Would be great to add a (big) docblock explaining all of these and how they interact, just have to find the right place for it :)

As of WP 5.6:

1. There are three options that (may) control core auto-updates: auto_update_core_dev, auto_update_core_minor, and auto_update_core_major. There is UI only for the last one, and that UI is on the Dashboard -> Updates screen. There are various constants and filters that override these options.

2. There is the WP_AUTO_UPDATE_CORE constant that can be used to enable or disable core auto-updates for beta, rc, minor and major versions.
   • This constant can be overridden by the allow_dev_auto_core_updates, allow_minor_auto_core_updates, and allow_major_auto_core_updates filters.
   • This constant and the filters that override it can be overridden by the auto_update_[core] filter in WP_Automatic_Updater::should_update().

3. Then there is also the AUTOMATIC_UPDATER_DISABLED constant that overrides the WP_AUTO_UPDATE_CORE constant and all the filters that override it (including the last one).
   • The AUTOMATIC_UPDATER_DISABLED constant also has a filter automatic_updater_disabled to override it.
   • In addition AUTOMATIC_UPDATER_DISABLED and the corresponding filter can be overridden for themes and plugins by the plugins_auto_update_enabled and themes_auto_update_enabled filters in wp_is_auto_update_enabled_for_type(). (Currently this function only supports plugins and themes. Perhaps would be good to add support for core too?)

Did I miss something? :)

Please edit/correct the above and lets add it in a docblock, probably best in update-core.php or for the abstracted function @helen mentioned above.

[SergeyBiryukov]

In 49591:

I18N: Remove HTML tags from translatable strings on WordPress Updates screen.

Follow-up to [49587].

Props fierevere.
See #51742.

[SergeyBiryukov]

In 49592:

Upgrade/Install: Account for the automatic_updater_disabled filter in core auto-update settings UI.

Follow-up to [49587].

Props markparnell, audrasjb.
See #51742.

[SergeyBiryukov]

Replying to audrasjb:

51742.10.diff fixes this issue.

Just a quick note that in [49592] I changed the value passed to the filter from null to false, for consistency with the corresponding Site Health check​ and the documented type of 'bool'​.

I know we pass null to the auto_update_{$type} filter to detect whether nothing has hooked into it, as discussed in #50848, but in this case it looks like we just need to detect whether the updater is disabled or not, so passing false seems appropriate.

[audrasjb]

Add "only" to Core auto-updates status string when sites are set to auto-update to minors versions only

[audrasjb]

As seen with @azaozz and @karmatosed in ​Core auto-updates Slack channel today, let's add "only" to the auto-updates status string when updates are set to minor versions only.

Before patch:

This site is only automatically kept up to date with maintenance and security releases of WordPress.
<a>Enable automatic updates for all new versions of WordPress.</a>

After patch

This site is automatically kept up to date with maintenance and security releases of WordPress.
<a>Enable automatic updates for all new versions of WordPress.</a>

[audrasjb]

Before applying 51742.11.diff

[audrasjb]

After applying 51742.11.diff

[audrasjb]

Add "only" to Core auto-updates status string when sites are set to auto-update to minors versions only and adding a heading in case we are running a development version of WordPress

[audrasjb]

Before 51742.12.diff on development versions of WordPress

[audrasjb]

After 51742.12.diff on development versions of WordPress

[audrasjb]

Add "only" to Core auto-updates status string when sites are set to auto-update to minors versions only and adding a heading in case we are running a development version of WordPress. Also adds a class to the enable|disable link for better CSS/JS targeting

[audrasjb]

In 51742.13.diff:

• Add "only" to Core auto-updates status string when sites are set to auto-update to minors versions only
• Adding a specific heading in case we are running a development version of WordPress
• Add a class to the enable|disable link for better CSS/JS targeting

Adding commit keyword as all of this was discussed and reviewed in core-auto-updates slack channel.

[pbiron]

51742.14.diff​ incorporates 51742.13.diff​ (with one minor change to the new "only" string as suggested ​on slack) and adds an additional check for whether the site is under VCS and displays a separate "auto-updates disabled" for that case.

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

[helen]

After 15.diff, VCS message, on development with an update available

[helen]

After 15.diff, VCS message, on latest development, showing that you can still install the nightly which seems fine

[helen]

After 15.diff, VCS message, on 5.5.2 with an update available

[helen]

After 15.diff, VCS message, on 5.5.3 with no update available

[helen]

51742.15.diff​ results in the 4 screenshots above when under VCS with various faked version strings (outdated develop, up to date develop, outdated stable, up to date stable). It's an iteration on 51742.14.diff​ that cleans up some logic in core_upgrade_preamble() so that only one h2 is shown at a time (falling back to You have the latest version of WordPress. if something odd happens). It also ensures that the core-major-auto-updates-saved query arg is stripped from the URL because it's only for a dismissible notice.

The one kind of funny thing here is that if you are on the latest dev version it will actually still give you the button to install the latest nightly but it doesn't necessarily say that's an update, which I think is fine because of the nature of nightlies and how they're built. You will also typically see the update available message on SVN checkouts because the nightly version number is bumped more often than it is in SVN. I kind of wonder if we should be hiding that section entirely for VCS checkouts but that's not for 5.6, and it's going to make working on that section a lot more annoying later as contributors :)

[audrasjb]

51742.15.diff is testing good on my side 👍

[helen]

In 49638:

Upgrade/Install: Consistent layout and accurate messages on the update screen.

• Clarifies that if you are on maintenance/security auto-updates that you are only on those and therefore there are more options available.
• Adds a message if a version control system has been detected, as automatic updates are disabled in that case.
• Ensures only one heading between update available, you are on a dev version, and you are on latest appears at any given time, falling back to you are on latest if something strange happens with the returned update data.
• Removes some older strings related to auto-updates, which greatly simplifies the above.
• Strips the core-major-auto-updates-saved query arg from the URL, as it is related to a dismissible notice.

Props audrasjb, pbiron, helen.
Fixes #51742.

Merged in ​https://github.com/WordPress/wordpress-develop/commit/8372bbdfa5141e7ed1d0d0854d4c6dc2dab183df