Site-kit-wp: Fix inconsistencies in standalone GA tracking script

[felixarntz]

Bug Description

In certain cases where we're not in a full Site Kit context, the plugin uses a "standalone tracking script" (via Tracking::print_standalone_event_tracking_script()) exposing a global window.sendAnalyticsTrackingEvent instead of relying on the sendAnalyticsTrackingEvent function from our JS infrastructure (assets/js/util/standalone.js). The main problem (apart from duplicate code) is that the "PHP-generated" version is different from the main one, and limited, which results in slightly inconsistent data in GA.

We should streamline this so that we only have one piece of code for the tracking script.

I see three possible approaches:

• Either we introduce a googlesitekit-standalone-tracking.js that simply imports the function from standalone.js and exports it on window. This would be close to how our PHP implementation works today, while fixing the inconsistencies. The script would need to rely on googlesitekit_admin (which is globally loaded throughout the admin anyway) to have window._googlesitekitBase available.
• Or we introduce a googlesitekit-tracking.js that going forward would be the _only_ way to send tracking events. It would expose the function on window.googlesitekit. Any code that needs to send tracking events, should then rely on it.
  
  
  
  • It would for example become a dependency of googlesitekit_admin.
  
  
  • The data on window._googlesitekitBase should become a "virtual asset" (see #1004) and then a dependency of googlesitekit_admin and googlesitekit-tracking.
  
  
  • We should also introduce a googlesitekit-gtag asset that enqueues the script, and a googlesitekit-gtag-config "virtual asset" that prints the JS configuration.
  
  
• Or we move the entire responsibility more to JS, with a single googlesitekit-tracking.js that also takes care of adding (or removing) gtag on the fly.
  
  
  
  • In addition to exposing the main tracking function on window.googlesitekit, it should also expose three functions enableTracking(), disableTracking(), and IsTrackingEnabled().
  
  
  • These functions should be used throughout the entire codebase to track events and to react on users opting in to or out from tracking, and to check whether tracking is enabled.
  
  
  • window.googlesitekitTrackingEnabled should become window._googlesitekitTrackingEnabled and only be read to populate the initial value of googlesitekit.isTrackingEnabled().
  
  

---

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

Acceptance criteria

New tracking module

• There should be a assets/js/util/tracking/index.js file.
• The script should export the following functions:
  
  
  
  • enableTracking(): sets the internal flag for tracking enabled to true and, if not yet in the page, dynamically loads the gtag script and configures it (both of this is currently in PHP's Tracking::print_gtag_script(); the tracking ID should be taken from window._googlesitekitBase.trackingID where it is already present)
  
  
  • disableTracking(): sets the internal flag for tracking enabled to false and, if present in the page, dynamically removes the gtag script and ensures the global gtag function is no longer usable
  
  
  • sendTrackingEvent( eventCategory, eventName, eventLabel = '', eventValue = '' ): should basically replace the current sendAnalyticsTrackingEvent function
  
  
  • isTrackingEnabled() returns the current value of the internal flag for tracking enabled
  
  
• On load, the script should run some bootstrap logic to set itself up:
  
  
  
  1. Populate the initial value of the internal flag for tracking from window._googlesitekitBase.trackingEnabled (this value should replace the current window.googlesitekitTrackingEnabled)
  
  
  2. If that initial value is true, immediately call enableTracking() so that gtag is available right away
  
  
• The module file should not handle making API calls to e.g. change the stored setting value - it should be agnostic of the storage mechanism, the setting should be handled by the respective pieces of code that do this already today.
• The module file should not depend on any third-party code, i.e. it should be like the functions in util/standalone.js. This is important so that it can be included in googlesitekit-admin.js without significantly increasing the file size.

Migrating existing logic

• All scripts that currently call sendAnalyticsTrackingEvent() or check window.googlesitekitTrackingEnabled should instead rely on the new module functions.
• All duplicate logic currently in place to set up gtag (from PHP and JS) should be removed and adjusted to rely on the new functions.
• There should no longer be window.googlesitekitTrackingEnabled or sendAnalyticsTrackingEvent (see above).
• The PHP Tracking class should be simplified by removing now unused parts.

Implementation Brief

• Create a new assets/js/util/tracking/index.js module which exports functions making up the internal tracking API
• In addition to those mentioned in the AC, a bootstrap() function should also be exported to set the intial state
  
  
  
  • This function should be called internally when the module is loaded, the funtion is only exported for testing
  
  
  • Call enableTracking() or disableTracking() based on _googlesitekitBase.trackingEnabled
  
  

• enableTracking() should be async, which resolves once the script is on the page and loaded

  • Should inject https://www.googletagmanager.com/gtag/js?id=TRACKING_ID if not present on the page (give this a unique data-googlesitekit-gtag attribute so it is safe to select)

• disableTracking() can simply set the internal flag to false which will prevent any tracking call from being dispatched (no need to touch global gtag function as it isn't Site Kit specific)

Migration

• Remove Tracking::print_gtag_script
• Remove Tracking::print_standalone_event_tracking_script
• Remove sendAnalyticsTrackingEvent from standalone.js
• Update <Optin>
• Update assets/js/googlesitekit-adminbar-loader.js

QA Brief

• Opt-out of anonymous usage stats
• Navigate to a Site Kit screen
  
  
  
  • Ensure there is no script[data-googlesitekit-gtag]
    
    _You can search for this verbatim in the Chrome elements tab
    
    (see screenshot below for example when found)_
  
  
  • window._googlesitekitDataLayer should not exist
  
  
• Navigate to a screen which has a tracking opt-in, such as the splash page or Admin Settings
  
  
  
  • Ensure above script tag is not on the page
  
  
  • Check the checkbox to enable tracking
  
  
  • Without reloading the page, ensure the above script tag IS on the page now
    
    
  
  
  • window._googlesitekitDataLayer should now exist, and contain a few elements
  
  
• Once opted-in, navigate to a Site Kit screen
  
  
  
  • Reload the page
  
  
  • Ensure the above script tag IS on the page now
  
  
  • window._googlesitekitDataLayer should exist, and contain a few elements
  
  

Changelog entry

• Standardize GA tracking snippets and inconsistent data passed in events by introducing a proper tracking API in JavaScript.

[tofumatt]

I'm definitely a fan of the last two approaches—the third one where all tracking is handled by JS and there's one place that takes responsibility for adding/removing/checking for the actual tracking code being loaded makes the most sense to me. It reduces the complexity of using/disabling/enabling the tracking code and will prevent duplicate code.

[aaemnnosttv]

@felixarntz I'm in favor of the pure JS options as well. One question I have though is why would we expose this as a new standalone script/asset or global? We no longer dispatch any tracking events from outside of our React apps, so this could just as easily be handled just as any other module. Is there a reason why it should also be exposed globally?

Also, this issue should address #714 as well.

[felixarntz]

@aaemnnosttv Good point! I see Tracking::print_standalone_event_tracking_script() is still called, but the script is actually no longer used right? In this case, we should indeed eliminate that function. Exposing the tracking script individually was based on me thinking we would still need it in this scenario - if that is not the case, we should drop the method and not have a googlesitekit-tracking.js and instead just implement this module to be used internally.

[aaemnnosttv]

Correct. That function is called but the global function is no longer used as it was only used in the Activation component. All the remaining usage I see is internally in our bundles, so there should be no reason left to keep this around.

Are you suggesting that googlesitekit-tracking.js would then be the name of the internal module? Why not have a assets/js/util/tracking module which exports the functions defined above? I don't think an additional entrypoint is necessary.

[felixarntz]

@aaemnnosttv

Are you suggesting that googlesitekit-tracking.js would then be the name of the internal module?

No I meant that file should not exist, there would just be an internal module like you're suggesting. I've updated the ACs with this.

The only important thing here is that this module should remain free of 3P dependencies, mainly because of its relevance for googlesitekit-admin.js. I wonder whether we can somehow, in the overall project, flag such files easily. What do you think to have an assets/js/standalone folder (so the new module would become assets/js/standalone/tracking)? Maybe there's a more suitable term than standalone, but I think it would be useful in the long term to have a distinct place for pieces of code that should have minimal dependencies (like e.g. what is currently in util/standalone.js).

[tofumatt]

The IB here sounds good, just a few little points to make:

Should inject https://www.googletagmanager.com/gtag/js?id=TRACKING_ID if not present on the page (give this an ID so it is easy to select)

Adding an ID to a script tag, as I recall, creates a global variable with the same name as the ID attribute. It might be safer to use something like data-id or even just data-sitekit="tracking" or something like that, to reduce the chance something like id="gtag" overrides/conflicts with existing global variables.

---

Another question: is there any reason to remove the script instead of just disabling it? When we bootstrap/enable the script dynamically adding it to the page makes sense, but once we've added it there's not much benefit to removing the script for the rest of the life of the page over disabling the tracking events from being sent—that part is already done in our API, so it might reduce complexity not to bother removing the script tag.

[aaemnnosttv]

Thanks for the suggestion @tofumatt. I'll update the IB to use a different selector strategy.

As for your other question, I did mention this in the IB

• disableTracking() can simply set window.gtag to a no-op function
  
  
  
  • No need to remove gtag js as it is already loaded in memory if on the page
  
  

[tofumatt]

Ah, whoops, I must've just missed it, my bad! Though setting window.gtag to something else might make re-enabling the script after it's been disabled awkward—we'll be overwriting someone else's global.

I think it would be safest/less complex if disableTracking() only prevented our code from calling out to the window.gtag APIs, but didn't modify them in any way.

[felixarntz]

IB ✅

[cole10up]

Tested

Setup SK without opt-in for tracking

Navigated to SK dashboard and searched for script[data-googlesitekit-gtag]

Navigated to Admin settings and searched for script[data-googlesitekit-gtag]

Navigated to admin and confirmed opt in without refreshing page shows the tag

Navigated to SK dashboard and confirmed script is there

Passed QA ✅