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.23k stars 284 forks source link

Use GA4 data for `DashboardTopEarningPagesWidget` #8059

Closed tofumatt closed 7 months ago

tofumatt commented 8 months ago

Feature Description

The DashboardTopEarningPagesWidgetGA4 should be updated to use GA4 data/APIs once #8048 is available to detect AdSense linked accounts in GA4 properties.


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

Acceptance criteria

Implementation Brief

In assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4.js:

Test Coverage

Test Coverage

QA Brief

When the ga4AdSenseIntegration feature flag is disabled, this KMW tile should not appear.

Changelog entry

tofumatt commented 8 months ago

Note: parts of the logic for rendering this widget is covered in #8050. This IB should add the loading || isGatheringData === undefined logic from here though: https://github.com/google/site-kit-wp/blob/8612fe95e03b6029aab6cd32c9df6afd1d860225/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidget.js#L152-L158

hussain-t commented 7 months ago

Hi @aaemnnosttv @tofumatt, I've encountered a challenge as we progress with this. In the GA4 migration documentation, I couldn't locate the GA4 equivalents for ga:adsenseRevenue, ga:adsenseECPM, and ga:adsensePageImpressions. It appears that totalAdRevenue might correspond to ga:adsenseRevenue, but the direct mappings for the other two metrics are unclear. Additionally, I found some extra metrics in this support guide. The Analytics UI provides some insight into the new metrics, but this implies we'll need to adapt our widget and revise the design doc accordingly. Could you give me insights or guidance on approaching these metric mappings and the widget modification?

aaemnnosttv commented 7 months ago

@hussain-t as noted in the design doc (see Querying AdSense metrics via Analytics 4), there is not an equivalent for these metrics, but the only one we're really concerned about is totalAdRevenue which we need to filter for the specific AdSense account to get the numbers just for AdSense.

While the legacy component we're reimplementing here has references to other metrics in it as you pointed out, they aren't actually used in its current state. It could be those were removed from the presentation at some point without being removed from the report we request, but the component will only show linked page titles with one column for earnings (revenue).

Let me know if you have any other questions 👍

hussain-t commented 7 months ago

Sounds good. Thanks, @aaemnnosttv 👍

aaemnnosttv commented 7 months ago

Construct the report arguments object with the following properties and pass it to the getReport selector:

  • dimensions: pageTitle and pagePath,
  • metrics: totalAdRevenue

@hussain-t please refer to the part of the design doc I referenced in the comment above. totalAdRevenue is not AdSense-specific and could contain revenue from other sources, or even other AdSense accounts than the one connected in SK. In order to get accurate metrics, we need to apply filtering in the request.

hussain-t commented 7 months ago

Thanks, @aaemnnosttv. I've updated the IB to include the dimension filters. I've also verified it using https://ga-dev-tools.google/ga4/query-explorer/. Please LMK if that looks good.

hussain-t commented 7 months ago

Hi @tofumatt @bethanylang, I have marked this issue as blocked due to its dependency on #8050. Displaying the AdSenseLinkCTA based on the adSenseLinked account setting is being implemented in 8050. Please refer to this comment and this.

mxbclang commented 7 months ago

Thanks @hussain-t! Heads up for @ivonac4 as well as she's managing sprints at the moment.

ivonac4 commented 7 months ago

Thanks for the info, updated in the Epic tracker and Design doc.

aaemnnosttv commented 7 months ago
  • dimensions: [ 'pageTitle', 'pagePath', 'adSourceName' ]

@hussain-t I don't think we need to include adSourceName as a dimension in the request since we won't display that anywhere, it should only be needed as a filter. We can confirm this during execution/review though.

getAdsenseLinked

FWIW, this is getAdSenseLinked in GA4.

IB ✅

zutigrm commented 7 months ago

There is still a pending issue in execution before the blocking dependency in MR can be merged. Un-assigning myself until this is ready to be worked on

zutigrm commented 7 months ago

Got a confirmation that blocking issues will most likely be merged tomorrow, assigning myself back

tofumatt commented 7 months ago

I've checked with @marrrmarrr to see whether this tile should filter out non-AdSense revenue or not. Currently it doesn't, which is expected from a QA perspective (eg. revenue from other sources should appear, see: https://10up.slack.com/archives/CBKKQEBR9/p1708348963677589?thread_ts=1708348556.944389&cid=CBKKQEBR9).

If it should be filtered, we'll file a follow-up issue.

mohitwp commented 7 months ago

QA Update ⚠️

@kuasha420

Question > There is some data difference between DashboardTopEarningPagesWidget and 'Top Earning Pages" KM widget. I'm expecting that revenue data for pages in both widgets will be similaer. But see below screenshots under KM Widget revenue data for `[What do “L” or “N” car stickers mean in Ireland? – Oi.ie – Brasileiros na Irlanda] page is different in both widget in KM it is $0.55 and in Dashboard widget it is $0.51. Key metrics data is more close to actual revenue which is $0.56.

![image](https://github.com/google/site-kit-wp/assets/94359491/39c93e9a-3f69-48c6-8fa8-bb833de5cfc3) ![image](https://github.com/google/site-kit-wp/assets/94359491/f01b0aa1-cd97-4029-af68-120fda322158) ![image](https://github.com/google/site-kit-wp/assets/94359491/7549813c-32c6-4621-8b09-c0af5801a654)
kuasha420 commented 7 months ago

@mohitwp Thanks for the findings! This is a known issue and will be fixed in #8281 where data for both sources will be filtered by AdSense only. Cheers!

mohitwp commented 7 months ago

QA Update ✅

![image](https://github.com/google/site-kit-wp/assets/94359491/bb7641ae-52a0-4c9b-9474-8db50cb3f5fa)