Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#51742 closed defect (bug) (fixed)

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

Reported by: audrasjb's profile audrasjb Owned by: audrasjb's profile audrasjb
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.6
Component: Upgrade/Install Keywords: has-patch has-screenshots commit
Focuses: ui Cc:

Description

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.

Attachments (31)

51742.diff (2.0 KB) - added by audrasjb 4 years ago.
Upgrade/Install: Disable major auto-updates checkbox when WP_AUTO_UPDATE_CORE constant and/or allow_major_auto_core_updates filter are defined
Capture d’écran 2020-11-10 à 00.59.55.png (365.0 KB) - added by audrasjb 4 years ago.
Auto-updates set to true using a constant or a filter.
Capture d’écran 2020-11-10 à 00.59.23.png (359.5 KB) - added by audrasjb 4 years ago.
Auto-updates set to false using a constant or a filter.
Capture d’écran 2020-11-10 à 01.02.29.png (357.6 KB) - added by audrasjb 4 years ago.
Auto-updates option available when no constant or filter takes precedent
51742.patch (2.8 KB) - added by mukesh27 4 years ago.
Updated patch will remove extra space and empty p tag
Capture d’écran 2020-11-10 à 13.11.09.png (243.0 KB) - added by audrasjb 4 years ago.
51742.2.diff
51742.2.diff (2.9 KB) - added by audrasjb 4 years ago.
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
51742.3.diff (5.1 KB) - added by audrasjb 4 years ago.
Move the auto-updates UI ti the top and replace form/checkbox with action links
Capture d’écran 2020-11-11 à 01.17.09.png (364.2 KB) - added by audrasjb 4 years ago.
Running trunk: auto-updates are not enable so we have a link to enable them
Capture d’écran 2020-11-11 à 01.18.46.png (385.2 KB) - added by audrasjb 4 years ago.
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)
Capture d’écran 2020-11-11 à 01.20.56.png (400.4 KB) - added by audrasjb 4 years ago.
Auto-updates settings are already managed using a constant or filter: the option is not available and there is a sentence to explain that.
51742.4.diff (7.7 KB) - added by audrasjb 4 years ago.
Address Andrew’s feedback and use enabled/disabled for option value
51742.5.diff (7.6 KB) - added by azaozz 4 years ago.
51742.6.diff (7.9 KB) - added by audrasjb 4 years ago.
Updates 51742.5.diff to also take care of AUTOMATIC_UPDATER_DISABLED constant
51742.7.diff (8.1 KB) - added by azaozz 4 years ago.
51742.8.diff (8.1 KB) - added by audrasjb 4 years ago.
Implement string changes
51742.9.diff (11.2 KB) - added by helen 4 years ago.
51742.10.diff (672 bytes) - added by audrasjb 4 years ago.
Handles automatic_updater_disabled filter as per Mark’s comment
51742.11.diff (565 bytes) - added by audrasjb 4 years ago.
Add "only" to Core auto-updates status string when sites are set to auto-update to minors versions only
screencapture-jeanbaptisteaudras-2019-wp-admin-update-core-php-2020-11-16-14_33_52.png (366.6 KB) - added by audrasjb 4 years ago.
Before applying 51742.11.diff
screencapture-jeanbaptisteaudras-2019-wp-admin-update-core-php-2020-11-16-14_34_32.png (367.0 KB) - added by audrasjb 4 years ago.
After applying 51742.11.diff
51742.12.diff (1.2 KB) - added by audrasjb 4 years ago.
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
Capture d’écran 2020-11-16 à 15.01.39.png (268.1 KB) - added by audrasjb 4 years ago.
Before 51742.12.diff on development versions of WordPress
Capture d’écran 2020-11-16 à 15.04.15.png (285.9 KB) - added by audrasjb 4 years ago.
After 51742.12.diff on development versions of WordPress
51742.13.diff (1.9 KB) - added by audrasjb 4 years ago.
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
51742.14.diff (3.4 KB) - added by pbiron 4 years ago.
51742.15.diff (4.6 KB) - added by helen 4 years ago.
Screen Shot 2020-11-17 at 1.50.00 PM.png (149.7 KB) - added by helen 4 years ago.
After 15.diff, VCS message, on development with an update available
Screen Shot 2020-11-17 at 1.50.46 PM.png (138.4 KB) - added by helen 4 years ago.
After 15.diff, VCS message, on latest development, showing that you can still install the nightly which seems fine
Screen Shot 2020-11-17 at 1.51.09 PM.2.png (145.4 KB) - added by helen 4 years ago.
After 15.diff, VCS message, on 5.5.2 with an update available
Screen Shot 2020-11-17 at 1.51.27 PM.png (115.7 KB) - added by helen 4 years ago.
After 15.diff, VCS message, on 5.5.3 with no update available

Change History (90)

@audrasjb
4 years ago

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

@audrasjb
4 years ago

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

@audrasjb
4 years ago

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

@audrasjb
4 years ago

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

#1 @audrasjb
4 years ago

  • Keywords has-patch has-screenshots needs-copy-review needs-design-feedback dev-feedback 2nd-opinion added; needs-patch removed

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
4 years ago

