Make WordPress Core

Opened 19 months ago

Closed 15 months ago

Last modified 15 months ago

#57999 closed defect (bug) (fixed)

Don't show error message when there is nothing to update

Reported by: presskopp's profile Presskopp Owned by: audrasjb's profile audrasjb
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: good-first-bug has-patch has-screenshots has-testing-info commit
Focuses: administration, ui-copy Cc:

Description

If either

wp-admin/update-core.php?action=do-plugin-upgrade

or

wp-admin/update-core.php?action=do-theme-upgrade

is called directly and if there is nothing to be updated, still an error is shown:

Please select one or more plugins to update.

or

Please select one or more themes to update.

Let's not show the error anymore.

Attachments (11)

57999.diff (787 bytes) - added by Presskopp 19 months ago.
57999-notice.diff (1.1 KB) - added by sabernhardt 19 months ago.
Adding a notice instead of an error if there are no updates
57999.1.diff (955 bytes) - added by pbiron 18 months ago.
themes.png (67.7 KB) - added by Presskopp 17 months ago.
plugins.png (65.6 KB) - added by Presskopp 17 months ago.
plugins-with-57999-1.png (48.0 KB) - added by pbiron 17 months ago.
plugins-after-57999-1.diff has been applied
plugins-with-57999-notice.png (46.7 KB) - added by pbiron 17 months ago.
plugins after 57999-notice.diff has been applied
themes-with-57999-1.png (44.6 KB) - added by pbiron 17 months ago.
themes after 57999-1.diff has been applied
themes-with-57999-notice.png (46.6 KB) - added by pbiron 17 months ago.
themes after 57999-notice.diff has been applied
Plugins Before Applying The Patch.png (162.9 KB) - added by zunaid321 17 months ago.
Updates Page Before Applying Patch
Updates Page After Applying Patch.png (216.9 KB) - added by zunaid321 17 months ago.
Updates Page After Applying Patch

Download all attachments as: .zip

Change History (44)

@Presskopp
19 months ago

#1 @Presskopp
19 months ago

  • Keywords has-patch added

@sabernhardt
19 months ago

Adding a notice instead of an error if there are no updates

#2 @sabernhardt
19 months ago

  • Component changed from Administration to Upgrade/Install
  • Focuses administration ui-copy added

The error message is inappropriate when there are no updates to select, though I think it might be worth adding an info notice in this case. I used existing text strings.

#3 @Presskopp
19 months ago

FYI: I decided to not put any notice in, because down on the page it already tells us that there are no updates. :-)

#4 @sabernhardt
19 months ago

It might help to know the ways to get the theme/plugin update action for that page other than typing or pasting it in the address bar.

I know of one other way, for which the notice would not help much:

  1. Go to the Updates page.
  2. Select all plugins or all themes and click the button to update them.
  3. Wait for the updates to complete.
  4. Navigate to another page.
  5. Use the browser's Back button and refresh the page.

#5 @Presskopp
18 months ago

  • Keywords good-first-bug added

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


18 months ago

#7 @pbiron
18 months ago

  • Milestone changed from Awaiting Review to 6.3
  • Owner set to pbiron
  • Status changed from new to assigned

@pbiron
18 months ago

#8 @pbiron
18 months ago

57999.1.diff is a slight mod of 57999.diff. The difference prevents a stray </p></div> from being output when there are no updates available.

#9 @pbiron
18 months ago

