When you don't have enough data, and switch to 7 days and then back to larger data it shows an error that the data isn't available

bjackson27 commented 5 years ago

Noticed this on my site. I don't have any data from the last 7 days so when I switch to the last 7 days it rightly shows and error. When I switch back to 14, 28 or 90 days the error still displays.

Steps to recreate: 1) View dashboard (or module page) as normal 2) Switch to 7 day view - receive gathering stats message 3) Switch to 14, 28 or 90 day view - receive gathering stats message above the graph

Currently I get:

I expect to get:

Currently I get:

I expect

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

"Gathering data" message should only be displayed for views where there's no data returned from the API.

Implementation Brief

Set zeroData in state any time new data is loaded.
Merge #1876

Changelog entry

Fix bug where state of not having sufficient data for a Site Kit widget would sometimes incorrectly persist when switching the date range.

QA Brief

Test that switching between an empty view and a populated view does not display the error as pictured in https://github.com/google/site-kit-wp/issues/184#issuecomment-668780740
This state can be hard to reproduce; if you want a copy of cached sessionStorage data to test it, ping me and I can send you some privately. The data I have been testing with needs to be anonymised so I don't want to post it here 😄

felixarntz commented 5 years ago

Related: #447

Potentially this will already fix it.

ThierryA commented 4 years ago

Moving this to IB as it doesn't have an estimate

felixarntz commented 4 years ago

Let's check if this is still relevant after the latest changes to the date range selector. I prioritized this for the upcoming release since we should fix it if still a problem, but it could be that it already got fixed by now - in that case this should be closed.

tofumatt commented 4 years ago

This does still happen with the new datepicker, at least on legacy pages that are using withData:

2020-08-04 20 24 03

tofumatt commented 4 years ago

Just posting here to document what I'm finding, as this wasn't very intuitive to me. It looks like this setState call:

https://github.com/google/site-kit-wp/blob/897c92e479c3577ceceb3355b8c58c5c7287c4e3/assets/js/components/higherorder/withdata.js#L196

Isn't properly "reset" when the date range changes, meaning the empty "Last 7 Days" request sets a zeroData state in the component, which isn't reset when switching back to "Last 28 Days". Resetting that when the date range changes should fix things, just need to figure out where to do that appropriately here.

tofumatt commented 4 years ago

This one was much tougher to track down than implement, so PR is ready and the IB is set to review. Turns out: it was still happening! 😄