Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[HS-1071657] Migrate cart and view product data layer events to GA4 #1083

Merged
merged 4 commits into from
Jan 9, 2024
Merged

[HS-1071657] Migrate cart and view product data layer events to GA4 #1083

merged 4 commits into from
Jan 9, 2024

Conversation

canac
Copy link
Contributor

@canac canac commented Dec 20, 2023

Migrate add_to_cart, remove_from_cart, and view_item events to GA4.

https://secure.helpscout.net/conversation/2457822430/1071657?viewId=7296729

@canac canac added the On Staging Will be merged to the staging branch by Github Actions label Dec 20, 2023
@canac canac changed the title [HS-1071657] Migrate cart and view product data layer events to GA4. [HS-1071657] Migrate cart and view product data layer events to GA4 Dec 20, 2023
@wrandall22
Copy link
Contributor

I'd prefer the first 2 commits to be squashed together before merging.

@canac
Migrate add_to_cart, remove_from_cart, and view_item to GA4
c045f8b
@canac
Fix tests sharing state
db46df5 
Previously, focusing on cartRemove would cause those tests to fail because
it was relying on cartAdd or buildProductVar to initialize digitalData
@canac
Copy link
Contributor Author

canac commented Dec 20, 2023

Will requested that testing_transaction be explicitly set to false instead of being missing.

We should consider sending a boolean instead of the strings "true" and "false" like branded checkout does, but I don't want to change that yet until we can prove that it won't break anything.

@canac
Copy link
Contributor Author

canac commented Dec 21, 2023

According to Will, this change is not high urgency and wait until after the code freeze to be merged.

dr-bizz
dr-bizz reviewed Dec 22, 2023
View reviewed changes
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great! Thanks for improving my code with the testingTransaction

src/app/analytics/analytics.factory.js
detail: {
products: [generateProduct(modifiedProductData)]
}
// value is unavailable until the user selects a gift amount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there ever be a view_item which has a value? If no, should we have it this object, or do the analytics guys need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. view_item won't ever have a value amount at this stage because the user hasn't selected the gift amount or frequency yet. When I implemented the GA4 spec for branded checkout, the spec docs said, "If no value exists for an attribute in the dataLayer, please pass JavaScript undefined as the value." That's why I'm passing undefined instead of omitting that field.

@canac canac merged commit 3c526b4 into master Jan 9, 2024
5 checks passed
@canac canac deleted the hs-1071657-datalayer branch January 9, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dr-bizz dr-bizz dr-bizz left review comments

@wrandall22 wrandall22 wrandall22 approved these changes

Assignees
No one assigned
Labels
On Staging Will be merged to the staging branch by Github Actions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@canac @wrandall22 @dr-bizz