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.25k stars 291 forks source link

Display Top Earning Pages Widget with a CTA to ask users to link their AdSense and Analytics when both modules are connected #8050

Closed tofumatt closed 8 months ago

tofumatt commented 10 months ago

Feature Description

We'll want a version of the "Top Earning Pages Widget" (#6248) that shows a CTA to link the user's AdSense and Analytics accounts when they are not linked (or we don't detect that they are linked).

See: https://docs.google.com/document/d/1-JenwPTAkw0eTkmESzjTOfgphs9qpRYH0h3xk_s3a4o/edit#heading=h.ykym38w7pngs

NOTE: This issue must be deployed in the same release as #6248 and #8051.


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

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

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

Changelog entry

techanvil commented 10 months ago

Hey @tofumatt, regarding the second IB point, I think it would be worth being more specific about what other content/logic we should include, as it looks like some of this should be covered by https://github.com/google/site-kit-wp/issues/8059.

It's also worth noting that the AC for this issue seems to be partially duplicated in #8059 - cc @hussain-t who's working on the IB for that issue which can probably refer in part to this one.

tofumatt commented 9 months ago

Thanks @techanvil. I referenced the lines in the IB, but I've added some notes to the IB here.

I've also referenced this IB in #8059 as well 👍🏻

techanvil commented 9 months ago

Thanks @tofumatt, looks good. Just one thing, the GA4 version of the widget already includes a copy of the Footer component so it looks like we can remove that sub-point.

mxbclang commented 9 months ago

@tofumatt Just noting that this is one that we can get started on at any time, so would be great to get these IB updates taken care of so we can start on it ASAP. Thanks!

techanvil commented 9 months ago

Thanks @tofumatt, IB :white_check_mark:

nfmohit commented 9 months ago

Hi folks!

I've set this issue as blocked by #8048 as the new setting adSenseLinked (see https://github.com/google/site-kit-wp/issues/8047#issuecomment-1903498239) is not yet available in the analytics-4 datastore.

Thanks!

CC: @tofumatt @techanvil @bethanylang @ivonac4

ivonac4 commented 9 months ago

@tofumatt I can see you removed this issue from the AdSense GA4 Integration epic. Was that on purpose? If so, can you loop me in please? Thankss

mxbclang commented 9 months ago

@ivonac4 I'm guessing that was an error, as it happened at the same time this was removed from the release. Adding it back!

ivonac4 commented 9 months ago

Okay, thanks!

tofumatt commented 9 months ago

(Indeed, that looks to be a Zenhub bug! 😅 Thanks @ivonac4 for catching it and @bethanylang for restoring it!)

mohitwp commented 8 months ago

QA Update ❌

Issue : If AdSense module is shared then On View only dashboard error is showing.

image

![image](https://github.com/google/site-kit-wp/assets/94359491/9e87f167-53a1-42c0-9674-75e739e01523) ![image](https://github.com/google/site-kit-wp/assets/94359491/d11d83ec-b791-45be-be37-e652071d505a)
mohitwp commented 8 months ago

QA Update ❌

@zutigrm

Issue : As reported in slack earlier and in #6248 . I used oi.ie site for testing. Analytics and AdSense are already linked for this site but still on main dashboard under Monetization section prompt to link AdSense and Analytics is showing.

image

zutigrm commented 8 months ago

The real adSenseLinked setting value is not pulled yet, so only default value is present, which is false, so it will always show connect section. The value will be fetched in other issue, see this comment

cc: @mohitwp @tofumatt

tofumatt commented 8 months ago

Adding #8049 as a blocker, and removing this issue from this release as we can't really properly test it (or release this widget) until the data can also be fetched from the Analytics API.

I'll create a follow-up issue to not render this for now.

mohitwp commented 8 months ago

QA Update ✅

Note : Putting note here to test again when #8049 get merge.

**Main Dashboard -** ![image](https://github.com/google/site-kit-wp/assets/94359491/82a89048-159c-470c-b059-1fab69a5a2aa) ![image](https://github.com/google/site-kit-wp/assets/94359491/45f06b59-bc36-4736-9ac9-928a8bfe2e1e) **View only dashboard -** ![image](https://github.com/google/site-kit-wp/assets/94359491/0bb70e2a-cae3-457b-87e4-f5940894cfeb) ![image](https://github.com/google/site-kit-wp/assets/94359491/c6fb0c26-9b10-4328-ac8c-c8d38cde9be6) **Ad Blocker active** ![image](https://github.com/google/site-kit-wp/assets/94359491/4137a7c6-47b8-4b05-b37e-9ba43a0d7a24) **Link both AdSense+GA4 via code **snippet**** **Main Dashboard -** ![image](https://github.com/google/site-kit-wp/assets/94359491/3e9b78d0-2613-4bda-9d82-b5a3430a40c9) ![image](https://github.com/google/site-kit-wp/assets/94359491/153eb684-bd44-4b93-a338-eacee0a4f8e2) **View only dashboard -** ![image](https://github.com/google/site-kit-wp/assets/94359491/0f5cc689-4c3a-49b8-8449-8caff91b96b6) ![image](https://github.com/google/site-kit-wp/assets/94359491/eb860777-e82c-4c2f-96f7-8455bbc1accc)