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

Refactor the Audience Tiles to use new pivot report infrastructure #8726

Closed
15 tasks
benbowler opened this issue May 17, 2024 · 15 comments
Closed
15 tasks

Refactor the Audience Tiles to use new pivot report infrastructure #8726

benbowler opened this issue May 17, 2024 · 15 comments
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority QA: Eng Requires specialized QA by an engineer Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@benbowler
Copy link
Collaborator

benbowler commented May 17, 2024

Feature Description

As discussed on Slack, while we are initially implementing the Audience Tiles to use separate, per-audience reports to retrieve the cities and "top content" metrics (one report per audience per metric), we can reduce the number of reports needed by using pivot reports. This will allow us to retrieve metric data for all of the audiences in a single report (i.e., one report per metric).

We should add support for running pivot reports, and refactor the Audience Tiles to use them.

This is not critical to the release and can be done post-launch, hence the P2 priority.

Please also note this comment relating to the AudienceTile and AudienceTiles refactoring: #8484 (comment)

This ticket has been split from #8484, and only concerns updating the Audience Tiles to use the new pivot report infrastructure, which will be completed in that ticket.


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

Acceptance criteria

  • The AudienceTiles component (introduced via Add the Audience Tiles widget (Storybook) #8136) should be refactored to use pivot reports for the "top cities" and "top content" metrics, thus reducing the corresponding request count by a factor of the number of selected audiences.
  • The AudienceTiles and AudienceTile components should be further refactored so the reports that are retrieved in AudienceTiles are passed directly to AudienceTile, and the corresponding data parsing/extraction logic is moved from AudienceTiles to AudienceTile. Hopefully this logic can be simplified in the process.
  • Additionally, refactor the AudienceTile stories to replace the hardwired mock data with generated mock report data.

Implementation Brief

Audience Segmentation Report Updates

  • Update the assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTilesWidget.js component:

    • Update topCitiesReportOptions to the following report structure:
      startDate,
      endDate,
      dimensions: [ { name: 'city' }, { name: 'audienceResourceName' } ],
      dimensionFilters: {
      	audienceResourceName: configuredAudiences,
      },
      metrics: [ { name: 'totalUsers' } ],
      pivots: [
      	{
      		fieldNames: [ 'city' ],
      		orderby: [
      			{ metric: { metricName: 'totalUsers' }, desc: true },
      		],
      		limit: 3,
      	},
      	{
      		fieldNames: [ 'audienceResourceName' ],
      	},
      ],
      
    • Update the topCitiesReport to use the new getPivotReport selector instead of getReportForAllAudiences, passing topCitiesReportOptions as the only prop.
    • Update topContentReportOptions to the following report structure:
      startDate,
      endDate,
      dimensions: [ { name: 'pagePath' }, { name: 'audienceResourceName' } ],
      dimensionFilters: {
      	audienceResourceName: configuredAudiences,
      },
      metrics: [ { name: 'screenPageViews' } ],
      pivots: [
      	{
      		fieldNames: [ 'pagePath' ],
      		orderby: [
      			{ metric: { metricName: 'screenPageViews' }, desc: true },
      		],
      		limit: 3,
      	},
      	{
      		fieldNames: [ 'audienceResourceName' ],
      	},
      ],
      
    • Update the topContentReport to use the new getPivotReport selector instead of getReportForAllAudiences, passing topContentReportOptions as the only prop.
    • Update topContentPageTitlesReportOptions to the following report structure:
      startDate,
      endDate,
      dimensions: [ { name: 'pagePath' }, { name: 'pageTitle' }, { name: 'audienceResourceName' } ],
      dimensionFilters: {
      	audienceResourceName: configuredAudiences,
      },
      metrics: [ { name: 'screenPageViews' } ],
      pivots: [
      	{
      		fieldNames: [ 'pagePath' , 'pageTitle' ],
      		orderby: [
      			{ metric: { metricName: 'screenPageViews' }, desc: true },
      		],
      		limit: 15,
      	},
      	{
      		fieldNames: [ 'audienceResourceName' ],
      	},
      ],
      
    • Update the topContentPageTitlesReport to use the new getPivotReport selector instead of getReportForAllAudiences, passing topContentPageTitlesReportOptions as the only prop.
  • Remove the getReportForAllAudiences selector and all related code/tests.

  • Update assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceTiles.js:

AudienceTile Updates

Test Coverage

  • Confirm existing tests pass, along with VRTs.

QA Brief

  1. A round of smoke test to ensure that this feature has not changed anything in the Audience tiles widget. Also that data should be intact within those tiles.

  2. Ensure that top cities are displaying in the widget. Verify the data from Google Analytics dashboard that it is displaying correctly.

  3. Ensure that "Top content by pageviews" are displaying in the widget. Verify the data from Google Analytics dashboard that it is displaying correctly.

QA:Eng

In the browser dev tools check that pivot reports are loading and getting the response.

Changelog entry

  • Refactor the Audience Tiles to use pivot reports, reducing the number of reports needed to retrieve the tile data.
@benbowler
Copy link
Collaborator Author

FYI, this ticket was split from #8484 on 17 May 2024. #8484 is a blocker.

@techanvil techanvil added P1 Medium priority Type: Enhancement Improvement of an existing feature Module: Analytics Google Analytics module related issues Team M Issues for Squad 2 labels May 17, 2024
@techanvil
Copy link
Collaborator

Hey @benbowler, thanks for drafting this IB. A few points:

  • The first report in AudienceTiles doesn't seem to need the update - it's already being retrieved via getReport() rather than getReportForAllAudiences(), and seems fine as it is AFAICT.
  • The remaining reports should be updated to use the getPivotReport() selector in line with the proposed changes to Add support for GA4 pivot reports #8484, assuming they have been made.
  • The report with options topContentPageTitlesReportOptions should also be covered in the IB (including the refactoring of the restructuring code).
  • A minor point but the report option code blocks are slightly misaligned in places.
  • Another minor one, I think the point in Data Mock Updates about updating "AudienceTile stories to use this new data-mock functionality" should refer to "the new data-mock functionality introduced in Add support for GA4 pivot reports #8484" for the sake of clarity.

@techanvil
Copy link
Collaborator

Thanks for the update @benbowler. It looks like these two points have been missed, though, please can you take a look?

  • The report with options topContentPageTitlesReportOptions should also be covered in the IB (including the refactoring of the restructuring code).
  • A minor point but the report option code blocks are slightly misaligned in places.

@techanvil techanvil assigned benbowler and unassigned techanvil May 30, 2024
@benbowler benbowler assigned techanvil and unassigned benbowler May 30, 2024
@techanvil
Copy link
Collaborator

Thanks Ben! This IB LGTM. ✅

@techanvil techanvil removed their assignment May 30, 2024
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Jun 3, 2024
@ankitrox ankitrox self-assigned this Jun 13, 2024
@ankitrox
Copy link
Collaborator

This is ticket is ready to be moved to CR, but it should be done once #8484 is merged and tested successfully as #8484 is the dependency for this and any changes in 8484 may lead to the changes in this task.

@techanvil
Copy link
Collaborator

Thanks @ankitrox - in cases like this, we don't tend to wait for the dependency to go through QA, simply having it merged is enough to move this to CR. Please remember to merge develop into this issue's branch, and retarget the PR first :)

