[Woo POS] MVP Analytics: Card payment success event properties #15148

iamgabrielma · 2025-02-14T11:19:21Z

Description

This PR it's a continuation of #15138, and adds the following missing properties to pos_card_present_collect_payment_success event:

milliseconds_since_order_creation_success
milliseconds_since_reader_ready_to_collect_payment
milliseconds_since_card_tapped
checkout_tap_count

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

In POS, add some items to cart, tap checkout, go back to the cart (make some edits or not) and tap checkout again, then follow the card payment flow. At the end, observe that the event has been tracked with elapsed times and a correct number of checkout_tap_count.

🔵 Tracked pos_card_present_collect_payment_success, properties: [is_wpcom_store: false, milliseconds_since_card_tapped: 5625.0, plan: , site_url: https://indiemelon.mystagingwebsite.com, checkout_tap_count: 3, store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, was_ecommerce_trial: false, milliseconds_since_customer_interaction_started: 27118.0, milliseconds_since_order_creation_success: 19099.0, blog_id: -1, milliseconds_since_reader_ready_to_collect_payment: 9861.0]

Repeat the flow just tapping checkout once, observe the count is 1.

While I have a specific issue logged for adding more testing around tracks, in general I found very tricky to add unit tests due to how "invasive" these are in POS, and how much we'd need to mock.

I have considered if this change warrants user-facing release notes and have added them to 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:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

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.

dangermattic · 2025-02-14T11:19:55Z

	1 Message
📖	This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed: The tracks events must be validated in the Tracks system. Verify the internal Tracks spreadsheet has also been updated. Please consider registering any new events. The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

wpmobilebot · 2025-02-14T11:30:28Z

📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS
	Build Number	pr15148-af7ce8f
	Version	21.7
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`af7ce8f`
	App Center Build	WooCommerce - Prototype Builds #13013

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

…ytics

staskus

Thanks! It works well! However, there are a few concerns to address:

If we consider order creation always when we checkout or only when the new order is actually created?
Processing event happens multiple times, even a couple of seconds after the tap.
Checkout tracker is not reset if POS is closed and reopened.

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

WooCommerce/WooCommerceTests/POS/Analytics/POSCollectOrderPaymentAnalyticsTests.swift

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

The Stripe SDK returns multiple `.processing` events, but we want to capture the first one in the stream only. This flag is reset as soon as the payment has been successful

iamgabrielma · 2025-02-18T12:19:15Z

Thanks for the review @staskus ! This is ready for another round. Just a recap as per slack:

If we consider order creation always when we checkout or only when the new order is actually created?

When it's actually created we trigger the event as soon as possible. At the moment we track the orderCreationSuccess event in the order controller, but we milliseconds for "order success" just after we exit the function in the aggregate model. Ideally we would track those two at the same time but it's going to make everything very messy, so it doesn't seem to be worth it.

Processing event happens multiple times, even a couple of seconds after the tap.

We resolved this by having a flag in the analytics tracker, as we couldn't rely on .cardRemoved. the flag resets on payment success and on user interaction starts. This also helps to reset it when there's a cash payment as a side-effect, but we can trigger it explicitly in the next PR when handling those.

Checkout tracker is not reset if POS is closed and reopened.

That's alright, we reset trackers on user interaction start 👍

staskus

Thank you for the fixes!

Left one comment about syncOrder state

staskus · 2025-02-18T12:33:00Z

WooCommerce/Classes/POS/Controllers/PointOfSaleOrderController.swift

        let posCartItems = cartItems.map {
            POSCartItem(item: $0.item, quantity: Decimal($0.quantity))
        }

        guard !orderState.isSyncing,
              !posCartItems.matches(order: order) else {
-            return
+            return .failure(SyncOrderStateError.syncFailure)


If posCartItems.matches(order: order) it means that that .success(.orderNotChanged) not shat syncing failed.

Appreciated, I got confused with the negative guard 🙇 fixed here and added a new case and test: af7ce8f

iamgabrielma added 3 commits February 14, 2025 11:28

add milliseconds_since_reader_ready_to_collect_payment

5628911

Add milliseconds_since_card_tapped

fa33dfb

iamgabrielma added type: task An internally driven task. category: tracks Related to analytics, including Tracks Events. feature: POS labels Feb 14, 2025

iamgabrielma added this to the 21.8 milestone Feb 14, 2025

iamgabrielma added 2 commits February 14, 2025 12:35

Add checkout_tap_count

7f47ab8

clean up

48adb52

iamgabrielma marked this pull request as ready for review February 14, 2025 11:49

iamgabrielma commented Feb 14, 2025

View reviewed changes

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift Outdated Show resolved Hide resolved

make test target compile

b94025c

iamgabrielma requested a review from staskus February 14, 2025 11:56

This was referenced Feb 14, 2025

[Woo POS] MVP Analytics: Track time intervals for POS events #15129

Closed

[Woo POS] MVP Analytics #13277

Closed

[Woo POS] MVP analytics: Track remaining MVP events and properties #15151

Merged

iamgabrielma added 2 commits February 17, 2025 11:23

Unit test for trackCheckoutTapped

0f16ec7

DI analytics and additional unit tests for POSCollectOrderPaymentAnal…

f4e300e

…ytics

staskus reviewed Feb 17, 2025

View reviewed changes

staskus assigned staskus and iamgabrielma Feb 17, 2025

iamgabrielma added 4 commits February 18, 2025 08:48

update syncOrder signature to result a result type

0dd8512

move reset to analytics tracker

b046537

add hasTrackedProcessingPayment flag

f2668a9

The Stripe SDK returns multiple `.processing` events, but we want to capture the first one in the stream only. This flag is reset as soon as the payment has been successful

Reset all ongoing counters on user interaction start

efff5ea

iamgabrielma mentioned this pull request Feb 18, 2025

[Woo POS] Analytics: Unit Test Improvements #15179

Open

iamgabrielma requested a review from staskus February 18, 2025 12:19

staskus approved these changes Feb 18, 2025

View reviewed changes

fix issue with returning the wrong result on sync

af7ce8f

iamgabrielma merged commit 06b9022 into trunk Feb 18, 2025
12 checks passed

iamgabrielma deleted the task/collect-payment-success-event-properties branch February 18, 2025 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Woo POS] MVP Analytics: Card payment success event properties #15148

[Woo POS] MVP Analytics: Card payment success event properties #15148

iamgabrielma commented Feb 14, 2025 •

edited by staskus

Loading

dangermattic commented Feb 14, 2025 •

edited

Loading

wpmobilebot commented Feb 14, 2025 •

edited

Loading

staskus left a comment

iamgabrielma commented Feb 18, 2025

staskus left a comment

staskus Feb 18, 2025

iamgabrielma Feb 18, 2025

[Woo POS] MVP Analytics: Card payment success event properties #15148

[Woo POS] MVP Analytics: Card payment success event properties #15148

Conversation

iamgabrielma commented Feb 14, 2025 • edited by staskus Loading

Description

Testing information

Reviewer (or Author, in the case of optional code reviews):

dangermattic commented Feb 14, 2025 • edited Loading

wpmobilebot commented Feb 14, 2025 • edited Loading

staskus left a comment

Choose a reason for hiding this comment

iamgabrielma commented Feb 18, 2025

staskus left a comment

Choose a reason for hiding this comment

staskus Feb 18, 2025

Choose a reason for hiding this comment

iamgabrielma Feb 18, 2025

Choose a reason for hiding this comment

iamgabrielma commented Feb 14, 2025 •

edited by staskus

Loading

dangermattic commented Feb 14, 2025 •

edited

Loading

wpmobilebot commented Feb 14, 2025 •

edited

Loading