[Woo POS] MVP analytics: Handle card present events from POS #15138

iamgabrielma · 2025-02-13T14:57:36Z

Description

This PR creates POSCollectOrderPaymentAnalytics, a new implementation of the CollectOrderPaymentAnalyticsTracking protocol. We use this in order to track card-payment related events and their properties from POS without interfering with IPP events. We also need to do so as the event tracking logic is very deeply embedded into the card payment service.

Since we have to keep track of time intervals for different operations (order creation, card readiness, transaction successful), we also DI this instance into the aggregate model in order to be able to pass starting and ending time for these intervals.

Only the pos_card_present_collect_payment_success and one property (milliseconds_since_customer_interaction_started) is tracked on this PR, there are more properties that we need to track for this single event, but these will come on a following PR:

// Missing properties at this point:
milliseconds_since_order_creation_success
milliseconds_since_reader_ready_to_collect_payment
milliseconds_since_card_tapped
checkout_tap_count

Testing information

Run POS with either physical card reader or simulated one
Process a card present transaction
Upon success, observe the event pos_card_present_collect_payment_success with property milliseconds_since_customer_interaction_started is captured:

🔵 Tracked pos_card_present_collect_payment_success, properties: [milliseconds_since_customer_interaction_started: 21879.66299057007, is_wpcom_store: false, plan: , store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f, was_ecommerce_trial: false, site_url: https://indiemelon.mystagingwebsite.com, blog_id: -1]

Repeat for an IPP transaction in the app side. Observe that the pos event is not tracked, but the default IPP card_present_collect_payment_success instead.

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.

…r POS events and use it within the card reader adaptor

dangermattic · 2025-02-13T14:58:21Z

	1 Warning
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

	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-13T15:08:41Z

📲 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	pr15138-1bf71d3
	Version	21.7
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	`1bf71d3`
	App Center Build	WooCommerce - Prototype Builds #12967

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

staskus

Thanks! Works well 👍

I also tested IPP payments to confirm that it doesn't use POS logic.

I noticed when POS opens I get pos_pos:

staskus · 2025-02-14T08:06:41Z

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

@@ -149,6 +153,14 @@ extension PointOfSaleAggregateModel {
        paymentState = .card(.idle)
        cardPresentPaymentInlineMessage = nil
    }
+
+    private func trackCustomerInteractionStarted() {


;nit

We can put in these trackings into a different extension for code organization

Updated! 66141e4

staskus · 2025-02-14T08:10:15Z

WooCommerce/Classes/ViewRelated/Orders/Collect Payments/CollectOrderPaymentAnalytics.swift


+    func trackCustomerInteractionStarted()


nit;

Theoretically, we could have

POSCollectOrderPaymentAnalyticsTracking: CollectOrderPaymentAnalyticsTracking that has func trackCustomerInteractionStarted(), and POSCollectOrderPaymentAnalytics would implement POSCollectOrderPaymentAnalyticsTracking. This way non-POS parts of the code, such as various preflight controllers wouldn't have knowledge of trackCustomerInteractionStarted. POS aggregate model would expect POSCollectOrderPaymentAnalyticsTracking.

Yes, playing a bit more with the conformances here looks a good idea since we have so much shared logic 🤔

staskus · 2025-02-14T08:11:22Z

WooCommerce/Classes/ViewRelated/Hub Menu/HubMenuViewModel.swift

@@ -136,6 +136,7 @@ final class HubMenuViewModel: ObservableObject {
    }()

    private(set) var cardPresentPaymentService: CardPresentPaymentFacade?
+    private(set) var collectOrderPaymentAnalyticsTracker = POSCollectOrderPaymentAnalytics()


Yeah, I don't love that we use HubMenuViewModel as POS factory but we can deal with both creation of cardPresentPaymentService and collectOrderPaymentAnalyticsTracker in some future improvement PR if needed.

staskus · 2025-02-14T08:17:26Z

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

+    private func trackCustomerInteractionStarted() {
+        // At the moment we're assumming that an interaction starts simply when the cart is zero
+        // but a more complex logic will be needed for other cases
+        if cart.count == 0 {


Should clearing the cart reset the customer interaction time?

Good question, I don't think we're clear on that yet. Initially it would seem we would, but I have some doubts about it. Context: pdfdoF-6hn#comment-7582-p2

Thanks. Lets sync with Android. Otherwise, in some cases we could have inconclusive data.

staskus · 2025-02-14T08:21:26Z

WooCommerce/Classes/POS/Analytics/POSCollectOrderPaymentAnalytics.swift

+    func trackSuccessfulPayment(capturedPaymentData: CardPresentCapturedPaymentData) {
+        let elapsedTime = calculateElapsedTimeInMilliseconds(start: customerInteractionStarted, end: Date().timeIntervalSince1970)
+        ServiceLocator.analytics.track(event:
+                .PointOfSale.cardPresentCollectPaymentSuccess(millisecondsSinceCustomerIteractionStated: elapsedTime))


We send value such as 16712.467908859253, should we floor it, and send only the millisecond part 16712?

That sounds good to me. Updated here: 4748a30

🔵 Tracked pos_card_present_collect_payment_success, properties: [plan: , site_url: https://indiemelon.mystagingwebsite.com, milliseconds_since_customer_interaction_started: 27589.0, blog_id: -1, is_wpcom_store: false, was_ecommerce_trial: false, store_id: c5bd46cc-1804-4f7b-badb-bb98c449127f]

Since we floor the value and we return a Double, we always get the .0 which I don't think it's necessary, perhaps returning an Int here is enough as the event key already tells is milliseconds. I'll iterate on this when adding the rest of properties.

Agree. Lets sync with Android on this.

…-pos # Conflicts: # WooCommerce/Classes/POS/Presentation/POSFloatingControlView.swift

iamgabrielma · 2025-02-14T09:42:00Z

Thanks for the review!

I noticed when POS opens I get pos_pos:

Yes, this duplication was triggering me a bit but was the "official" event 😅 (Ref: pdfdoF-6hn#comment-7556-p2 ) , it's resolved now: b5fd2b3

iamgabrielma added 3 commits February 13, 2025 15:08

Create new implementation for CollectOrderPaymentAnalyticsTracking fo…

f346fac

…r POS events and use it within the card reader adaptor

DI POSCollectOrderPaymentAnalytics through card reader service

94eae8e

DI tracker into aggregate model from shared instance

3e3424a

iamgabrielma added 4 commits February 13, 2025 16:08

track elapsed time

f43fb21

move property to key value

da2729f

extract POSCollectOrderPaymentAnalytics to file

f958e4f

make tests compile

5fd196f

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

iamgabrielma added this to the 21.8 milestone Feb 13, 2025

iamgabrielma marked this pull request as ready for review February 13, 2025 15:24

iamgabrielma requested a review from staskus February 13, 2025 15:25

staskus approved these changes Feb 14, 2025

View reviewed changes

iamgabrielma added 5 commits February 14, 2025 10:02

remove prefix from event

b5fd2b3

move tracking to extension

66141e4

apply floor to elapsed time

4748a30

Merge branch 'trunk' into task/15128-resolve-card-present-events-from…

bfac5ec

…-pos # Conflicts: # WooCommerce/Classes/POS/Presentation/POSFloatingControlView.swift

fix merge conflict in previews

1bf71d3

iamgabrielma enabled auto-merge February 14, 2025 09:42

iamgabrielma merged commit a477857 into trunk Feb 14, 2025
12 checks passed

iamgabrielma deleted the task/15128-resolve-card-present-events-from-pos branch February 14, 2025 10:00

This was referenced Feb 14, 2025

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

Merged

[Woo POS] MVP analytics: improve POSCollectOrderPaymentAnalyticsTracking conformance #15149

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Woo POS] MVP analytics: Handle card present events from POS #15138

[Woo POS] MVP analytics: Handle card present events from POS #15138

iamgabrielma commented Feb 13, 2025 •

edited by staskus

Loading

dangermattic commented Feb 13, 2025 •

edited

Loading

wpmobilebot commented Feb 13, 2025 •

edited

Loading

staskus left a comment

staskus Feb 14, 2025

iamgabrielma Feb 14, 2025

staskus Feb 14, 2025

iamgabrielma Feb 14, 2025

staskus Feb 14, 2025

staskus Feb 14, 2025

iamgabrielma Feb 14, 2025

staskus Feb 14, 2025

staskus Feb 14, 2025

iamgabrielma Feb 14, 2025

staskus Feb 14, 2025

iamgabrielma commented Feb 14, 2025

[Woo POS] MVP analytics: Handle card present events from POS #15138

[Woo POS] MVP analytics: Handle card present events from POS #15138

Conversation

iamgabrielma commented Feb 13, 2025 • edited by staskus Loading

Description

Testing information

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

dangermattic commented Feb 13, 2025 • edited Loading

wpmobilebot commented Feb 13, 2025 • edited Loading

staskus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamgabrielma commented Feb 14, 2025

iamgabrielma commented Feb 13, 2025 •

edited by staskus

Loading

dangermattic commented Feb 13, 2025 •

edited

Loading

wpmobilebot commented Feb 13, 2025 •

edited

Loading