Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot read property 'clicks' of undefined error on Search Console widget-based dashboard #3348

Closed
aaemnnosttv opened this issue May 13, 2021 · 4 comments
Labels
Module: Search Console Search Console module related issues P0 High priority Type: Bug Something isn't working
Milestone

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented May 13, 2021

Bug Description

When viewing the Search Console dashboard, it is possible to encounter the following fatal error:

Cannot read property 'clicks' of undefined

    in Stats (created by ModuleOverviewWidget)
    in div (created by Widget)
    in div (created by Widget)
    in Widget (created by WithWidgetSlug(Widget))
    in WithWidgetSlug(Widget) (created by ModuleOverviewWidget)
    in ModuleOverviewWidget (created by WidgetRenderer)
    in WidgetRenderer (created by WidgetAreaRenderer)
    in div (created by Cell)
    in Cell (created by WidgetCellWrapper)
    in WidgetCellWrapper (created by WidgetAreaRenderer)
    in div (created by Row)
    in Row (created by WidgetAreaRenderer)
    in div (created by WidgetAreaRenderer)
    in div (created by Grid)
    in Grid (created by WidgetAreaRenderer)
    in WidgetAreaRenderer (created by WidgetContextRenderer)
    in div (created by WidgetContextRenderer)
    in WidgetContextRenderer (created by ModuleApp)
    in ModuleApp (created by GoogleSitekitModule)
    in GoogleSitekitModule
    in RestoreSnapshots (created by Root)
    in ErrorHandler (created by Root)
    in Root

The problem happens when the API returns an odd number of rows for a request that includes the previous range for comparison. This leads to an off-by-one situation when iterating over the previous data due to the way the rows are sliced.

// Split the data in two chunks.
const half = Math.floor( data.length / 2 );
const latestData = data.slice( half );
const olderData = data.slice( 0, half );
const googleChartData = getSiteStatsDataForGoogleChart(
latestData,
olderData,

The error happens right here because the index in the array of previous rows does not exist (because it was not returned from the API:

const prevMonth = previous[ index ][ selectedColumn ];

This is likely a matter of timing regarding when data is available as it is the very last (most recent) row which is missing
image

This request was for a 28 day range + compare which should have returned 56 rows (28 * 2), but instead it returned 55 (54 + 1 for the 0th row).

image

Steps to reproduce

  1. Visit Search Console dashboard with empty cache
    • This depends on timing related to data availability from the API where the most recent day's data is not yet available

Additional Context

  • Introduced in 1.30.0

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

Acceptance criteria

  • Search Console report data consumers should not assume an even number of rows when handling report data that includes the comparison range
  • Values referenced in a missing row should use a zero value as a fallback

Implementation Brief

Test Coverage

  • Add tests for partitionReport

Visual Regression Changes

  • There should be none, but possible if we a reference image based on a fixture that was based on an incomplete report
  • Not likely though since this only seemed to affect the widget-based dashboard which does not have VRT coverage yet

QA Brief

  • All dashboards and widgets that display stats/charts with comparison data (i.e. previous range plot or change percent) should be checked to ensure no errors are present and things look as expected
  • There was one visual change in the Analytics audience overview chart which resulted in slightly different data shown in the chart
  • Ensure the above steps to reproduce do not result in an error
  • Widget-based and legacy views/dashboards should be checked including different date ranges

Changelog entry

  • Fix potential Cannot read property 'clicks' of undefined JS error that could occur when an uneven number of Search Console report rows was returned.
@aaemnnosttv aaemnnosttv added Type: Bug Something isn't working P0 High priority Module: Search Console Search Console module related issues labels May 13, 2021
@aaemnnosttv aaemnnosttv self-assigned this May 13, 2021
@aaemnnosttv aaemnnosttv changed the title Cannot read property 'clicks' of undefined error on Search Console dashboard Cannot read property 'clicks' of undefined error on Search Console widget-based dashboard May 13, 2021
@felixarntz
Copy link
Member

IB ✅

@aaemnnosttv @eclarke1 @fhollis Let's squeeze this into the current sprint and upcoming release. A PR is already underway so this should be doable. If needed maybe we can postpone #3272 for the next sprint?

@felixarntz felixarntz added this to the Sprint 49 milestone May 13, 2021
@felixarntz felixarntz assigned aaemnnosttv and unassigned felixarntz May 13, 2021
@felixarntz felixarntz assigned aaemnnosttv and unassigned felixarntz May 14, 2021
@wpdarren
Copy link
Collaborator

QA Update: Pass ✅

As per conversation with @aaemnnosttv QA have run through a first pass for this ticket.

  • All dashboards and widgets that display stats/charts with comparison data have been checked.
  • On widgets with tabs for different data, i.e. Audience overview, the comparison data/graphs were checked.

Screenshot 1 - Screenshot 2 - Screenshot 3 - Screenshot 4

This testing was done on widget-based and legacy views/dashboards including different date ranges.

@felixarntz felixarntz assigned aaemnnosttv and unassigned felixarntz May 19, 2021
@aaemnnosttv
Copy link
Collaborator Author

Thanks @wpdarren ! @felixarntz I asked @wpdarren to give this a first pass earlier in the week if they had capacity since we had some extra time while you were away. Given the timeliness around the release and the larger footprint of the PR I thought we could perhaps iterate on any regressions while we were waiting for the next round of CR. Luckily none were found 😄

I think the only changes to the branch were additions to the test since Darren tested so this is probably good to go right to approval but I'll send it into QA just in case there is anything left to do.

@aaemnnosttv aaemnnosttv assigned wpdarren and unassigned aaemnnosttv May 19, 2021
@felixarntz
Copy link
Member

@aaemnnosttv Sounds good, let's bypass another round of QA then. cc @wpdarren

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Search Console Search Console module related issues P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants