-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Why does the GTM adapter rewrite top-level event schema in trackEvent? #438
Comments
To be honest, I haven't the foggiest clue. If you look at the first commit with this code you'll see that it's pretty much been there since its inception: 86d7e12. I'm not a GTM expert so I can't comment on why this might be desirable. I think if you can craft a path forward that offers folks a means of opting into this behaviour we can merge it. |
I should not that it should be done in two phases:
|
Hey @jherdman, I've finally gotten around to this! I've opened the deprecation PR as you can see above. I have the commit for the new behavior ready as well, but I wasn't sure if I should wait to open that one. Don't want to flood you with PRs! |
@nbenz Since your 2nd PR is not merged yet, I was wondering how you currently make it all work? My current implementation to track purchases looks like this: this.metrics.trackEvent({
event: 'purchase',
transaction_id: purchase.id,
value: purchase.total,
currency: purchase.currency,
items: [
{
item_id: purchase.itemCode,
item_name: purchase.label,
},
],
}); Then in GTM, I had to configure 4 user-defined data layer variables:
When you say "This breaks Enhanced Ecommerce tracking as it looks for the ecommerce key specifically." are you referring to the fact that you have to map variable names manually in GA4? |
@gzurbach sorry, I missed this comment. I basically just implemented what I have in my PR on my local branch. You can override the existing adapter by importing it into What you did sounds like it would also work, but I hated the idea of remapping event keys in some other system to account for some weird behavior in my JS. Much more robust to just send the correct data in the first place. |
I'm referencing this block here. All the top-level keys are rewritten with the prefix
event
. This breaks Enhanced Ecommerce tracking as it looks for theecommerce
key specifically.Obviously I can easily override the adapter to not do this, but I'm wondering if there's even a good reason to do it in the first place. It seems like, all things being equal, the adapter should just write the schema I told it I wanted. As far as I can tell the code has been there since the introduction of the adapter and there was no explanation for it in the relevant PR.
The text was updated successfully, but these errors were encountered: