-
-
Notifications
You must be signed in to change notification settings - Fork 437
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
Added order tracking for Google Analytics 4 #3092
Conversation
Possible to add tax and category information? |
I've added the tax amount but I wouldn't add the categories, the trees can be too different from store to store (very deep, with categories with the same names across branches etc) so IMHO we'd some configuration options and multiple implementations for that. I'll copy paste some code I wrote as a test:
but I'm not committing it, I think we shouldn't have the category for the time being. |
@loekvangool could you test this? |
This PR is the reason why I won't even take a look at #3121, you develop something that somebody asked, you update it based on someone else's feedback and then nobody cares taking a look at it. neither the person who asked for it nor the other one that asked for modifications... What a great waste of time. |
It's not a waste of time, unfortunately we lack people to allocate a little of their daily time for this project. I wouldn't push things, because there are still a few of us who are moving something around here. I also felt a frustration last year when for a while I didn't see any approved PR. |
Me? I gave thumbs up ... ?!? |
The base branch was changed.
GA3 stop july. Is possibile approve this before july? |
@matteotestoni if you review it (clicking on the "files changed" and then "review" button) you can make the difference, with 3 reviews with non-admins we can still merge! |
done |
I've to rebase it on the main branch 🤞 |
Based on the image above I would like to discuss a few questions
Testing this PR, the script is found in the page source code. Unfortunately I can't make the changes in a production server that I already manage to check what is displayed in the GA interface. <!-- BEGIN GOOGLE ANALYTICS 4 CODE -->
<script async src="https://www.googletagmanager.com/gtag/js?id=ADDISON74"></script>
<script>
window.dataLayer = window.dataLayer || [];
function gtag(){dataLayer.push(arguments);}
gtag('js', new Date());
gtag('config', 'ADDISON74');
</script>
<!-- END GOOGLE ANALYTICS 4 CODE --> |
in the future i'd still leave the select, for an eventual GA5 or whatever. the IP anonymization I've used it in the past. it's not necessary for GA4 since it's the default and only way the data is sent. |
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 approved it because this change must be made ASAP. I appreciate that Google Analytics 4 is the first option in the list. If there will be any issue, I have no doubt that they will not be reported and solved.
merging for 2 gray checks and 1 green. cherry picked on v19 too since the ga4 implementation wasn't complete but was already merged there. |
i've this same code on a customer's website since a while |
In #3090 it was pointed out that the previous UA/GA implementation supported the order data tracking, while the GA4 implementation isn't.
This PR aims to add the order tracking to GA4.
Fixed Issues (if relevant)
Manual testing scenarios (*)