Populate Google Tag data for existing site using GA4

[felixarntz]

Based on #6077 and #6080, for the GA4 snippet, a Google Tag ID should be used going forward. While this usually is the same as the GA4 measurement ID, we will need to migrate all sites to have this tag ID determined and stored. While #6081 implements this for the "new user" / "updating configuration" scenarios, we also need to have logic that populates the data for existing sites where GA4 is already active and fully configured.

---

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

Acceptance criteria

If the gteSupport feature flag is enabled, and only with the Analytics / GA4 module active:

• If the Analytics_4 module setting googleTagID is empty, the Site Kit dashboard should trigger an API request to determine and store it.
  • The logic can be entirely reused from Add Analytics setup/settings logic to determine Google Tag settings #6081, where this was already implemented for the more "organic" use-case.
  • The issue here is purely to populate the same data for sites when they are already configured with a prior implementation of the GA4 module without the GTE support. Eventually, this logic could be removed.
• The request should be cached in a way that this logic won't be attempted over and over again in case the request fails. It should typically only happen once anyway since the request is expected to always succeed, but such a safe guard needs to be put in place so that at least in case of failure Site Kit only attempts it once per hour (based on our regular client-side cache expiration).

Implementation Brief

Expose googleTagLastSyncedAtMs timestamp to facilitate request throttling

Within includes/Modules/Analytics_4/Settings.php, in get_default():

• Add the setting googleTagLastSyncedAtMs with the default value 0.

Within assets/js/modules/analytics-4/datastore/base.js:

• Add googleTagLastSyncedAtMs to the list of settings in settingSlugs.

Create *syncGoogleTagSettings() action

Within assets/js/modules/analytics-4/datastore/properties.js:

• Create a new generator action *syncGoogleTagSettings(). Within this action:
  • Retrieve the registry via yield Data.commonActions.getRegistry().
  • Add preliminary checks to verify the following before executing the action further:
    • The gteSupport feature flag is enabled - isFeatureEnabled( 'gteSupport' ).
    • Analytics 4 is connected - isModuleConnected( 'analytics-4' ).
    • The googleTagID setting is empty - getGoogleTagID().
    • The measurementID setting is not empty - getMeasurementID().
    • The current time as returned from Date.now() is one hour or more later than the time stored in the googleTagLastSyncedAtMs setting, or the googleTagLastSyncedAtMs setting is empty - getGoogleTagLastSyncedAtMs(), HOUR_IN_SECONDS. Note googleTagLastSyncedAtMs is in milliseconds, just as Date.now()
  • Retrieve the Google Tag settings for the current measurement ID by yielding the fetchGetGoogleTagSettings() action.
  • Update the googleTagAccountID, googleTagContainerID and googleTagID settings by dispatching their individual setter actions.
  • Update the googleTagLastSyncedAtMs setting with the value of the current time as returned by Date.now().
  • Save the Analytics 4 settings - saveSettings().

Call *syncGoogleTagSettings() from the Dashboard

Within assets/js/components/DashboardEntryPoint.js:

• Add a useMount() hook that dispatches the *syncGoogleTagSettings() action.

Test Coverage

• Add tests for *syncGoogleTagSettings().
• Ensure the tests for DashboardEntryPoint still work and consider adding a test here too.
• Fix any other failing tests.

QA Brief

• Set up Site Kit.
• Using the tester plugin, make sure the gteSupport feature flag is disabled.
• Connect the Analytics module with a GA4 property having Google Tag settings.
• If you run the Analytics 4 getSettings() selector at this point (see below), you'll see the Google Tag settings are not yet populated.
• Now, enable the gteSupport feature flag.
• The Dashboard should now display a notification to redo the setup for Tag Manager, this is because the Tag Manager readonly scope is needed for GTE and for the settings sync. Click on Redo setup and go through the OAuth flow.
• Upon returning to the Site Kit dashboard, SK should now internally populate the Google Tag settings.
• In the browser console, run the following code:

googlesitekit.data.select( 'modules/analytics-4' ).getSettings()

• In the response returned, verify that the Google Tag setting properties (e.g. googleTagID) are populated.

Changelog entry

