WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 4 days ago

#49577 closed defect (bug) (fixed)

Site Health Status Dashboard provides incorrect items count on initial load

Reported by: garrett-eclipse Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: trunk
Component: Site Health Keywords: commit has-patch fixed-major dev-reviewed
Focuses: ui Cc:

Description

Hello,

On initial install of trunk or 5.4-RC1 I login to find 6 items mentioned on the Site Health Status Dashboard widget. Navigating to the Site Health Status I only find 3 items. If I wait for the Site Health Status page to finish working and return to the dashboard I now see only 3 items mentioned.

Cheers

Attachments (4)

Screen Shot 2020-03-03 at 2.18.34 PM.png (18.4 KB) - added by garrett-eclipse 3 weeks ago.
Initially I see 6 items mentioned in the Dashboard widget
Screen Shot 2020-03-03 at 2.29.06 PM.png (45.3 KB) - added by garrett-eclipse 3 weeks ago.
Upon navigating to the Site Health Status screen and letting it execute I find only 3 issues listed.
49577.patch (748 bytes) - added by Clorith 7 days ago.
Screen Shot 2020-03-14 at 1.31.08 PM.png (50.0 KB) - added by garrett-eclipse 7 days ago.
No information yet... on initial install using trunk.

Download all attachments as: .zip

Change History (25)

@garrett-eclipse
3 weeks ago

Initially I see 6 items mentioned in the Dashboard widget

@garrett-eclipse
3 weeks ago

Upon navigating to the Site Health Status screen and letting it execute I find only 3 issues listed.

#1 @garrett-eclipse
3 weeks ago

This dashboard widget was introduced in #47606

#2 @SergeyBiryukov
3 weeks ago

  • Milestone changed from Awaiting Review to 5.4

#3 @Clorith
12 days ago

Hmmm, I'm not sure how we could reliably prevent this number form being sometimes off a little.

So the dashboard widget works off a cached result from when it last ran, that means the issues may have gone up or down since then as site settings change etc, that's why it re-runs whenever you visit the page.

I'm very open to thoughts here on how we could better this scenario though.

#4 @garrett-eclipse
12 days ago

Thanks @Clorith it's interesting on a clean install the count starts higher. There should be no cache at that point correct? On loading the dashboard with no cached item could all the tests be run as though you're on the site-health page. And then rely on cache from that point forward?

#5 @Clorith
12 days ago

If there is no data, you should be seeing a message stating there is no data yet, and letting you know that you can either visit the site health page to populate it, or wait for it to run automatically in a week.

Are you able to replicate this with every new instance you make (I wasn't able to replicate it showing passed tests like this for my self, but I only tried once) ?

I'm wondering if there's something going on with a not-fully-setup site running the cron too early, thus getting partial errors, which are then resolved the next time the tests run, but for that to be the case it would have to be replicable in some way.

#6 @garrett-eclipse
12 days ago

Yes, just installed fresh local MAMP install and see the 6 items upon logging in, go into the actual page let it load and get 4, return to dashboard to see it's updated to 4 items.

My install steps;

  1. Create Folder - mkdir 49577-site-health-dashboard; cd 49577-site-health-dashboard
  2. Install trunk - svn co https://develop.svn.wordpress.org/trunk .
  3. Install dependencies - npm install
  4. Build - grunt build
  5. Map to MAMP dir - ln -s /path/to/install/49577-site-health-dashboard/build /Applications/MAMP/htdocs/49577
  6. Create DB in PHPMyAdmin
  7. Run install on localhost:8888/49577
  8. Login to Dashboard - See a count of 6 items.
  9. Navigate to Site Health screen - Wait for it to run and get 4 items;
  • You should remove inactive plugins
  • You should remove inactive themes
  • Your site does not use HTTPS
  • Background updates may not be working properly
  1. Return to Dashboard - See count is updated to show 4 items.

Let me know if you're not able to replicate and I'll setup a clean install accessible from online for you to test.

This ticket was mentioned in Slack in #core-site-health by garrett-eclipse. View the logs.


12 days ago

#8 @Clorith
12 days ago

Hmm, is this single or multisite, I wonder if that's what differs, as I've only had my test ran on a multisite, I'll look into it and see what I can come up with, thanks for the input!

#9 @garrett-eclipse
12 days ago

Thanks @Clorith to confirm this is a fresh single site install on local MAMP using trunk.

#10 @roytanck
10 days ago

As a test, I just installed a fresh copy of RC2 locally (XAMPP), and added some error_log statements to the dashboard widget before my first visit. It appears that the health-check-site-status-result transient has been set by the time the dashboard widget is first shown.

In my case it contained 10 good tests, 7 recommended and 0 critical. Once the tests ran, that changed to {"good":"9","recommended":"5","critical":"2","0":"NaN"}. Not sure what the NaN is about, but different numbers.

Could it be that the tests are run during install, and this is somehow premature?

If I remove the transient from the database, the "no information" message is shown, as expected.

Last edited 10 days ago by roytanck (previous) (diff)

@Clorith
7 days ago

#11 @Clorith
7 days ago

  • Keywords commit has-patch added

You are absolutely right @roytanck, it is running too early in first time setup scenarios, causing the results to be a bit unreliable.

49577.patch changes the time in which the tests run for the first time to be one week after the creation of the cron job.

I considered setting it to a day, but realistically, if you are tinkering on your own time for example, you may not be done setting things up, as you only had an hour, and plan to continue during the weekend for example.

By setting it to a week, it follows the pattern of how often it will run in the first place, yet giving the user ample time to do what they want with their site.

The data can still be populated by visiting the Site Health section though.

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


7 days ago

#13 @Clorith
7 days ago

Sorry, I misspoke, I set the first test to run after a day, because then the basic setup would be done, and then it'll keep running a week later for any remaining changes you may wish to do to your site, this way it's not "mid setup" when it tries to do the first checks :)

#14 @roytanck
7 days ago

@Clorith I actually had almost the same fix in my local install :). A day seems long enough for me.

I couldn't really find a more elegant/reliable way of delaying the cron job, so this seems like a good fix to me.

#15 @joostdevalk
7 days ago

Patch LGTM

#16 @SergeyBiryukov
7 days ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 47456:

Site Health: Run the first scheduled site health check a day after the initial site setup.

This reduces the chance of displaying incorrect results due to running the check too early in first time setup scenarios.

Props Clorith, garrett-eclipse, roytanck, joostdevalk.
Fixes #49577.

#17 @SergeyBiryukov
7 days ago

  • Keywords fixed-major dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.4 branch after a second committer's review.

@garrett-eclipse
7 days ago

No information yet... on initial install using trunk.

#18 @garrett-eclipse
7 days ago

Thanks everyone, testing trunk now I see the 'No information yet...' as seen in the uploaded screenshot.

This works nicely and would be great to also apply to 5.4 after the second committer review.

Appreciate everyone investigating/fixing.

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


5 days ago

#20 @whyisjake
5 days ago

  • Keywords dev-reviewed added; dev-feedback removed

LGTM too, :shipit:

#21 @SergeyBiryukov
4 days ago

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

In 47466:

Site Health: Run the first scheduled site health check a day after the initial site setup.

This reduces the chance of displaying incorrect results due to running the check too early in first time setup scenarios.

Props Clorith, garrett-eclipse, roytanck, joostdevalk.
Reviewed by whyisjake, SergeyBiryukov.
Merges [47456] to the 5.4 branch.
Fixes #49577.

Note: See TracTickets for help on using tickets.