One downside of the approach in 57999.1.diff and 57999.diff is they can result in get_{theme|plugin}_updates() being called twice on the same page load of update-core.php (this was pointed out to me by @costdev.

I've looked for a way to prevent those multiple calls, but as far as I can tell, that would take a major refactoring of the flow in update-core.php and I don't think that's worth it. If someone else thinks they know an easy to do so, then I'm all ears.

Last edited 18 months ago by pbiron (previous) (diff)

#10 @pbiron
17 months ago

  • Keywords needs-testing added

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


17 months ago

#12 @pbiron
17 months ago

  • Keywords needs-design-feedback added

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


17 months ago

@Presskopp
17 months ago

@Presskopp
17 months ago

#14 @Presskopp
17 months ago

  • Keywords has-screenshots added

@pbiron
17 months ago

plugins-after-57999-1.diff has been applied

@pbiron
17 months ago

plugins after 57999-notice.diff has been applied

@pbiron
17 months ago

themes after 57999-1.diff has been applied

@pbiron
17 months ago

themes after 57999-notice.diff has been applied

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


17 months ago

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


17 months ago

#17 @afragen
17 months ago

I’m not sure the notice really adds anything except to emphasize that something didn’t happen.

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


17 months ago

#19 @pbiron
17 months ago

  • Keywords has-testing-info added; needs-design-feedback removed

#20 @pbiron
17 months ago

We discussed this ticket during the component meeting earlier today.

Based on feedback we received from the #design team, the consensus of those at the meeting is that:

  1. yes, the current error notice should be removed (and not replaced with an info notice)
  2. we still need to evaluate whether the slight performance hit of calling get_{plugin|theme_updates() twice as in 57999.1.diff is a problem or not (see #comment:9). One possible way to reduce the performance hit was discussed during the meeting, but it would be out of scope for this ticket (but certainly a good candidate for another ticket)

To summarize, it's looking good that we'll be able to commit this for 6.3.

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


17 months ago

#22 @zunaid321
17 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/57999/57999.1.diff

Environment

  • OS: Windows 11 (22H2)
  • Web Server: nginx/1.23.4
  • PHP: 7.4.33
  • WordPress: 6.3-alpha-55505-src
  • Browser: Chrome Version 113.0.5672.126 (Official Build) (64-bit)
  • Theme: Twenty Twenty-Three

Before Applying The Patch

  • Followed the instructions of @sabernhardt
  • ✅ Error message does show up after updating the plugins

After Applying The Patch

  • Followed the same instructions
  • ✅ Error message does not show up after updating the plugins

Plugins Used

  • wordpress-seo.20.4
  • all-in-one-wp-migration.7.70

Special thanks to @costdev for helping me out regarding applying patches!

Last edited 17 months ago by zunaid321 (previous) (diff)

@zunaid321
17 months ago

Updates Page Before Applying Patch

@zunaid321
17 months ago

Updates Page After Applying Patch

This ticket was mentioned in Slack in #core-test by zunaid321. View the logs.


17 months ago

#24 @shuvoaftab
17 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/57999/57999.1.diff

Environment

  • OS: macOS 12.3.1
  • WordPress: 6.3-alpha-55828
  • PHP: 8.1.1
  • Server: Apache/2.4.46 (Unix) mod_fastcgi/mod_fastcgi-SNAP-0910052141 OpenSSL/1.0.2u mod_wsgi/3.5 Python/2.7.13
  • Database: mysqli (Server: 5.7.34 / Client: mysqlnd 8.1.1)
  • Browser: Edge 107.0.1418.28 (macOS)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins:
    • WordPress Beta Tester 3.3.8

Actual Results

  • ✅ Issue resolved with the patch.

Additional Notes

  • Tested with All Plugins and themes updated and then again a theme with an older version and it also validates the warning to update the older version theme

Supplemental Artifacts

Plugins Upgrade Before Patch: https://prnt.sc/jr9kMGCj9Yb4
Plugins Upgrade After Patch: https://prnt.sc/kZR7qcoBdUDp

Themes Upgrade Before Patch: https://prnt.sc/1ZFEgHeN8MyP
Themes Upgrade After Patch: https://prnt.sc/Lv_xYox3tDUV

Themes Upgrade After Patch with one theme to be updated: https://prnt.sc/A57XQvpkFvmQ

Version 0, edited 17 months ago by shuvoaftab (next)

This ticket was mentioned in Slack in #core-test by shuvoaftab. View the logs.


17 months ago

#26 @ugyensupport
17 months ago

Bug Report

Description

Don't show error message when there is nothing to update

Environment

  • WordPress: 6.3-alpha-55848
  • PHP: 8.0.0
  • Server: Apache/2.4.10 (Debian)
  • Database: mysqli (Server: 5.5.59-MariaDB-1~wheezy / Client: 5.5.62)
  • Browser: Chrome 113.0.0.0 (macOS)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins:
    • Akismet Anti-Spam: Spam Protection 5.1
    • All-in-One WP Migration 7.74-7-g3a9c5abd
    • All-in-One WP Migration S3 Extension 3.75-2-g1815870
    • WordPress Beta Tester 3.4.0

Steps to Reproduce

  1. If either wp-admin/update-core.php?action=do-plugin-upgrade or wp-admin/update-core.php?action=do-theme-upgrade is called directly and if there is nothing to be updated, still an error is shown: Please select one or more plugins to update. or Please select one or more themes to update.
  2. 🐞 Bug occurs.

Expected Results

  1. ✅ Issue resolved with the patch ;

Patch tested: https://core.trac.wordpress.org/attachment/ticket/57999/57999.1.diff

Actual Results

  1. ❌ Should add https://core.trac.wordpress.org/attachment/ticket/57999/57999.1.diff to the upcoming WordPress Version to work smoothly.

https://ibb.co/xh9156R

Last edited 17 months ago by ugyensupport (previous) (diff)

This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.


16 months ago

#28 @sabernhardt
16 months ago

The patch has a space before the semicolon following both get_theme_updates() and get_plugin_updates(). Those spaces probably could be removed while during the commit process.

This ticket was mentioned in PR #4743 on WordPress/wordpress-develop by @costdev.


16 months ago
#29

Previously, when the do-plugin-upgrade or do-theme-upgrade actions were accessed directly on update-core.php, an error message stating "Select one or more (plugins|themes) to update" would be shown even if there was nothing to update.

This ensures that the error message only appears when there is something to update.

#30 @costdev
16 months ago

  • Keywords commit added; needs-testing removed

PR 4743 updates the previous patch to remove some errant spaces before semi-colons and verifies that no related CI job fails.

With two successful test reports, I'm removing needs-testing and adding for commit consideration.

#31 @audrasjb
15 months ago

  • Owner changed from pbiron to audrasjb
  • Status changed from assigned to reviewing

#32 @audrasjb
15 months ago

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

In 56107:

Upgrade/Install: Only show errors if there is nothing to update.

Previously, when the do-plugin-upgrade or do-theme-upgrade actions were accessed directly on update-core.php, an error message stating "Select one or
more (plugins|themes) to update" would be shown even if there was nothing to update.

This ensures that the error message only appears when there is something to update.

Props Presskopp, sabernhardt, pbiron, afragen, zunaid321, shuvoaftab, ugyensupport, costdev.
Fixes #57999.

Note: See TracTickets for help on using tickets.