@ankitrox
Copy link
Collaborator

Thanks @techanvil . I will move this to CR as soon as #8484 gets merged. I will also take care to update base branch and merge it to feature one.

Thank you.

@ankitrox ankitrox removed their assignment Jun 20, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Jun 24, 2024
@techanvil techanvil assigned ankitrox and unassigned techanvil Jun 25, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Jun 26, 2024
@ankitrox ankitrox added the QA: Eng Requires specialized QA by an engineer label Jun 26, 2024
@techanvil techanvil assigned ankitrox and unassigned techanvil Jun 26, 2024
@ankitrox ankitrox assigned techanvil and unassigned ankitrox Jun 28, 2024
@techanvil techanvil removed their assignment Jul 1, 2024
@wpdarren
Copy link
Collaborator

wpdarren commented Jul 3, 2024

@kelvinballoo as per our chat on Slack. I have assigned this to you. Once you have completed your testing please unassign yourself and we'll get an engineer to look over it for the QA:Eng part.

@kelvinballoo
Copy link
Collaborator

QA Update ⚠️

Despite being simple, this is a bit difficult to smoke test because a lot of things could be because of other tickets not deployed yet.

The tiles are appearing as how they were previously. ✅
That said, the tricky part is about verifying whether data is intact .
Here are some observations to be reviewed.
(For context, I used oi.ie property for testing)

ITEM 1:
A round of smoke test to ensure that this feature has not changed anything in the Audience tiles widget. Also that data should be intact within those tiles.

  • What are we actually meaning by intact data?
  • Verifying the data from Analytics doesn't seem to correspond. Refer to the video attached.
Intact.data.mov
__________________________________________________________________

ITEM 2:
Ensure that top cities are displaying in the widget. Verify the data from Google Analytics dashboard that it is displaying correctly.
The top cities are displaying but it's showing one of them as '(not set)'

The second city is (not set) Screenshot 2024-07-03 at 18 23 44

On analytics, I could see the cities. Is it something to be set up in the BE?
Screenshot 2024-07-03 at 18 25 07


ITEM 3:

Ensure that "Top content by pageviews" are displaying in the widget. Verify the data from Google Analytics dashboard that it is displaying correctly.
I've verified the number from GA and the tile for the 'All visitors' at the 'Top content by pageviews' section and they matched for the first 2 but not the third.

The 3rd item is 245 on the tile. Screenshot 2024-07-03 at 18 33 54

The 3rd item is 403 in Analytics
Screenshot 2024-07-03 at 18 34 05


ITEM 4:
I feel the results of the tiles are not properly matched.

Refer to the attached video for details

Unmatched.results.mov

@techanvil
Copy link
Collaborator

techanvil commented Jul 3, 2024

Hey @kelvinballoo, what I'd suggest for the smoke test is to refer back to this comment I left on #8763 where I point out that we can see the same metrics elsewhere on the dashboard.

It's worth noting that all of the metrics for an audience are viewable elsewhere in Site Kit, so when viewing the "all visitors" audience, we can cross reference other widgets as a sanity check for the figures.
To illustrate the point, here's a map of the metrics on the Audience Tile to the same metrics on Key Metric tiles and the All Traffic widget:

image

You can cross reference the metrics like this to check if anything has changed.

If the wider dashboard matches the All Visitors audience tile but you find discrepancies between the figures and what you see on Analytics, this can be raised as a separate issue for investigation.

@kelvinballoo
Copy link
Collaborator

QA Update ✅

Thanks @techanvil , that really helps.

I've verified this way and I can confirm that the tiles data is matching the data on the other part of the SK dashboard.
Marking this as Pass but not moving this to approval yet since it's a 'QA Eng' ticket.

Screenshot 2024-07-04 at 14 05 15 Screenshot 2024-07-04 at 14 05 23

@techanvil
Copy link
Collaborator

QA:Eng ❌

While performing QA:Eng for this issue, I've had a horrible realisation that pivot reports aren't actually suitable for the audience tiles in the way we thought they were.

This has come to light while examining the report output for audiences which have visitors from different cities.

As can be seen in the current audience data for oi.ie, the "Mauritius users" audience has visitors from London:
image

While "Returning visitors" has visitors from Dublin, "(not set)", and Cork:
image

However when both of these audiences are visible, we only see Dublin, "(not set)", and Cork for both audiences, with no sign of London:

image

This was admittedly made harder to spot by the fact we have the open "2nd tile has no data" bug, #8921 which adds a bit of ambiguity. However the drilling into the actual reports as returned by the API indicates this is indeed the case and the reports are simply not returning the data in the way that we would like.

Essentially, each pivot in the report effectively results in a sub-query to retrieve all the values for that query similar to a regular report where the dimensionFilter and so forth come into play. It's then using the returned values as the values to pivot on. So when retrieving the top 3 cities while both audiences are included in the filter, it's always returning Dublin, "(not set)", and Cork, as these cities all have more totalUsers than London.

It's only when we remove "Returning visitors" from the query - meaning our report is filtering results for just the "Mauritius users" audience - that London is returned, because the other cities are being excluded - they are only associated with the other audience which is no longer included in the filter.

To help to understand this further here are some illustrative reports and their responses to compare:

