-
Notifications
You must be signed in to change notification settings - Fork 218
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
Add Google Analytics 4 and Universal Analytics Ecommerce translation layer #1170
Add Google Analytics 4 and Universal Analytics Ecommerce translation layer #1170
Conversation
This looks good to me, adding @jethron to have a look through too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like handy wrappers, thanks @igneel64! I think the single object arg vs multiple args is the only big thing I'd change. Maybe currency after that.
|
||
export function transformUAProductsToSPProducts({ | ||
products, | ||
currencyCode = 'USD', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a reasonably sane default, but it's unfortunate that it has different semantics to undefined currency in GA... I think currencyCode was a relatively late addition to the ecommerce
definition so a few implementations will not have it there and just use their single property-level currency so this isn't gonna be very compatible.
Would be nice if you could pass in a defaultCurrencyCode
or something when you install the plugin and have that be used instead, probably a broader scope than this PR though. Failing that, it's annoying but maybe this should be an optional param in all the callers of these util functions so it can at least be easily hardcoded in GTM without needing a dataLayer update (which may involve a different team for our users).
export function trackGA4SelectPromotion(ecommerce: GA4EcommerceObject) {} | ||
export function trackGA4ViewCart() {} | ||
|
||
export function trackGA4AddToCart(ecommerce: GA4EcommerceObject & Currency, finalCartValue: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to follow the normal v3 idiom of just taking a single object parameter instead of multiple ones?
Currently we lose the ability to specify contexts etc that the wrapped trackAddToCart
function & co has.
Maybe that's a good incentive to do a native implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeap, as advertised in the accelerator page, this is a 'quick' way to migrate your most important GA ecommerce events to snowplow using these wrappers. If you are at a level of custom contexts on ecommerce events, then I think it is a good place to start the native migration.
I think this would be the point where the user would like to add their custom dimensions in a context, sent together with the standard snowplow ecommerce attributes.
Kinda torn again, it would be really easy make the second argument an options
object which would include the required attributes, where needed e.g. finalCartValue
, and the addition of custom contexts and tracker selection (will probably do that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jethron
Added a simple options argument which for now reference the currency where required and any other attribute which is required in some cases. I believe that for now it is good enough for the simple translation layer and the extra contexts etc. could be a ticket to the native implementation.
ad02159
to
90375ab
Compare
BundleMonFiles added (6)
Total files change +94.09KB 0% Final result: ✅ View report in BundleMon website ➡️ |
90375ab
to
223930c
Compare
223930c
to
1877ba8
Compare
d70353b
to
cb13d3b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there... 😛
… of the possible ways
cb13d3b
to
7bf4d6d
Compare
Additional plugin utility functions to be paired up with the GA4-UA-ee capability https://deploy-preview-14--snowplow-axl-ecom.netlify.app/accelerators/ecommerce/tracking/ua_ga4_migration/