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

Dashboard crashes when report doesn't contain rows #3983

Closed
eugene-manuilov opened this issue Sep 2, 2021 · 6 comments
Closed

Dashboard crashes when report doesn't contain rows #3983

eugene-manuilov opened this issue Sep 2, 2021 · 6 comments
Labels
Good First Issue Good first issue for new engineers Module: Analytics Google Analytics module related issues P0 High priority Type: Bug Something isn't working
Milestone

Comments

@eugene-manuilov
Copy link
Collaborator

eugene-manuilov commented Sep 2, 2021

Bug Description

During testing the plugin on my local machine, I encountered a situation when Google Analytics API returns a report with an empty rows property. I suspect it happens because the selected Analytics account has no data, which is fine. What is not fine is that the DashboardSearchVisitorsWidget widget tries to access the report data rows length without checking whether rows is an actual array:

const dataRows = sparkData[ 0 ].data.rows;
// Loop the rows to build the chart data.
for ( let i = 0; i < dataRows.length; i++ ) {
const { values } = dataRows[ i ].metrics[ 0 ];
const dateString = dataRows[ i ].dimensions[ 0 ];
const date = parseDimensionStringToDate( dateString );
sparkLineData.push( [ date, values[ 0 ] ] );
}

This leads to fatal errors and the whole dashboard crashes. We need to make sure that widgets display zero states if there are no rows in the report data.

Additional details:

Report arguments
metrics[0][expression]: ga:users
metrics[0][alias]: Users
dimensions[0][name]: ga:date
dimensions[1][name]: ga:channelGrouping
startDate: 2021-08-05
endDate: 2021-09-01
dimensionFilters[ga:channelGrouping]: Organic Search
Report response
[
  {
    "nextPageToken": null,
    "columnHeader": {
      "dimensions": [
        "ga:date",
        "ga:channelGrouping"
      ],
      "metricHeader": {
        "metricHeaderEntries": [
          {
            "name": "Users",
            "type": "INTEGER"
          }
        ]
      }
    },
    "data": {
      "dataLastRefreshed": null,
      "isDataGolden": null,
      "rowCount": null,
      "samplesReadCounts": null,
      "samplingSpaceSizes": null,
      "totals": [
        {
          "values": [
            "0"
          ]
        }
      ]
    }
  }
]

Screenshots

2021-09-02_19-07


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

Acceptance criteria

  • Dashboard widgets should not raise fatal errors if zero rows are returned from the API
    • This is possible with Analytics even when includeEmptyRows is enabled for a request.

Implementation Brief

  • Inside assets/js/modules/analytics/components/dashboard/DashboardSearchVisitorsWidget.js, default the dataRows assignment to an empty array if the sparkData array is empty, for example by using optional chaining:
  const dataRows = sparkData?.[ 0 ]?.data?.rows || [];
  • Inside site-kit-wp/assets/js/modules/analytics/components/dashboard/DashboardGoalsWidget.js, repeat the step above to default the dataRows assignment on line 164 to an empty array.

Test Coverage

  • No new tests required.

Visual Regression Changes

  • None anticipated.

QA Brief

Changelog entry

  • N/A
@eugene-manuilov eugene-manuilov added Type: Bug Something isn't working P0 High priority Module: Analytics Google Analytics module related issues labels Sep 2, 2021
@johnPhillips johnPhillips self-assigned this Sep 6, 2021
@johnPhillips johnPhillips added the Good First Issue Good first issue for new engineers label Sep 7, 2021
@johnPhillips johnPhillips removed their assignment Sep 7, 2021
@johnPhillips
Copy link
Contributor

Note: this is possibly related to #3880

@eugene-manuilov eugene-manuilov self-assigned this Sep 7, 2021
@eugene-manuilov
Copy link
Collaborator Author

Thanks, @johnPhillips. Looks like the same problem exists in the DashboardGoalsWidget component. Let's take care of it too.

@johnPhillips
Copy link
Contributor

Thanks @eugene-manuilov, good catch. I just spent a bit of time looking for any more but I think that's all of them now. I'll update the IB with this one 👍

@eugene-manuilov
Copy link
Collaborator Author

IB ✔️

@johnPhillips
Copy link
Contributor

Note: I ended up implementing this as part of #3880 as there was some overlap with that issue. Moving to QA.

@johnPhillips johnPhillips removed their assignment Sep 14, 2021
@wpdarren wpdarren self-assigned this Sep 14, 2021
@wpdarren
Copy link
Collaborator

QA Update: ✅

For the Unique Visitors from Search and Goals Completed widgets:

  • Can confirm that Zero total users loaded with no errors.
  • Can confirm that One or more total users, but none from search loaded with no errors.

image

@wpdarren wpdarren removed their assignment Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good first issue for new engineers Module: Analytics Google Analytics module related issues P0 High priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants