-
Notifications
You must be signed in to change notification settings - Fork 114
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
[Woo POS] MVP Analytics: Card payment success event properties #15148
base: trunk
Are you sure you want to change the base?
Conversation
We can add this event closer to order creation if we add it inside the OrderController sync function, however this couples both objects tighter. I think it’s fine to add the track event to the aggregate model as: - It will not track the event as a side effect of order sync, as this is only called on checkout, and we always create a new order when this happens - It would log the start time internally for both card and cash flows, however doesn’t matter as the event is only populated and sent for card events later on, and at this point we do not know how the customer is going to pay, as happens later in the flow.
Generated by 🚫 Danger |
|
orderStage = .finalizing | ||
await orderController.syncOrder(for: cart, retryHandler: { [weak self] in | ||
await self?.checkOut() | ||
}) | ||
collectOrderPaymentAnalyticsTracker.trackOrderCreationSuccess() |
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 had my doubts if this was the best place to add the order creation success event, I left my comments in the commit 3f9b72b
Description
This PR it's a continuation of #15138, and adds the following missing properties to
pos_card_present_collect_payment_success
event:In order to track these we follow the same design that the existing
milliseconds_since_customer_interaction_started
property, as we need to keep track of the elapsed time between the action starts and ends, before sending the event.Testing information
checkout_tap_count
.RELEASE-NOTES.txt
if necessary.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: