-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: add parity between ga4 and gtm for v1
#975
feat: add parity between ga4 and gtm for v1
#975
Conversation
- update fields on `order_completed` event contract of GA4 and Omnitracking integration to add new fields.
- @farfetch/[email protected] - @farfetch/[email protected]
This updates the github release title format to remove the `Release` string from it and use only the tag name as it will be the new default for the `main` releases.
- @farfetch/[email protected]
This fixes the exchanges result reducer by changing the return from action.payload.result to action.payload.
- @farfetch/[email protected]
- @farfetch/[email protected]
- @farfetch/[email protected]
- Update omnitracking contract to v17.
- remove invalid fields (started with "_") of ga4 analytics integration.
- Add analytics versions field to omnitracking output payload.
- @farfetch/[email protected] - @farfetch/[email protected]
- @farfetch/[email protected]
- add new parameter `page number` for cases of endlessScrolling in interact content events (scrolling)
- @farfetch/[email protected] - @farfetch/[email protected]
2bbe2e7
to
4760943
Compare
4760943
to
d00158f
Compare
const properties = getEventProperties(data); | ||
|
||
return { | ||
products: get(properties, 'products', []).map(getProductData), | ||
orderId: properties.orderId, | ||
total: properties.total, |
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 property total
could be missing.
I understand that was "replaced" by value, but for full compatibility, I think we need to guarantee that total
stay on payload, so I suggest on new method, add total
value, mapping to new value
property.
Note: this could be relevant if some tenat had previous third party integration that use this specific event, and this specific value.
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.
Fixed! I've added a verification on getEventTotalValue
method to check if total exists and, if so, map that to value
property.
d00158f
to
3e7177e
Compare
Description
Dependencies
None.
Checklist
Disclaimer
By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement