Make WordPress Core

Opened 2 months ago

Last modified 2 weeks ago

#57999 assigned defect (bug)

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

Reported by: presskopp's profile Presskopp Owned by: pbiron's profile pbiron
Milestone: 6.3 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: good-first-bug has-patch needs-testing has-screenshots has-testing-info
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 2 months ago.
57999-notice.diff (1.1 KB) - added by sabernhardt 2 months ago.
Adding a notice instead of an error if there are no updates
57999.1.diff (955 bytes) - added by pbiron 6 weeks ago.
themes.png (67.7 KB) - added by Presskopp 4 weeks ago.
plugins.png (65.6 KB) - added by Presskopp 4 weeks ago.
plugins-with-57999-1.png (48.0 KB) - added by pbiron 4 weeks ago.
plugins-after-57999-1.diff has been applied
plugins-with-57999-notice.png (46.7 KB) - added by pbiron 4 weeks ago.
plugins after 57999-notice.diff has been applied
themes-with-57999-1.png (44.6 KB) - added by pbiron 4 weeks ago.
themes after 57999-1.diff has been applied
themes-with-57999-notice.png (46.6 KB) - added by pbiron 4 weeks ago.
themes after 57999-notice.diff has been applied
Plugins Before Applying The Patch.png (162.9 KB) - added by zunaid321 3 weeks ago.
Updates Page Before Applying Patch
Updates Page After Applying Patch.png (216.9 KB) - added by zunaid321 3 weeks ago.
Updates Page After Applying Patch

Download all attachments as: .zip

Change History (37)

@Presskopp
2 months ago

#1 @Presskopp
2 months ago

  • Keywords has-patch added

@sabernhardt
2 months ago

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

#2 @sabernhardt
2 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
2 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
2 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
2 months ago

  • Keywords good-first-bug added

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


6 weeks ago

#7 @pbiron
6 weeks ago

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

@pbiron
6 weeks ago

#8 @pbiron
6 weeks 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
6 weeks 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 6 weeks ago by pbiron (previous) (diff)

#10 @pbiron
4 weeks ago

  • Keywords needs-testing added

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


4 weeks ago

#12 @pbiron
4 weeks ago

  • Keywords needs-design-feedback added

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


4 weeks ago

@Presskopp
4 weeks ago

@Presskopp
4 weeks ago

#14 @Presskopp
4 weeks ago

  • Keywords has-screenshots added

@pbiron
4 weeks ago

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

@pbiron
4 weeks ago

plugins after 57999-notice.diff has been applied

@pbiron
4 weeks ago

themes after 57999-1.diff has been applied

@pbiron
4 weeks ago

themes after 57999-notice.diff has been applied

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


4 weeks ago

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


4 weeks ago

#17 @afragen
4 weeks 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.


3 weeks ago

#19 @pbiron
3 weeks ago

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

#20 @pbiron
3 weeks 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.


3 weeks ago

#22 @zunaid321
3 weeks 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 3 weeks ago by zunaid321 (previous) (diff)

@zunaid321
3 weeks ago

Updates Page Before Applying Patch

@zunaid321
3 weeks ago

Updates Page After Applying Patch

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


3 weeks ago

#24 @shuvoaftab
3 weeks 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

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

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


3 weeks ago

#26 @ugyensupport
2 weeks 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 2 weeks ago by ugyensupport (previous) (diff)
Note: See TracTickets for help on using tickets.