-
Notifications
You must be signed in to change notification settings - Fork 295
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
Create the "Top earning content" key metric widget tile #6248
Comments
Moving this issue to stalled for now as we will have to wait for the GA4 API to support an integration with AdSense. c.c. @aaemnnosttv |
@tofumatt FYI, we discussed this issue for the Top earning content tile during today's Updates & Planning call. This as one of the tiles we initially had planned for KMW, but the GA4 integration with AdSense wasn't yet ready. Now that we're going to be building the integration, we'd like to add this tile, too. @aaemnnosttv that we should be able to work on it and get it implemented in the latter part of the AdSense + GA4 epic. I'm going to make a note to myself to move it out of Stalled and into AC once we start moving on engineering for this epic. Let me know if you see any issues with this approach! |
@kuasha420 Just noting that we'd like to include this in the next sprint (starting w/c 12 Feb) so we should probably get it moving to ACR soon. Thanks! |
@kuasha420 can we add an estimate to this task at this point? |
@ivonac4 Usually estimates aren't added until the IB step. If you're looking for an estimate to help with sprint planning, I'd put it in as a 19 to be safe – most of the tiles we've built in the past were 15 or 19 point issues. :) |
AC ✔️ |
Hi @kuasha420 , what content should we use for |
Hi @zutigrm, thanks for drafting this IB. The HOC is an interesting idea. However, I don't think we'll need it: we already have the So, please can you make an appropriate update to this IB. |
@wpdarren As mentioned in slack are you able to link AdSense and Analytics using oi.ie or any of your live site and not getting prompt if analytics and AdSense are already linked. Like as I mentioned in slack happening with oi.ie site ? |
@mohitwp the testing on this ticket is related to the I am wondering if we need to wait for another ticket to be merged because we can test this ticket and #8051 so maybe @benbowler @tofumatt can help here. |
I believe this issue depends on However, it does need a followup PR as per #8249 (review). |
@tofumatt @zutigrm I have pushed a few updates to the PR #8257 and have tested it myself and I'm able to view it successfully when manually updating the Back into CR with you @tofumatt |
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
|
@mohitwp Yes, I believe this is the correct report link because it shows the "Total revenue" metric for this specific page. |
QA Update ✅
|
Note : Putting note here to test again when #8049 get merge. |
Feature Description
This key metric widget tile should only appear if users have both Analytics (4) and AdSense modules connected. When there is no
adSenseLinked
account available in Analytics settings (see #8048), this widget should display a CTA asking users to link their AdSense and Analytics (see #8050).If the
adSenseLinked
account setting is set, the actual content should be displayed.NOTE: This issue must be deployed in the same release as #8051 and #8050.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
title
:Top earning pages
description
: Pages that generated the most AdSense revenueslug
:kmAnalyticsAdSenseTopEarningContent
totalAdRevenue
pagePath
pageTitle
showing up as the first column, and totalAdRevenue showing up as the second column (with currency sign).analytics-4
andadsense
module. If either module is not connected, the correspondingConnect{GA4/AdSense}CTATileWidget
should be rendered instead.Implementation Brief
Update
assets/js/modules/adsense/components/widgets/TopEarningContentWidget.js
PopularContentWidget
as starting pointreportOptions
with parameters outlined in the AC3
for consistency with other widgetsselect( MODULES_ANALYTICS_4 ).getAdsenseLinked()
and whether the context is view-only dashboard. If the context is not view-only andadsenseLinked
is false, output early<AdSenseLinkCTA />
component wrapped with<Widget>
componentnumFmt
function, pass an object as second parameter{style: 'currency', currency}
. You can obtaincurrency
fromreport
object -report.metadata.currencyCode
Move
AdSenseLinkCTA
component over to the Adsense moduleassets/js/modules/analytics-4/components/common/AdSenseLinkCTA.js
to theassets/js/modules/adsense/components/common/AdSenseLinkCTA.js
Add
KM_ANALYTICS_ADSENSE_TOP_EARNING_CONTENT
to theassets/js/components/KeyMetrics/key-metrics-widgets.js
title
anddescription
text defined in ACdescription
text forinfoTooltip
displayInList
it should show if following conditions are met:analytics-4
andadsense
modules are activeisKeyMetricActive
selector onCORE_USER
storegetAdSenseLinked()
)Update
assets/js/modules/adsense/index.js
:KM_ANALYTICS_ADSENSE_TOP_EARNING_CONTENT
widget registration, forisActive
parameter:isKeyMetricActive
, check if we are in view only context, and in that case Analytics and AdSense should be linked (getAdSenseLinked
), otherwise widget should not be displayed for view only usersTest Coverage
assets/js/components/KeyMetrics/MetricsSelectionPanel/index.test.js
where applicableTopEarningContent
QA Brief
ga4AdSenseIntegration
feature flagTopEarningContentWidget
in Storybook and confirm each state matches designs/AC.adSenseLinked
setting in the store (this will allow you to QA this ticket before Schedule a cron task to syncadSenseLinked
status #8049 is merged).You can achieve this by running the following the developer console:
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: