site-kit-wp
site-kit-wp copied to clipboard
Extend Consent Mode conditions for determining Ads connection status via Analytics tag config
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
fetchGetGoogleTagContainerDestinationsaction fromMODULE_ANALYTICS_4store to retrieve the latest container destinations- It will fetch and update the list of container destinations. You can obtain required parameters for that action -
googleTagAccountIDandgoogleTagContainerID- from Analytics 4 settings.
- It will fetch and update the list of container destinations. You can obtain required parameters for that action -
- Dispatch the
- 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 existingisAdsConnectedvariable - 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
Adsmodule, since it was implemented recently. Since it is a very tiny change we can do it as part of this issue.
- 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
- 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 existingisAdsConnectedvariable. This will cover the conditional rendering of bothrecommendedbadge and notice mentioned in AC
- Do the same here, include the check for
Test Coverage
- Update any failing test/stories.
- Check
assets/js/modules/analytics-4/datastore/properties.test.jsfor tests undersyncGoogleTagSettings, 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
AC ✔️
IB ✔️
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
fetchGetGoogleTagContainerDestinationsaction fromMODULE_ANALYTICS_4store to retrieve the latest container destinations
- It will fetch and update the list of container destinations. You can obtain required parameters for that action -
googleTagAccountIDandgoogleTagContainerID- 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
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.
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
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.
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 aha I see what you mean, got it. I misunderstood that part that it should be stored in the DB settings.
@zutigrm sorry, I realise the AC could have been clearer in this regard!
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/
- Created a Google Ads Conversion Tracking
- 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.
- 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.
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.
Hi @kelvinballoo. The other issue has now been fixed. Please let me know if you still encounter an issue with testing this. Thanks!
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.
-
Before running the script in browser console, there was no consent mode:
- After running the script, consent mode and recommended badge appeared as expected:
- 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.
Moving this ticket to Approval.