• Ensure Google Tag data is populated for sites which already have Google Analytics 4 configured.

[techanvil]

Hi @nfmohit, this IB is looking good. I would suggest one small change. In the Test Coverage section, you have specified the following point:

• Add test case for the new sync_google_tag_data() method.

However, sync_google_tag_data() will be a private method, so it's not directly testable (via normal invocation, although there is a way round it). To avoid confusion so the implementer doesn't attempt to directly test a private method (we actually do in a handful of places, but it's not recommended), I'd suggest changing to something like "add PHPUnit test coverage for the new changes" or such like.

[nfmohit]

Good catch. I've updated the IB. Thank you, @techanvil !

[techanvil]

Thanks @nfmohit!

IB ✅

[mxbclang]

NOTE: This issue is blocked by #6081 in Sprint 92.

[aaemnnosttv]

Thanks @techanvil, the approach you've outlined here would work well I think, I just have a few thoughts for your consideration.

After thinking about this a bit more, using client-side cache alone would not be sufficient to prevent the request from being made more than once per hour. It would more/less accomplish this for the current user, but because client-side cache is per user, and now even per WP session, it could happen more frequently, especially for a site with multiple concurrent users.

Also, since the relevant data we're checking is not user-specific, but specific to the tag configuration, it would make sense to manage the state in a more centralized location.

Rather than introducing some separate value (as the transient was used for initially), we stored a googleTagLastSyncedAt timestamp to the GA4 module settings? Then, even in a concurrent user scenario, the rate at which this is refreshed should not be more than once per hour and it should require less substantial additions to backend or frontend structures.

The client side would then only need to read that timestamp and trigger a sync if the current time was outside of the 1 hour threshold.

Call *syncGoogleTagSettings() from the Dashboard

Within assets/js/components/DashboardEntryPoint.js:

• Add a useMount() hook that dispatches the *syncGoogleTagSettings() action.

A minor suggestion here – this reads as a good opportunity for a custom hook, something like useSyncGoogleTagSettingsEffect. This also provides a place to potentially move some/all of the conditional logic from the syncGoogleTagSettings action into the hook, although it could all stay in the action with the hook as a utility for encapsulation.

Regarding your ideas around wiring up actions to our cache API is something I've been thinking about as well, so +1 to that. That part might not be necessary if you agree with the suggestions above but let's make sure we capture this in some new issues either way.

[techanvil]

Thanks @aaemnnosttv, great suggestion regarding the centralized timestamp, I've updated the IB to make use of a googleTagLastSyncedAt setting as suggested. I've also created issues #6472, #6473 and #6474 to capture the ideas about the cache API.

The only thing I'm not so sure about is the idea of creating a custom useSyncGoogleTagSettingsEffect hook. The DashboardEntryPoint component is so small, it seems a bit premature to extract a hook which itself is going to only contain a single action dispatch. I don't think we need it for code reuse and without containing any logic it would just add a layer of indirection to understanding what action is being dispatched. If DashboardEntryPoint was one of our overly large components then I'd be more in favour of it, but I don't think it's really worthwhile at this point. I don't think it's really necessary for the sake of moving conditional logic either, as without a clear case for code reuse, we'd just have to refer to two places to understand the flow rather than one, and it seems better to keep the logic within *syncGoogleTagSettings, IMHO.

Also, it's worth bearing in mind that the AC does mention that "Eventually, this logic could be removed"...

[aaemnnosttv]

Thanks @techanvil ! IB overall reads great to me, just one last question.

One thing I'm thinking about is the format of the googleTagLastSyncedAt value in case we ever wanted to use this in PHP too, since PHP references the epoch in seconds where as Date.now is in milliseconds. We could potentially convert the integer into seconds when saving, and back to millis when passing to the client, or we could use a datetime string which might be less vulnerable to mistakes in translation or errors in interpretation. What do you think?

[mohitwp]

QA Update ⚠️

@nfmohit

• Tested on main.
• Followed the QAB and verified the response for googlesitekit.data.select( 'modules/analytics-4' ).getSettings().
• Connected the Analytics module with a GA4 property having Google Tag settings.

Q-1) In response Google Tag setting properties (e.g. googleTagID) are showing without any value, is this expected ?

