-
Notifications
You must be signed in to change notification settings - Fork 286
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
Use GA4 data for DashboardTopEarningPagesWidget
#8059
Comments
Note: parts of the logic for rendering this widget is covered in #8050. This IB should add the site-kit-wp/assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidget.js Lines 152 to 158 in 8612fe9
|
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 |
@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 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 👍 |
Sounds good. Thanks, @aaemnnosttv 👍 |
@hussain-t please refer to the part of the design doc I referenced in the comment above. |
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. |
Thanks @hussain-t! Heads up for @ivonac4 as well as she's managing sprints at the moment. |
Thanks for the info, updated in the Epic tracker and Design doc. |
@hussain-t I don't think we need to include
FWIW, this is IB ✅ |
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 |
Got a confirmation that blocking issues will most likely be merged tomorrow, assigning myself back |
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. |
QA Update
|
QA Update ✅
|
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
DashboardTopEarningPagesWidgetGA4
should fetch the top earning pages from the GA4 API and display them in the same way thatDashboardTopEarningPagesWidget
does.adSenseLinked
account setting is detected (see: AddadSenseLinked
andadSenseLinkedLastSyncedAt
settings tomodules/analytics-4
data store #8048).adSenseLinked
account setting is not set,AdSenseLinkCTA
should appear instead, instructing the user on how to link their accounts. See: https://docs.google.com/document/d/1-JenwPTAkw0eTkmESzjTOfgphs9qpRYH0h3xk_s3a4o/edit#heading=h.g3qjqjqd6s0xImplementation Brief
In
assets/js/modules/adsense/components/dashboard/DashboardTopEarningPagesWidgetGA4.js
:DashboardTopEarningPagesWidget.js
intoDashboardTopEarningPagesWidgetGA4.js
.AdSenseLinkCTA
based on theadSenseLinked
account setting is being implemented in #8050.MODULES_ANALYTICS_4
store instead ofMODULES_ANALYTICS
:isGatheringData
getReport
getErrorForSelector
hasFinishedResolution
getAdsenseLinked
getServiceReportURL
getReport
selector:dimensions
:[ 'pageTitle', 'pagePath', 'adSourceName' ]
metrics
:[ { name: 'totalAdRevenue' } ]
,dimensionFilter
:{ filter: { fieldName: 'adSourceName', stringFilter: { matchType: 'EXACT', value: 'Google AdSense account (pub-XXXXX)', }, }, }
(Replacepub-XXXXX
with the actual AdSense account ID. See Querying AdSense metrics via Analytics 4)orderBys
:[ { metric: { metricName: 'totalAdRevenue' } } ]
,limit
:5
.tableColumns
array with the following:title
and theurl
fromrow.dimensionValues
instead ofrow.dimensions
.getServiceReportURL
with theunifiedPagePathScreen
filters and pass it to theLink
component.field
tometricValues.0.value
instead ofmetrics.0.values.0
.data?.rows
to therows
prop of theReportTable
component instead ofdata?.[ 0 ]?.data?.rows
.Test Coverage
DashboardTopEarningPagesWidgetGA4.stories.js
.Test Coverage
QA Brief
ga4AdSenseIntegration
feature flaggooglesitekit.data.dispatch('modules/analytics-4').setAdSenseLinked(true)
When the
ga4AdSenseIntegration
feature flag is disabled, this KMW tile should not appear.Changelog entry
The text was updated successfully, but these errors were encountered: