Extend Consent Mode conditions for determining Ads connection status to include a check for an Ads tag as a destination of a Google Tag
Feature Description
For the Consent Mode feature, we have so far implemented the check for Ads being connected to look for the presence of the Ads Conversion ID or an Ads link.
We should extend these conditions to include the additional check discussed under "when does this apply?" in the one-pager - checking for the presence of Ads tag as a destination of the connected Google Tag.
Note that we are now able to use the Tag Manager API to look up a container by tag ID, so this is now technically feasible. See the tagId parameter for the containers/lookup endpoint and the feature request where it was introduced.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Within the Consent Mode feature, the conditions for determining whether Ads is connected should include a check for the presence of an Ads tag as a destination of the connected Google Tag.
- The Google Tag for the connected Analytics property should be checked.
- When Tag Manager is connected the tags for the connected container should be checked.
- Note that the case where a variable is used for the tag configuration will be handled in a separate issue: #9556
- In terms of user-visible behaviour:
- The Setup CTA Banner should be displayed when an Ads tag is detected (while the Consent Mode
enabledsetting isfalseand the banner has not been dismissed). - 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 should be displayed when an Ads tag is detected.
- The Ads users' variant of the confirmation dialog shown when toggling the Consent Mode switch off on the Settings screen should be displayed when an Ads tag is detected.
- The Setup CTA Banner should be displayed when an Ads tag is detected (while the Consent Mode
Implementation Brief
Consent Mode backend updates
- [ ] Update
includes/Core/Consent_Mode/REST_Consent_Mode_Controller.php, creating a new REST endpointrecommend-consent-mode, in this endpoint:- [ ] First check if the Ads module is connected using the
is_connectedmethod, if this is true, return true on the endpoint immediately. - [ ] Next check if Analytics is connected using the
is_connectedmethod and if Ads is linked to Analytics by checking theadSenseLinkedsetting, return true on the endpoint immediately. - [ ] Finally, check if Tag Manager is connected using the
is_connectedmethod, if so:- [ ] Use
$this->get_tagmanager_service()->accounts_containers_versions->live()to get the live version of the configured GTM tag using theaccountIDandinternalContainerIDstored in the module settings: https://github.com/google/site-kit-wp/blob/165c8bb73003e7861b3dc2eead677d034def431e/includes/Modules/Tag_Manager.php#L328-L330 - [ ] Use
array_searchto check if any of the destinations in the tag[] key of the response returned for this version are of the typeawct, which is an "Google Ads Conversion Tracking" tag, return true if so. - [ ] Use
array_filterto find destinations in the tag[] key of the response where the type isgoogtag.- [ ] Get the tagId for this tag by filtering the
parameterskey for this tag, wherekeyistagId. This should give a tagId in the formG-XXXXXXX. - [ ] Loop through each of these tags, using the new Tag Manager container lookup API, calling
$tagmanagerService->accounts_containers->lookup( array( 'tag_id' => $tag_id ) ). - [ ] Use the Tag Manager destinations API, passing the
accountIdandcontainerIdreturned above like so:$tagmanagerService->accounts_containers_destinations->get("accounts/{account_id}/containers/{container_id}/destinations", array( 'destinationId' => 'G-XXXXXX') )to get a list of destination tags for this container. - [ ] Loop through the returned destinations, check if any of the returned tags destinationIds start with the chars
AW-, return true if so.
- [ ] Get the tagId for this tag by filtering the
- [ ] Use
- [ ] If none of the above conditions are met, return false.
- [ ] First check if the Ads module is connected using the
Datastore changes
- [ ] Update
assets/js/googlesitekit/datastore/site/consent-mode.js, create a new fetch store calledgetRecommendConsentModeto retrieve the values from the new REST endpoint above, like so: https://github.com/google/site-kit-wp/blob/165c8bb73003e7861b3dc2eead677d034def431e/assets/js/googlesitekit/datastore/site/consent-mode.js#L80-L90
Setup CTA Updates
- [ ] Update the render condition for
assets/js/components/consent-mode/ConsentModeSetupCTAWidget.jsto use the newgetRecommendConsentModeselector to render the CTA instead of theisAdsConnectedselector: https://github.com/google/site-kit-wp/blob/165c8bb73003e7861b3dc2eead677d034def431e/assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js#L122-L127
Recommended Badge
- [ ] Update
assets/js/components/settings/SettingsCardConsentMode.jsto use the newgetRecommendConsentModeselector, instead of theisAdsConnectedselector to render the Recommended badge: https://github.com/google/site-kit-wp/blob/165c8bb73003e7861b3dc2eead677d034def431e/assets/js/components/settings/SettingsCardConsentMode.js#L111-L116
Test Coverage
- Create thorough tests for the new endpoint in
tests/phpunit/integration/Core/Consent_Mode/REST_Consent_Mode_ControllerTest.php.
QA Brief
Changelog entry
AC ✔️
Hey @techanvil, I had a deep dive into the Tag Manager docs and API and it appears I can do what is required here using existing APIs. Did I miss a way that GTM can be configured where a tags/containers can be nested in a way that requires us to look up containers to check for Ads linking? If so can you share an example of how to set up tag manager to reach this case and I will update the IB.
Hey @benbowler, thanks for looking into this. The trickier case that you'll need to update the IB for is when the GTM container points to a Google tag, which in turn points to an Ads tag as one of its destinations.
As a prerequisite you'll need a Google tag which points to an Ads tag. You can configure this via the Ads UI:
Or by editing a tag via the entry point on https://tagmanager.google.com/#/home#tags, updating it to point to an Ads tag as a destination:
Then, when creating your container in Tag Manager, set up a tag using the Google Tag type:
Specify the Google tag ID (it may have multiple IDs, you can choose any of them) in the Tag Configuration (or for a more advanced use case you can specify this indirectly via a variable):
When you retrieve the live container version from the API you'll see the tag entry has the type googtag.
You'll need to use the tagId parameter to containers/lookup as mentioned in the Feature Description to retrieve the container for the Google tag, noting its account ID and container ID. Note that you can't rely on the list of tagIds here to determine whether the tag points to an Ads tag, as these will not necessarily map to destination IDs.
Then, use the destinations endpoint to retrieve the list of destinations for the Google tag. These can be examined to determine whether there's an Ads tag as a destination by checking for a destinationId with the prefix AW-:
It's not the most straightforward one and I hope that helps, please drop me a line if you have any further questions!
Hi @benbowler, and apologies, while reviewing the updated IB here I realised that I'd missed a point from the AC, which I've now added. The IB will need an update to accommodate this change:
- The Ads users' variant of the confirmation dialog shown when toggling the Consent Mode switch off on the Settings screen should be displayed when an Ads tag is detected.
Additionally:
- It looks like the intention of the specced
recommend-consent-modeendpoint is to provide the same functionality as the existingisAdsConnected()selector, augmented to check the connected Tag Manager tag when present. However, as specced the endpoint is missing the check for an Ads tag as a destination of the connected Analytics property. Assuming we continue with this endpoint, this logic would need to be included. - I'm not averse to creating a new endpoint to encapsulate all the logic on the backend. We could alternatively take the approach of adding an endpoint just for the Tag Manager checks, and add a check for this to the existing
isAdsConnected()selector. - If we do fully migrate the logic to the new endpoint maybe the endpoint (and corresponding datastore functions) should be called
is-ads-connected,ads-connection-status, or something along those lines, as we might want to use the same check outside of the context of recommending Consent Mode. - All of the existing use of the
isAdsConnectedvalue should be replaced by the new version. I'd suggest we simply update the current selector rather than introduce a new one. - We are adding potentially quite a few additional Tag Manager API calls to every dashboard page load. To mitigate this we should update the
useSelect()inConsentModeSetupCTAWidgetthat retrieves theisAdsConnectedvalue (orrecommendConsentModeif we still go that route) so that it only calls the selector if the other conditions that would prevent the CTA from displaying are all true:
https://github.com/google/site-kit-wp/blob/53da5b5f2855089e4c74fdde73c020aaa344750e/assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js#L77-L79
https://github.com/google/site-kit-wp/blob/53da5b5f2855089e4c74fdde73c020aaa344750e/assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js#L122-L127
- We might want to iterate on this in a future issue to cache the value and only update it say once per hour.
- There is a less common case we should handle where the tag ID is specified via a variable in the Tag Manager container configuration, which I alluded to this in my comment above. We can tackle this in a separate issue - I've created an issue for it and added a mention of this to the AC as well to make it clear.
- For reference, you can see how we do support a simple variable lookup in the
getLiveContainerGoogleTagID()selector.
- For reference, you can see how we do support a simple variable lookup in the
Thanks @techanvil, I'll review and update the IB.
Thanks @benbowler! It's looking good. A few last points:
- On reflection I think
ads-measurement-statuswould be preferable for a REST endpoint name as it's more indicative of a "resource", we can return an object with aconnectedboolean property. This would be along similar lines to the existingtrackingendpoint. The name is also a nod to the CoMo one-pager, see "when does this apply?". - We should check the connected Analytics property before checking Tag Manager as this can allow us to return early without any additional API requests.
- We still need a simple
isAdsConnectedselector to go with the resolver.
In order to help move this along I've updated the IB accordingly myself. Please take a look at my changes, if you're happy with them feel free to move this to the EB.
Thanks Tom, I have reviewed this and moving it to EB.
The current apiclient library we are using includes google/apiclient-services v0.355.0 which does not have updated Tag manager API, so 'lookup' => ['path' => 'tagmanager/v2/accounts/containers:lookup', 'httpMethod' => 'GET', 'parameters' => ['destinationId' => ['location' => 'query', 'type' => 'string']]] definition from our current package. And lookup from this IB point:
Loop through each of these tags, using the new Tag Manager container lookup API, calling $tagmanagerService->accounts_containers->lookup( array( 'tag_id' => $tag_id ) )
Does not work, as tag ID is not supported as parameter in the package we have, hence throwing fatal error.
Updating dependencies under google/apiclient is at that limit of services version, and we cannot update client version due to the minimum php version constrains (any version over current one requires php 8).
Some of the options we have:
- Apply all logic up to this point - meaning all the checks switched to the backend, except for additional lookup inside the
googtagTag, and file a follow up to extend that part once we switch to the generated service dependencies (@aaemnnosttv is experimenting on this) instead of require from external source - Move issue back to backlog until it is unblocked, switching to generated services dependencies, etc, and finish everything afterwards
WDYT? @techanvil @aaemnnosttv
Hey @zutigrm! Thanks for the update, and sorry that we missed the lack of support for the tag_id parameter during definition.
I think your first suggestion makes the most sense - do as much as we can now, and file a followup issue to update the service library and implement the remaining requirements. It will still be an incremental improvement that would be better to deliver sooner rather than later, plus it will result in two smaller issues/PRs rather than one larger one, so it will be easier to review.
@zutigrm did you create the follow up issue you mentioned, otherwise I can create it so it gets tracked.
Added a follow up issue
QA Update: ✅
Verified:
-
I setup Site Kit, and on each setup test following scenarios:
- Connect Ads module - it returns
true - Connect Analytics module that is connected with Ads - it returns
true - Connect Tag manager module, and connect tag with Ads, - it returns
true
- Connect Ads module - it returns
-
Having no Analytics, or Ads, or Tag manager active - it returns
false -
Having Analytics module active, but it is not connected with Ads, and Ads module is not active - it returns
false -
Having Tag Manger active with Tags that have no containers pointing to the Ads - it returns
false -
Consent mode notifications shows as before in settings and in dashboard as setup CTA
Screenshots