Q-2) On enabling 'gteSupport' feature flag Additional permission required Pop up appears. Is this expected ?

cc @techanvil

Recording.242.mp4

[techanvil]

Thanks for raising this @mohitwp. I've investigated, and it's evident that following the QAB as detailed does not result in the GTE settings being immediately updated. Arguably though, this is following the spec to the letter, but this leads to a question for @felixarntz about whether we should change things to make it behave more intuitively.

Following the QAB, setting up Site Kit with Analytics but without the gteSupport feature flag enabled, and then enabling the gteSupport flag on the Dashboard, the *syncGoogleTagSettings() action is triggered. This results in a request for the GTE settings in order to sync them, however the request returns a missing_required_scopes error as the user has not yet granted the https://www.googleapis.com/auth/tagmanager.readonly scope.

Due to the way *syncGoogleTagSettings() handles this error (i.e. to ignore it, which is in keeping with the spec, in theory), this results in the googleTagLastSyncedAtMs timestamp being set to the current time, while the "Additional Permissions Required" modal is displayed. When the user proceeds with and returns from the OAuth flow, *syncGoogleTagSettings() is invoked again but bails out as the timestamp has just been set, so as far as the action is concerned, it's just made an attempt to sync settings and should not try again for another hour.

As mentioned this is in keeping with the spec but it raises the question, should we in fact make an exception for the missing_required_scopes error and not update the timestamp in this case, to ensure the sync runs again fully when the user returns from OAuth, so the GTE settings are updated at this point? Otherwise, despite requesting the scope from the user, no action will be taken until an hour has gone by and they hit the Dashboard again.

It seems to be we probably should do this, but we should also consider that if the user does not grant the scope, maybe is unable to for some reason, we could end up in a situation where the sync attempt & scope request is made on every Dashboard load, which is something we're trying to avoid as per the spec.

Alternatively we could add a bit more logic in so the sync is specifically reattempted only when returning from the OAuth flow via a store snapshot or similar...

@felixarntz, please can you advise on how you want to proceed with this one?

[felixarntz]

@techanvil Great catch! Thinking about this further, we may be best off not attempting any of the syncing logic if the user hasn't granted the scope yet.

A few considerations here:

• The scope will only be missing for existing plugin users that previously were using Analytics but not Tag Manager. New users will be prompted to grant both scopes right away, so this is only a problem for existing users (which of course are a lot, so it is definitely important to improve this).
• Since the new Tag Manager scope will be an Analytics base scope, users will already see a banner notification immediately after activating the feature flag (see Enhance experience for existing users to grant new Tag Manager scope for GTE #6421), so they should (hopefully) grant the scope very quickly.
• I think we should check before even attempting the sync whether the current user has already granted the scope. If not, we already know ahead of attempting it that it will fail, so in that case we should not attempt it and thus also not set the timestamp that is associated with it.

This also avoids the somewhat ugly experience of being on the dashboard and suddenly seeing this popup without even having invoked any action. Does that sound reasonable and would work well with our implementation?

[techanvil]

This also avoids the somewhat ugly experience of being on the dashboard and suddenly seeing this popup without even having invoked any action. Does that sound reasonable and would work well with our implementation?

Thanks @felixarntz - sounds like a good solution. This was a quick fix, I've created a followup PR and tweaked the QAB accordingly, cc @mohitwp.

[mohitwp]

QA Update ✅

• Tested on main.

• Followed the QAB and now Google Tag setting properties (e.g. googleTagID) are populated after enabling gteSupport Feature flag.

• Populated googleTagID is same TagID which is showing as connected Tag Id in Analytics account.

• The Dashboard now displays a notification to redo the setup for Tag Manager.

• Note—_I'm not sure from where we are fetching googleTagAccountID and googleTagContainerID ? I verified tag container ID and account ID, of the connected tag container, but that is different. Confirmed with @nfmohit that it is fetching expected response, might be I'm not looking at right place.

cc @techanvil @aaemnnosttv

When gteSupport Feature flag is not enabled -

When gteSupport Feature flag is enabled -

Recording.250.mp4