-
Notifications
You must be signed in to change notification settings - Fork 275
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
Create Proxy Event Tracking Function to Add Site Kit as Source for All ICE Events #8641
Comments
From the context/ACs here, would there be a separate, inline script if the user was using two ICE scripts (eg. using two ICE-supported plugins)? Why output the script inline instead of having a single, common script file we build and include whenever any ICE JS script is used? |
@tofumatt as per out comms offline, I think it makes sense to:
I'll update the AC accordingly. |
Just one question really, when you say a “proxy function”, do you mean an actual JS Proxy? Or do you just mean that our scripts should include/reference the new trackEvent function and use it to send events? If it’s the latter, the AC should specify that it should be available (on a Site Kit JS global I’d think). If the former: why? 😅 Proxies when it’s just for our code seems odd. Maybe it’s just a wording thing that’s tripping me up here 😆 |
@tofumatt no no, not an actual proxy :) I simply meant a helper function that abstracts our calls to |
FWIW, I think we should avoid using |
Sounds good, moving to IB 👍🏻 |
I do not think this is correct, as Conversion Tracking is not specific to the Ads module, and can be turned on from GA4 without the ads module active, and vice-versa. I think it may be better to register this script from within the |
@10upsimon Yes that makes more sense. Thanks. IB updated |
Thanks @zutigrm IB ✅ |
Flagging here that @eugene-manuilov doesn't update the tracking for MailChimp or Popup Maker as these weren't merged at the time this ticket was started. Should we hold back this ticket and address that here when these two are merged or update those separately? |
@benbowler I think it makes sense to add those here. I'll do that in the PR, it's not a big deal to include 🙂 EDIT: Never mind, they were included later 🙂 |
QA Update
|
@mohitwp yes, I can confirm that the source appears for the |
QA Update ✅Thanks @wpdarren !
Note : As mentioned on #8553 (comment) for WooCommerce only 'add to cart' event is getting tracked. Optin monster WP Forms MailChimp Popup Maker WooCommerce - |
Noting that the source parameter may change but it is currently implemented as defined. One thing from the AC
All the provider scripts should be loaded using |
Feature Description
As part of the initial event conversion infrastructure work, the need to add an event parameter citing Site Kit as the source of events tracked as part of this epic has been identified.
Given the decoupled nature of the event tracking infrastructure - where supported plugins have their own JS files - it has been discussed that the simplest way to achieve this would be via a proxy style function that is defined in a script that in turn is a dependency to individual plugin script enqueues, much like
Script_Data
functions. Note, this is not an actual proxy function, merely a helper function abstracting our calls togtag
when we track events.This body of work requires the need to define said function in a self contained script asset, set it as a dependency for all singular plugin JS scripts and update all individual plugin JS scripts to make use of this new helper function. This helper function should be define against a globally available Site Kit specific var, such as
_googlesitekit
, so that all plugins requiring it can access it easily from said global.Said function should accept an event name and an event data object. Said event data object will then be merged so that it always contains the a
_source
property with a value of'site-kit'
.A proof-of-concept function was shared as follows:
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
gtag
and ensure thatsite-kit
is added as a_source
event parameter value for all events tracked via this proxy/helper function_googlesitekit
varImplementation Brief
assets/js/track-conversion-event.js
trackConversionEvent
and attach it to the_googlesitekit
global, exampleglobal._googlesitekit.trackConversionEvent
name
parameter - representing a conversion event name, and adata
parameter which holds event data object with any additional details conversion provider might include.gtag
function, using passed parameters, including_source: 'site-kit'
to the event data (it should be merged with data parameter if present). You can check example POC in the issue description.includes/Core/Conversion_Tracking/Conversion_Tracking.php
register_event_tracking_script
Script
including the above JS file and return the instance. It should include'execution' => 'async'
in the arguments.register
method under existingwp_enqueue_scripts
hook, before the scripts from active providers are enqueued:conversionInfra
feature flag is enabled retrieve the tracking script instance withregister_event_tracking_script
method and invokeregister_script()
andenqueue()
on it.global._googlesitekit.trackConversionEvent
and individualGoogle\Site_Kit\Core\Conversion_Tracking\Conversion_Events_Provider::register_script
method should include'execution' => 'async'
in the arguments ofScript
, and haveassets/js/track-conversion-event.js
as dependency.webpack/basicModules.config.js
webpack partialassets/js/track-conversion-event.js
to theentry
Test Coverage
QA Brief
_source: "site-kit"
.Changelog entry
The text was updated successfully, but these errors were encountered: