Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#49577 closed defect (bug) (fixed)

Site Health Status Dashboard provides incorrect items count on initial load

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.4
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 5 years 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 5 years 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 5 years ago.
Screen Shot 2020-03-14 at 1.31.08 PM.png (50.0 KB) - added by garrett-eclipse 5 years ago.
No information yet... on initial install using trunk.

Download all attachments as: .zip

Change History (25)

@garrett-eclipse
5 years ago

Initially I see 6 items mentioned in the Dashboard widget

@garrett-eclipse
5 years ago

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

#1 @garrett-eclipse
5 years ago

This dashboard widget was introduced in #47606

#2 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.4

#3 @Clorith
5 years 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
5 years 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
5 years 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
5 years 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.


5 years ago

#8 @Clorith
5 years 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
5 years ago

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

#10 @roytanck
5 years 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 5 years ago by roytanck (previous) (diff)

@Clorith
5 years ago

#11 @Clorith
5 years 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.


5 years ago

#13 @Clorith
5 years 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
5 years 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
5 years ago

Patch LGTM

#16 @SergeyBiryukov
5 years 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
5 years 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
5 years ago

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

#18 @garrett-eclipse
5 years 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 years ago

#20 @whyisjake
5 years ago

  • Keywords dev-reviewed added; dev-feedback removed

LGTM too, :shipit:

#21 @SergeyBiryukov
5 years 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.