site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Extend Consent Mode conditions for determining Ads connection status via Analytics tag config

Open techanvil opened this issue 1 year ago • 2 comments

Feature Description

We should extend the Consent Mode conditions for determining whether Ads is connected to include a check for the presence of an Ads tag as the destination of the connected Analytics property's Google tag.

As described under "when does this apply?" in the one-pager:

For Site Kit, this affects measurement that is connected to Ads in some way. The most explicit example is the placement of an Ads conversion ID tag on the page (e.g. AW-12345678). However, this also applies to Analytics in the event where it is connected to Ads in some way, such as an Ads link (service side link between a GA4 property and Ads account), or using an Ads tag as a destination of a Google Tag (GT-, G-)


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

Acceptance criteria

  • The list of destination IDs for the connected Analytics property's Google tag should be looked up and stored when the property is connected.
  • The list of destination IDs should be updated on a regular basis alongside the current Google tag settings sync (i.e. this should be included in the *syncGoogleTagSettings() action).
  • Within the Consent Mode feature, the conditions for determining whether Ads is connected should include a check for the presence of an Ads tag (starting with AW-) in the list of destination IDs. Specifically this means the conditions for showing the following should be updated:
    • The Setup CTA Banner.
    • The "Recommended" badge and the notice reading "If you have Google Ads campaigns for this site, it's highly recommended to enable Consent mode..." on the Consent Mode section of the Settings screen.

Implementation Brief

  • [ ] In https://github.com/google/site-kit-wp/blob/1ee2a8a87e7cf9bedb82ffa25563f3b7272c6345/assets/js/modules/analytics-4/datastore/properties.js#L589-L594
    • Dispatch the fetchGetGoogleTagContainerDestinations action from MODULE_ANALYTICS_4 store to retrieve the latest container destinations
      • It will fetch and update the list of container destinations. You can obtain required parameters for that action - googleTagAccountID and googleTagContainerID - from Analytics 4 settings.
  • Update assets/js/components/settings/SettingsCardConsentMode.js
    • Fetch the container destination ID's https://github.com/google/site-kit-wp/blob/39347c0732af356a099d9e81811835da929b7f2b/assets/js/components/notifications/GoogleTagIDMismatchNotification.js#L48-L51 and iterate through returned objects, and check if destinationId property contains string AW- to return true. Include this as additional check for existing isAdsConnected variable
    • It is not in scope, but it is a good opportunity to update this part https://github.com/google/site-kit-wp/blob/39347c0732af356a099d9e81811835da929b7f2b/assets/js/components/settings/SettingsCardConsentMode.js#L51-L52 to use the setting field from Ads module, since it was implemented recently. Since it is a very tiny change we can do it as part of this issue.
  • Update assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js
    • Do the same here, include the check for AW- in container destination IDs and include it in check for existing isAdsConnected variable. This will cover the conditional rendering of both recommended badge and notice mentioned in AC

Test Coverage

  • Update any failing test/stories.
  • Check assets/js/modules/analytics-4/datastore/properties.test.js for tests under syncGoogleTagSettings, it can include the test case for fetching the container destinations, and potentially some other cases might need updating/mocking this endpoint

QA Brief

Changelog entry

techanvil avatar Mar 27 '24 11:03 techanvil

AC ✔️

eugene-manuilov avatar Mar 27 '24 17:03 eugene-manuilov

IB ✔️

eugene-manuilov avatar Mar 29 '24 21:03 eugene-manuilov

Hi @techanvil & @zutigrm. I am looking for some clarifications regarding the ACs and IB.

Here is an AC:

The list of destination IDs for the connected Analytics property's Google tag should be looked up and stored when the property is connected.

Does this mean the container destinations should be stored in a module setting and updated as part of syncGoogleTagSettings, even though the container destinations are already accessible using the analytics-4 getGoogleTagContainerDestinations selector?

If the above is true, that doesn't seem to be covered in the IB. I am not sure what the first point of the IB does:

In https://github.com/google/site-kit-wp/blob/1ee2a8a87e7cf9bedb82ffa25563f3b7272c6345/assets/js/modules/analytics-4/datastore/properties.js#L589-L594

  • Dispatch the fetchGetGoogleTagContainerDestinations action from MODULE_ANALYTICS_4 store to retrieve the latest container destinations

    • It will fetch and update the list of container destinations. You can obtain required parameters for that action - googleTagAccountID and googleTagContainerID - from Analytics 4 settings.

The fetchGetGoogleTagContainerDestinations, or the analytics-4 getGoogleTagContainerDestinations selector basically uses the Tag Manager service to list container destinations. Simply calling this in syncGoogleTagSettings will not store the container destinations anywhere.

CC: @eugene-manuilov

nfmohit avatar Apr 09 '24 08:04 nfmohit

