-
Notifications
You must be signed in to change notification settings - Fork 69
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
Apply discounts to the ecommerce event if available #419
Changes from 1 commit
46128f0
4a67cd5
71dc1b2
74389ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ export const getProductFieldObject = ( product, quantity ) => { | |
variantData.item_variant = product.variation; | ||
} | ||
|
||
return { | ||
const data = { | ||
item_id: getProductId( product ), | ||
item_name: product.name, | ||
...getProductCategories( product ), | ||
|
@@ -26,6 +26,23 @@ export const getProductFieldObject = ( product, quantity ) => { | |
), | ||
...variantData, | ||
}; | ||
|
||
// Apply discounts to ecommerce events. | ||
// https://developers.google.com/analytics/devguides/collection/ga4/apply-discount?client_type=gtag | ||
if ( | ||
typeof product.price_after_coupon_discount === 'number' && | ||
! isNaN( product.price_after_coupon_discount ) && | ||
product.price_after_coupon_discount !== product.prices.price | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WDYT about changing this to |
||
) { | ||
data.discount = | ||
product.prices.price - product.price_after_coupon_discount; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the nice catch! |
||
data.price = formatPrice( | ||
product.price_after_coupon_discount, | ||
product.prices.currency_minor_unit | ||
); | ||
} | ||
|
||
return data; | ||
}; | ||
|
||
/** | ||
|
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.
Why do we need these two?
product.price_after_coupon_discount
seems to be always a numberThere 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.
Since there were not only purchase event would call the method
getProductFieldObject()
, for examplebegin_checkout
event would call this method as well. Initially I wanted to test ifproduct.price_after_coupon_discount
is defined but since the price after discount could be0
and0
is treated asfalse
, that's why I added these two conditions here.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.
Ah, I see. Thanks for clarifying. That might explain why we used
if ( typeof product.price_after_coupon_discount === 'number' )
But what about the NaN line check?
Also WDYT of doing something like:
if ( product?.price_after_coupon_discount < product.prices.price )
So we can save both checks number and nan
You can test it but in case
product.price_after_coupon_discount
is undefined this would be false still.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.
@puntope
I added it because I see
typeof NaN
is also anumber
, so added it for some prevention.Ah good point, thanks for the info.