Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend Consent Mode conditions for determining Ads connection status. #8382

Closed
10 tasks
techanvil opened this issue Mar 12, 2024 · 6 comments
Closed
10 tasks
Labels
P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@techanvil
Copy link
Collaborator

techanvil commented Mar 12, 2024

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:

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-)

...

GTE Note: we can't look up a tag's destinations by tag ID as we'd want to do here (i.e. check the destinations of tag ID we place). We can only look up a container by a destination ID, or look up destinations by a container. GTE is odd in that it is possible to have a Google tag which is also not a destination ID so we cannot assume this either.

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

  • The concept of an Ads link should be introduced to Site Kit, and the status of this (linked or not linked) should be refreshed for the connected Analytics property on a regular basis, with at least 24 hours between each refresh. In other words, it should be updated in a similar manner to the existing AdSense link status.
  • Within the Consent Mode feature, the conditions for determining whether Ads is connected should include a check for the presence of an Ads link. That is to say, Ads should be considered connected if either an Ads Conversion ID is present, an Ads link is detected, or both of these are true. This in turn means that:
    • The Setup CTA Banner should be displayed when an Ads link is detected (while the Consent Mode enabled setting is false and 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 link is detected.

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

  • Within the Analytics_4 module:
    • Add new adsLinked and adsLinkedLastSyncedAt settings.
    • Add a new GET:ads-links datapoint, similar to GET:adsense-links, with the following key differences:
      • Use the following call in create_data_request() to retrieve the links:
        • $this->get_service( 'analyticsadmin' )->properties_googleAdsLinks->listPropertiesGoogleAdsLinks()
      • Use $response->getGoogleAdsLinks() in parse_data_response() to return the links.
    • Include the adsLinked and adSenseLinkedLastSyncedAt settings in the Site Health section by adding them to get_debug_fields() in a similar manner to the AdSense settings:
      $debug_fields['adsense_linked'] = array(
      'label' => __( 'AdSense Linked', 'google-site-kit' ),
      'value' => $settings['adSenseLinked'] ? __( 'Connected', 'google-site-kit' ) : __( 'Not connected', 'google-site-kit' ),
      'debug' => Debug_Data::redact_debug_value( $settings['adSenseLinked'] ),
      );
      $debug_fields['adsense_linked_last_synced_at'] = array(
      'label' => __( 'AdSense Linked Last Synced At', 'google-site-kit' ),
      'value' => $settings['adSenseLinkedLastSyncedAt'] ? gmdate( 'Y-m-d H:i:s', $settings['adSenseLinkedLastSyncedAt'] ) : __( 'Never synced', 'google-site-kit' ),
      'debug' => Debug_Data::redact_debug_value( $settings['adSenseLinkedLastSyncedAt'] ),
      );
    • Ensure these new settings are reset when the property changes, again similar to the AdSense versions:
      // Reset AdSense link settings when propertyID changes.
      if ( $old_value['propertyID'] !== $new_value['propertyID'] ) {
      $this->get_settings()->merge(
      array(
      'adSenseLinked' => false,
      'adSenseLinkedLastSyncedAt' => 0,
      )
      );
      }
  • Add a new Synchronize_AdsLinked class which is based on Synchronize_AdSenseLinked, synchronizing the adsLinked setting and updating adSenseLinkedLastSyncedAt via a cron job with a 24 hour schedule.
    • Note that the conditions for scheduling the job only needs to include a check for the analytics-4 module being connected, it doesn't need to include a check for the ads module being connected.
    • Register an instance of this class in Analytics_4::register().

Client-side

  • Add adsLinked to the list of settings slugs for the JS analytics-4 datastore.
  • Include a check for adsLinked being true when determining isAdsConnected in the following locations:

Test Coverage

  • Add PHPUnit test coverage to the Analytics_4 class, this can follow the existing coverage for adSenseLinked and adSenseLinkedLastSyncedAt settings.
  • Add PHPUnit test coverage for the new Synchronize_AdsLinked class.
  • Fix/update VRT tests as needed.

QA Brief

  1. Set up SK with the Analytics module.
  2. Make sure you do not have an Ads conversion ID set.
  3. Run the following in the browser console googlesitekit.data.select( 'modules/analytics-4' ).getSettings() and verify that adsLinked is set to false.
  4. Link Google Ads with your configured Google Analytics property following this guide.
  5. It will need Site Kit around 24 hours to re-check if Ads is linked with Analytics. If you intend to expedite the wait time, you can use the "WP Crontrol" plugin to run the googlesitekit_cron_synchronize_ads_linked_data cron event immediately.
  6. After around 24 hours, or after manually running the cron event, the adsLinked status should now be updated. Refresh the SK dashboard and re-do step 3 to verify that adsLinked is now set to true.
  7. Verify that the Enable Consent Mode to preserve tracking for your Ads campaigns CoMo banner now appears in your SK dashboard.
  8. Verify that the "Recommended" badge now appears in Site Kit Settings -> Admin Settings -> Consent Mode.

Changelog entry

  • Add a linked Ads account as an option to surface Consent Mode features.
@techanvil techanvil added P1 Medium priority Type: Enhancement Improvement of an existing feature labels Mar 12, 2024
@techanvil techanvil added P0 High priority and removed P1 Medium priority labels Mar 21, 2024
@techanvil
Copy link
Collaborator Author

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.

@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Mar 21, 2024
@techanvil techanvil assigned techanvil and aaemnnosttv and unassigned techanvil Mar 21, 2024
@aaemnnosttv
Copy link
Collaborator

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.

@aaemnnosttv aaemnnosttv removed their assignment Mar 25, 2024
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Mar 26, 2024
@techanvil
Copy link
Collaborator Author

Thanks @aaemnnosttv, good spot there. I've added an additional point to the IB to make sure we don't miss this.

@nfmohit nfmohit self-assigned this Mar 27, 2024
@nfmohit nfmohit removed their assignment Mar 27, 2024
@tofumatt tofumatt assigned tofumatt and nfmohit and unassigned tofumatt Mar 27, 2024
@nfmohit nfmohit assigned tofumatt and unassigned nfmohit Mar 29, 2024
@tofumatt tofumatt assigned nfmohit and unassigned tofumatt Mar 31, 2024
@nfmohit nfmohit assigned tofumatt and unassigned nfmohit Apr 1, 2024
tofumatt added a commit that referenced this issue Apr 1, 2024
Consider Analytics Ads linked status in Consent Mode conditions
@tofumatt tofumatt removed their assignment Apr 1, 2024
@mohitwp mohitwp self-assigned this Apr 2, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Apr 2, 2024

QA Update ⚠️

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.

image

image

image

@nfmohit
Copy link
Collaborator

nfmohit commented Apr 2, 2024

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!

@nfmohit nfmohit removed their assignment Apr 2, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented Apr 3, 2024

QA Update ✅

  • Tested on dev environment.
  • Verified that by running manual running cron googlesitekit_cron_synchronize_ads_linked_data .
  • Verified when Analytics don't have Ads conversion id.
  • Verified that if adsLinked : True then CoMo banner is appearing on Dashboard and recommended badge is showing under settings.

image

image

image

Recording.853.mp4

@mohitwp mohitwp removed their assignment Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants