google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 290 forks source link

Pass actual dates in API calls in Analytics widgets #2236

Closed felixarntz closed 3 years ago

felixarntz commented 4 years ago

The Analytics dashboard and pageDashboard widgets should be migrated to pass startDate and endDate instead of dateRange (as well as compareStartDate and compareEndDate where applicable) with their API requests.


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

Acceptance criteria

Implementation Brief

Update args passed to getReport selector

DashboardAllTrafficWidget, DashboardPopularPagesWidget

DashboardBounceRateWidget, DashboardGoalsWidget

DashboardUniqueVisitorsWidget

Test Coverage

Visual Regression Changes

QA Brief

Changelog entry

felixarntz commented 3 years ago

@ryanwelcher The IB is on the right track, it's missing some arguments though which are necessary to pass to getDateRangeDates in order to maintain the current behavior (see how the Analytics PHP class calls parse_date_range for comparison):

The only other issue is that for assets/js/modules/analytics/components/dashboard/DashboardUniqueVisitorsWidget.js the described changes are slightly inaccurate, as there are two different requests made there which require different modifications.

ryanwelcher commented 3 years ago

IB Updated.

felixarntz commented 3 years ago

@ryanwelcher

  • In assets/js/modules/analytics/components/dashboard/DashboardUniqueVisitorsWidget.js
  • Remove dateRange from the commonArgs array.
  • Spread the results of select( CORE_USER ).getDateRangeDates({ offsetDays: 1, compare: true, weekdayAlign:true }) into the commonArgs array.

This is still not fully correct: When you look at the current logic, you'll see that sparklineArgs uses just the regular dateRange, and only args uses dateRange and multiDateRange. In other words, I think commonArgs should now only be used for the (potentially existing) url argument. Then, we need to spread the results from two different getDateRangeDates calls into sparklineArgs and args respectively - one not passing compare: true, the other passing it.

aaemnnosttv commented 3 years ago

@felixarntz this one is ready for another pass. I bumped the estimate a little to allow for more time in QA to make sure the requests are resulting in the same data on the front.

felixarntz commented 3 years ago

IB ✅

felixarntz commented 3 years ago

@benbowler Leaving a note here that we'll need to also make Storybook changes here so that the stories for those Analytics widgets still pass after the production code changes.

benbowler commented 3 years ago

Added QA Brief and marked ready for code review.

wpdarren commented 3 years ago

@aaemnnosttv @cole10up I'm sorry, but really struggling with this ticket. Feel like I am wasting time going through the console within XHR, trying to look for API data. Please could you give me steps to find what I need to check and compare.

Here's what I am seeing in console.

image

cole10up commented 3 years ago

@wpdarren - Checking on my end, I'm in the same boat. Looking for a bit more context on this one as well. I'm seeing the widget items when I search through the API console. But unsure what specifically I should be seeing.

image

wpdarren commented 3 years ago

QA updates: ⚠️ Engineers to confirm

@benbowler @aaemnnosttv I have two observations:

  1. DashboardBounceRateWidget I am unable to find the widget on the SK dashboard. This is for current or legacy. I can see them on the individual storyboard but not on my test site with development build. Should it be appearing? I can ee the bounce rate is actually on the Analytics page.
  2. Under Search funnel for Unique Visitors. The top widget title is 'Unique Visitors' but the bottom widget is 'Unique Visitors from Search' should these be the same? Screenshots: Top and Bottom

Verified:

  1. DashboardAllTrafficWidget
  2. DashboardPopularPagesWidget
  3. DashboardGoalsWidget
  4. DashboardUniqueVisitorsWidget
tofumatt commented 3 years ago
  1. For the DashboardBounceRateWidget, I see this widget on the details page for a particular URL:

2021-01-14 16 38 37

  1. I think those are the same (the new widget has a slightly condensed header is all), but the data in the charts definitely looks different, which seems wrong.

@benbowler: Can you take a look at @wpdarren's second point from the comment above?

benbowler commented 3 years ago

Hey @wpdarren , to answer your questions:

DashboardBounceRateWidget I am unable to find the widget on the SK dashboard. This is for current or legacy. I can see them on the individual storyboard but not on my test site with development build. Should it be appearing? I can ee the bounce rate is actually on the Analytics page.

The bounce rate widget is exclusively used on the stats breakdown for individual pages, not on any general dashboards. To see a stats breakdown for an individual page you can do the following:

  1. Create a new page with the slug "Features" in your local WordPress setup (This is assuming your set up to view elasticpress.com analytics data - you can in fact create any page that has data on the live site to view this breakdown screen).
  2. Then navigate to the page and click on the "More Details" link in the admin bar:
Screenshot 2021-01-14 at 16 26 21

That should take you to the breakdown of stats for the features page which includes the Bounce Rate widget.

Screenshot 2021-01-14 at 16 28 47

Under Search funnel for Unique Visitors. The top widget title is 'Unique Visitors' but the bottom widget is 'Unique Visitors from Search' should these be the same? Screenshots: Top and Bottom

The widget entitled 'Unique Visitors from Search' is a separate legacy widget called LegacyAnalyticsDashboardWidgetTopLevel, the DashboardUniqueVisitorsWidgethas the title Unique Visitors so this is the one to check between the release and new version:

Screenshot 2021-01-14 at 16 38 56
benbowler commented 3 years ago

@wpdarren As an aside I find the Chrome React developer tools extension useful for tracking down components I should be working on:

2021-01-14 16-43-31 2021-01-14 16_50_22

wpdarren commented 3 years ago

QA Update: Pass ✅

Verified:

  1. DashboardAllTrafficWidget
  2. DashboardPopularPagesWidget
  3. DashboardGoalsWidget
  4. DashboardBounceRateWidget
  5. DashboardUniqueVisitorsWidget