Updated patch will remove extra space and empty p tag

#2 @mukesh27
4 years ago

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

Last edited 4 years ago by mukesh27 (previous) (diff)

#3 @azaozz
4 years ago

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.

Last edited 4 years ago by azaozz (previous) (diff)

#4 @audrasjb
4 years ago

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 :)

#5 @mukesh27
4 years ago

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

#6 @audrasjb
4 years ago

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

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

@audrasjb
4 years ago

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

#7 @knutsp
4 years ago

What if AUTOMATIC_UPDATER_DISABLED is true?

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


4 years ago

@audrasjb
4 years ago

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

#9 follow-ups: @audrasjb
4 years ago

  • Keywords 2nd-opinion removed

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
4 years ago

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

@audrasjb
4 years ago

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
4 years ago

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

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


4 years ago

#11 in reply to: ↑ 9 @azaozz
4 years ago

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).
  1. 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.

#12 @audrasjb
4 years ago

  • Keywords needs-refresh added

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!

#13 in reply to: ↑ 9 ; follow-up: @azaozz
4 years ago

  • Keywords needs-refresh removed

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.
  1. 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).
  1. 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.
  1. 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.
  1. The WordPress auto-update settings updated. notice could perhaps be more specific and "tell" whether major core auto-updates were enabled or disabled.
  1. 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.
Last edited 4 years ago by azaozz (previous) (diff)

#14 follow-up: @audrasjb
4 years ago

@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.

#15 follow-up: @audrasjb
4 years ago

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 :)

#16 in reply to: ↑ 14 @azaozz
4 years ago

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 :)

#17 in reply to: ↑ 15 @azaozz
4 years ago

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 :)

#18 in reply to: ↑ 13 @audrasjb
4 years ago

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 :)

#19 @azaozz
4 years ago

  • Keywords needs-refresh added

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

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


4 years ago

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


4 years ago

#22 @karmatosed
4 years ago

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
4 years ago

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

@azaozz
4 years ago

#23 @azaozz
4 years ago

  • Keywords needs-testing added; needs-refresh removed

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.

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


4 years ago

@audrasjb
4 years ago

Updates 51742.5.diff to also take care of AUTOMATIC_UPDATER_DISABLED constant

#25 follow-up: @audrasjb
4 years ago

  • Keywords needs-design-feedback removed

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

#26 in reply to: ↑ 25 @azaozz
4 years ago

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
4 years ago

#27 @azaozz
4 years ago

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.

#28 @hedgefield
4 years ago

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.
Last edited 4 years ago by hedgefield (previous) (diff)

@audrasjb
4 years ago

Implement string changes

#29 @audrasjb
4 years ago

  • Keywords needs-copy-review dev-feedback needs-testing removed

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!

#30 @audrasjb
4 years ago

  • Keywords commit added

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

#31 @helen
4 years ago

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 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.
  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 version of WordPress</a> from now on.
Version 0, edited 4 years ago by helen (next)

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


4 years ago

#33 follow-up: @karmatosed
4 years ago

@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.

Last edited 4 years ago by karmatosed (previous) (diff)

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


4 years ago

#35 @audrasjb
4 years ago

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

#36 in reply to: ↑ 33 @marybaum
4 years ago

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
4 years ago

#37 @helen
4 years ago

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

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.

#38 follow-up: @helen
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[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/

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


4 years ago

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


4 years ago

#41 @markparnell
4 years ago

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
4 years ago

Handles automatic_updater_disabled filter as per Mark’s comment

#42 follow-ups: @audrasjb
4 years ago

  • Keywords commit removed

@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 :)

#43 in reply to: ↑ 38 @azaozz
4 years ago

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.

Last edited 4 years ago by azaozz (previous) (diff)

#44 in reply to: ↑ 42 @azaozz
4 years ago

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.
  1. 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().
  1. 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.

Last edited 4 years ago by azaozz (previous) (diff)

#45 @SergeyBiryukov
4 years ago

In 49591:

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

Follow-up to [49587].

Props fierevere.
See #51742.

#46 @SergeyBiryukov
4 years ago

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.

#47 in reply to: ↑ 42 @SergeyBiryukov
4 years ago

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.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

@audrasjb
4 years ago

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

#48 @audrasjb
4 years ago

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>

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


4 years ago

@audrasjb
4 years ago

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
4 years ago

Before 51742.12.diff on development versions of WordPress

@audrasjb
4 years ago

After 51742.12.diff on development versions of WordPress

@audrasjb
4 years ago

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

#50 @audrasjb
4 years ago

  • Keywords commit added

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.

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


4 years ago

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


4 years ago

@pbiron
4 years ago

#53 @pbiron
4 years ago

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.

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


4 years ago

@helen
4 years ago

@helen
4 years ago

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

@helen
4 years ago

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

@helen
4 years ago

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

@helen
4 years ago

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

#56 @helen
4 years ago

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 :)

#57 @audrasjb
4 years ago

51742.15.diff is testing good on my side 👍

#58 @helen
4 years ago

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

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.

Last edited 4 years ago by helen (previous) (diff)
Note: See TracTickets for help on using tickets.