Closed techanvil closed 7 months ago
I've raised the priority of this one as it appears that users who have Ads linked with their Analytics account are seeing a drop off in tracking without Consent Mode being enabled. Implementing this issue will ensure the setup banner on the dashboard, and the recommendations on the Settings screen are shown to more Ads users and help to mitigate this problem. See the related Slack conversation.
Thanks @techanvil !
AC + IB ✅
One thing I noticed which relates to the issue here are the debug field definitions for the AdSense links which lack the "Analytics" prefix that all of its fields should have. Otherwise, it's not clear what module the fields are for. We could fix this for the AdSense fields here since we're touching the same part, or in a separate issue, I just thought I'd flag it here so we don't replicate that omission.
Thanks @aaemnnosttv, good spot there. I've added an additional point to the IB to make sure we don't miss this.
Tested on dev environment.
@nfmohit I tested this using oi.ie site and manually trigger cron to link analytics and AdSense. AdSense link status is showing true but banner is not showing on main dashboard and Recommended" badge not appearing in Site Kit Settings -> Admin Settings -> Consent Mode.
Let me know if I'm missing something.
Thank you for sharing your observations, @mohitwp.
I think we're having a little confusion between "Ads" linked & "AdSense" linked. We're working with adsLinked
in this issue, and based on your screenshot, it looks like it isn't linked yet, i.e. adsLinked
is false
.
Could you ensure that it is actually linked and the cron job mentioned in the QAB is ran? Thank you!
googlesitekit_cron_synchronize_ads_linked_data
.
Feature Description
For the Consent Mode MVP, we implemented a simple check for Ads being connected which is to check for the presence of the Ads Conversion ID.
We should extend these conditions to include the additional checks discussed under "when does this apply?" in the one-pager, to the extent that it's practical to do so.
As described in the "when does this apply?" section:
With the above in mind, we should extend the conditions to include the check for the Ads link, and we should also include a check for an Ads tag being a destination of a connected Google tag if we can determine a sensible way to do so.
Update: Due to the limitations of the Tag Manager API mentioned above whereby we can't lookup a tag's destinations by tag ID, we're only going to be able to manage the Ads link part of this in the short term. I (@techanvil) have created a feature request to add an endpoint to look up a container by one of its tag IDs, which would then allow us to lookup the destinations for that container via the existing
destinations.list
endpoint.Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
enabled
setting isfalse
and the banner has not been dismissed).Implementation Brief
Note that this is basically copying the approach we take for the AdSense link status. It should be OK for now, but we might want to follow up later with some sort of abstraction around these so we don't keep repeating ourselves.
Server-side
Analytics_4
module:adsLinked
andadsLinkedLastSyncedAt
settings.GET:ads-links
datapoint, similar toGET:adsense-links
, with the following key differences:create_data_request()
to retrieve the links:$this->get_service( 'analyticsadmin' )->properties_googleAdsLinks->listPropertiesGoogleAdsLinks()
$response->getGoogleAdsLinks()
inparse_data_response()
to return the links.adsLinked
andadSenseLinkedLastSyncedAt
settings in the Site Health section by adding them toget_debug_fields()
in a similar manner to the AdSense settings: https://github.com/google/site-kit-wp/blob/c174714d3d389772fb85a201de567cbb240589e0/includes/Modules/Analytics_4.php#L429-L439ads
module being connected, as we're interested in the link status regardless of the Ads module connection status.Synchronize_AdsLinked
class which is based onSynchronize_AdSenseLinked
, synchronizing theadsLinked
setting and updatingadSenseLinkedLastSyncedAt
via a cron job with a 24 hour schedule.analytics-4
module being connected, it doesn't need to include a check for theads
module being connected.Analytics_4::register()
.Client-side
adsLinked
to the list of settings slugs for the JSanalytics-4
datastore.adsLinked
being true when determiningisAdsConnected
in the following locations:ConsentModeSetupCTAWidget
component: https://github.com/google/site-kit-wp/blob/c174714d3d389772fb85a201de567cbb240589e0/assets/js/components/consent-mode/ConsentModeSetupCTAWidget.js#L72-L75SettingsCardConsentMode
component: https://github.com/google/site-kit-wp/blob/c174714d3d389772fb85a201de567cbb240589e0/assets/js/components/settings/SettingsCardConsentMode.js#L50-L53Test Coverage
Analytics_4
class, this can follow the existing coverage foradSenseLinked
andadSenseLinkedLastSyncedAt
settings.Synchronize_AdsLinked
class.QA Brief
googlesitekit.data.select( 'modules/analytics-4' ).getSettings()
and verify thatadsLinked
is set tofalse
.googlesitekit_cron_synchronize_ads_linked_data
cron event immediately.adsLinked
status should now be updated. Refresh the SK dashboard and re-do step 3 to verify thatadsLinked
is now set totrue
.Enable Consent Mode to preserve tracking for your Ads campaigns
CoMo banner now appears in your SK dashboard.Changelog entry