Hi @nfmohit, thanks for raising this. The intention of the AC point is that we should be caching the list of destination IDs, and as you've suggested it seems a module setting would be most appropriate for this, keeping them alongside the cached googleTagAccountID and googleTagContainerID settings.

If we don't cache the destination IDs we could end up making an Analytics API request to look them up for every dashboard page load for the check in ConsentModeSetupCTAWidget, and we want to keep the number of additional requests to a minimum.

Please feel free to send this back to IB or to simply make the appropriate amendment during implementation, whichever you prefer.

techanvil avatar Apr 09 '24 08:04 techanvil

Hi @nfmohit

The first point covers that AC point, the fetch store will retrieve the list of container destination IDs and store it in the module settings https://github.com/google/site-kit-wp/blob/f9a0480c5612f3a6c32e2703a5070792fa7351e9/assets/js/modules/analytics-4/datastore/containers.js#L70-L83

cc: @techanvil

zutigrm avatar Apr 09 '24 08:04 zutigrm

Thank you for your response, @zutigrm. I think you mean the above will store the destination IDs in the client-side datastore and not the module settings. However, as per Tom's comment above, we are looking for a server-side cache of the destinations here, as the client-side data will be lost upon refresh, and syncGoogleTagSettings only executes periodically.

Ideally, we'll need to add a new module setting to Analytics, e.g. googleTagContainerDestinations and update that as part of syncGoogleTagSettings.

nfmohit avatar Apr 09 '24 09:04 nfmohit

Hi @ivonac4, as per our internal discussion and the above comments, please note that there is a small chance that this issue goes above estimate. Thank you!

nfmohit avatar Apr 09 '24 09:04 nfmohit

@nfmohit aha I see what you mean, got it. I misunderstood that part that it should be stored in the DB settings.

zutigrm avatar Apr 09 '24 09:04 zutigrm

@zutigrm sorry, I realise the AC could have been clearer in this regard!

techanvil avatar Apr 09 '24 09:04 techanvil

QA Update :warning:

Hi @nfmohit , I've tried to set up as per the ACs but could not see the Consent Mode banner notification appearing in my SK dashboard.

Here are the steps taken:

  • Set up Site Kit dashboard without setting up a conversion ID in the Analytics Module
  • Created a new account for my temporary site here: https://tagmanager.google.com/
  • Used the Google Tag from the Analytics module to create a Google Tag under the https://tagmanager.google.com/
    Screenshot 2024-04-15 at 20.54.17.png
  • Created a Google Ads Conversion Tracking
    Screenshot 2024-04-15 at 20.58.01.png
  • Ran the script in the console and although it shows Fulfilled promise, there was an error in the result section. I am not sure if it affects anything here.
    Screenshot 2024-04-15 at 20.07.42.png
  • After all this the consent banner doesn't appear at the Site Kit dashboard.

I am not really sure where the problem lies. It could be that the setup was wrong. Appreciate if you can have a look and maybe add details to the QAB if needed.

kelvinballoo avatar Apr 15 '24 14:04 kelvinballoo

QA Update ⚠️ - Testing on hold

Based on latest conversations with @nfmohit , it looks like the banner has actually stopped appearing, but due to a change in #8365.

Will pick up testing after the issue is resolved.

kelvinballoo avatar Apr 16 '24 12:04 kelvinballoo

Hi @kelvinballoo. The other issue has now been fixed. Please let me know if you still encounter an issue with testing this. Thanks!

nfmohit avatar Apr 16 '24 20:04 nfmohit

QA Update ✅

Tested on 6.5.2 PHP 8.0 and did the following steps:

  • Set up Site Kit with the Analytics account.
  • While setting up the Analytics account, choose to create a new Analytics account/property.
  • Once Analytics is set up, go to Site Kit settings -> Analytics settings. Took note of the Google Tag ID.
  • Go to https://tagmanager.google.com/#/home#tags and look for the Google Tag ID.
  • Add a Ads conversion ID to this Google Tag as destination.

Test Outcome:

  • In Google Tag manager, a Google Ads tag was added (AW-16533937991) as a destination.
    Screenshot 2024-04-17 at 17.46.50.png
  • Before running the script in browser console, there was no consent mode:

    Screenshot 2024-04-17 at 17.37.33.png
  • After running the script, consent mode and recommended badge appeared as expected:
    Screenshot 2024-04-17 at 17.45.52.png Screenshot 2024-04-17 at 17.46.12.png
  • After that, I ran the script googlesitekit.data.select( 'modules/analytics-4' ).getSettings() to validate that we are getting the correct Conversion ID and it's as expected.
    Screenshot 2024-04-17 at 17.58.37.png

Moving this ticket to Approval.

kelvinballoo avatar Apr 17 '24 10:04 kelvinballoo