Report request with a filter for the "Mauritius users" audience
{
  "dateRanges": [
    {
      "endDate": "2024-07-04",
      "startDate": "2024-06-07"
    }
  ],
  "dimensionFilter": {
    "andGroup": {
      "expressions": [
        {
          "filter": {
            "fieldName": "hostName",
            "inListFilter": {
              "values": ["oi.ie", "www.oi.ie"]
            }
          }
        },
        {
          "filter": {
            "fieldName": "audienceResourceName",
            "inListFilter": {
              "values": ["properties/285794360/audiences/8278021604"]
            }
          }
        }
      ]
    }
  },
  "dimensions": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    },
    {
      "name": "hostName"
    }
  ],
  "keepEmptyRows": true,
  "metrics": [
    {
      "name": "totalUsers"
    }
  ],
  "pivots": [
    {
      "fieldNames": ["city"],
      "limit": "3",
      "orderBys": [
        {
          "desc": "true",
          "metric": {
            "metricName": "totalUsers"
          }
        }
      ]
    },
    {
      "fieldNames": ["audienceResourceName"],
      "limit": "2"
    }
  ],
  "property": "properties/285794360"
}
Response for report filtered by the "Mauritius users" audience
{
  "pivotHeaders": [
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "London"
            }
          ]
        }
      ],
      "rowCount": 1
    },
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "properties/285794360/audiences/8278021604"
            }
          ]
        }
      ],
      "rowCount": 1
    }
  ],
  "dimensionHeaders": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    }
  ],
  "metricHeaders": [
    {
      "name": "totalUsers",
      "type": "TYPE_INTEGER"
    }
  ],
  "rows": [
    {
      "dimensionValues": [
        {
          "value": "London"
        },
        {
          "value": "properties/285794360/audiences/8278021604"
        }
      ],
      "metricValues": [
        {
          "value": "1"
        }
      ]
    }
  ],
  "metadata": {
    "currencyCode": "USD",
    "timeZone": "America/Los_Angeles"
  },
  "kind": "analyticsData#runPivotReport"
}
Report request with a filter for the "Returning visitors" audience
{
  "dateRanges": [
    {
      "endDate": "2024-07-04",
      "startDate": "2024-06-07"
    }
  ],
  "dimensionFilter": {
    "andGroup": {
      "expressions": [
        {
          "filter": {
            "fieldName": "hostName",
            "inListFilter": {
              "values": ["oi.ie", "www.oi.ie"]
            }
          }
        },
        {
          "filter": {
            "fieldName": "audienceResourceName",
            "inListFilter": {
              "values": ["properties/285794360/audiences/8281378703"]
            }
          }
        }
      ]
    }
  },
  "dimensions": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    },
    {
      "name": "hostName"
    }
  ],
  "keepEmptyRows": true,
  "metrics": [
    {
      "name": "totalUsers"
    }
  ],
  "pivots": [
    {
      "fieldNames": ["city"],
      "limit": "3",
      "orderBys": [
        {
          "desc": "true",
          "metric": {
            "metricName": "totalUsers"
          }
        }
      ]
    },
    {
      "fieldNames": ["audienceResourceName"],
      "limit": "2"
    }
  ],
  "property": "properties/285794360"
}
Response for report filtered by the "Returning visitors" audience
{
  "pivotHeaders": [
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "Dublin"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "(not set)"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "Cork"
            }
          ]
        }
      ],
      "rowCount": 151
    },
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "properties/285794360/audiences/8281378703"
            }
          ]
        }
      ],
      "rowCount": 1
    }
  ],
  "dimensionHeaders": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    }
  ],
  "metricHeaders": [
    {
      "name": "totalUsers",
      "type": "TYPE_INTEGER"
    }
  ],
  "rows": [
    {
      "dimensionValues": [
        {
          "value": "Dublin"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "139"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "(not set)"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "59"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "Cork"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "32"
        }
      ]
    }
  ],
  "metadata": {
    "currencyCode": "USD",
    "timeZone": "America/Los_Angeles"
  },
  "kind": "analyticsData#runPivotReport"
}
Report request with a filter for both audiences
{
  "dateRanges": [
    {
      "endDate": "2024-07-04",
      "startDate": "2024-06-07"
    }
  ],
  "dimensionFilter": {
    "andGroup": {
      "expressions": [
        {
          "filter": {
            "fieldName": "hostName",
            "inListFilter": {
              "values": ["oi.ie", "www.oi.ie"]
            }
          }
        },
        {
          "filter": {
            "fieldName": "audienceResourceName",
            "inListFilter": {
              "values": [
                "properties/285794360/audiences/8278021604",
                "properties/285794360/audiences/8281378703"
              ]
            }
          }
        }
      ]
    }
  },
  "dimensions": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    },
    {
      "name": "hostName"
    }
  ],
  "keepEmptyRows": true,
  "metrics": [
    {
      "name": "totalUsers"
    }
  ],
  "pivots": [
    {
      "fieldNames": ["city"],
      "limit": "3",
      "orderBys": [
        {
          "desc": "true",
          "metric": {
            "metricName": "totalUsers"
          }
        }
      ]
    },
    {
      "fieldNames": ["audienceResourceName"],
      "limit": "2"
    }
  ],
  "property": "properties/285794360"
}
Response for report filtered by both audiences
{
  "pivotHeaders": [
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "Dublin"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "(not set)"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "Cork"
            }
          ]
        }
      ],
      "rowCount": 151
    },
    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "properties/285794360/audiences/8281378703"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "properties/285794360/audiences/8278021604"
            }
          ]
        }
      ],
      "rowCount": 2
    }
  ],
  "dimensionHeaders": [
    {
      "name": "city"
    },
    {
      "name": "audienceResourceName"
    }
  ],
  "metricHeaders": [
    {
      "name": "totalUsers",
      "type": "TYPE_INTEGER"
    }
  ],
  "rows": [
    {
      "dimensionValues": [
        {
          "value": "Dublin"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "139"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "(not set)"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "59"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "Cork"
        },
        {
          "value": "properties/285794360/audiences/8281378703"
        }
      ],
      "metricValues": [
        {
          "value": "32"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "(not set)"
        },
        {
          "value": "properties/285794360/audiences/8278021604"
        }
      ],
      "metricValues": [
        {
          "value": "0"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "Cork"
        },
        {
          "value": "properties/285794360/audiences/8278021604"
        }
      ],
      "metricValues": [
        {
          "value": "0"
        }
      ]
    },
    {
      "dimensionValues": [
        {
          "value": "Dublin"
        },
        {
          "value": "properties/285794360/audiences/8278021604"
        }
      ],
      "metricValues": [
        {
          "value": "0"
        }
      ]
    }
  ],
  "metadata": {
    "currencyCode": "USD",
    "timeZone": "America/Los_Angeles"
  },
  "kind": "analyticsData#runPivotReport"
}

As can be seen, when just "Mauritius users" is included in the filter, the cities pivot values are as follows:

    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "London"
            }
          ]
        }
      ],
      "rowCount": 1
    },

When just "Returning visitors" is included in the filter, the values are as follows:

    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "Dublin"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "(not set)"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "Cork"
            }
          ]
        }
      ],
      "rowCount": 151
    },

When both audiences are included in the filter, the values are again as follows, because they are the top 3 cities that are associated with either of the audiences.

    {
      "pivotDimensionHeaders": [
        {
          "dimensionValues": [
            {
              "value": "Dublin"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "(not set)"
            }
          ]
        },
        {
          "dimensionValues": [
            {
              "value": "Cork"
            }
          ]
        }
      ],
      "rowCount": 151
    },

Note that event if we completely remove audienceResourceName from dimensionFilter, the same three cities are returned - these are the top cities across all audiences.

The "cities" case has been illustrated but the general principle applies to the "top content" metric as well.

From what I've seen so far it doesn't seem possible to use a pivot report in the way we'd like to, returning a different selection of pivot values for each audience.

It could the case that I've missed something and there is some way to achieve this, and we can investigate this further at a later date, but for now we need to revert the changes in this issue.

@techanvil techanvil assigned nfmohit and unassigned techanvil Jul 5, 2024
@techanvil
Copy link
Collaborator

Note that due to the above, this issue will be closed as "not planned" and not included in the forthcoming release.

@nfmohit
Copy link
Collaborator

nfmohit commented Jul 5, 2024

The PR #8969 that reverts all changes made by this issue has been merged. As discussed above, I'm closing this issue as not planned.

@nfmohit nfmohit closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
@nfmohit nfmohit removed their assignment Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: Analytics Google Analytics module related issues P1 Medium priority QA: Eng Requires specialized QA by an engineer Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants