[Mobile Payments] Add plugin slug to card reader tracks events so we can distinguish WCPay and Stripe

[allendav]

Closes: #5854

@ctarda @joshheald @koke - only one review is needed but all y'all are welcome - please see "known issues" below for problems I am working on still - I apologize for the size of the PR

Description

• This PR updates the following events to include the gatewayID (aka plugin slug) for the payment gateway in whose context the event occurs
  • cardReaderDiscoveryFailed
  • cardReaderConnectionSuccess
  • cardReaderConnectionFailed
  • cardReaderDisconnectTapped
  • cardReaderSoftwareUpdateTapped
  • cardReaderSoftwareUpdateStarted
  • cardReaderSoftwareUpdateFailed
  • cardReaderSoftwareUpdateSuccess
  • cardReaderSoftwareUpdateCancelTapped
  • cardReaderSoftwareUpdateCanceled
  • collectPaymentTapped
  • collectPaymentFailed
  • collectPaymentCanceled
  • collectPaymentSuccess
• The gatewayID is set to “woocommerce-payments” “woocommerce-gateway-stripe” or, in the unlikely event neither payment gateway account is found in CoreData, to “unknown”
• Some of these events also include error descriptions, battery levels, and whether a software update was required or optional. Following the guidance in docs/tracking-events.md for events with custom properties, this was done by adding a corresponding static func for each in WooAnalyticsEvent.swift
• To get this gatewayID to the point at which the tracks event is generated:
  • it is now passed into CardReaderConnectionController by CardReaderSettingsSearchingViewController and CollectOrderPaymentsUseCase
  • it is now available to CardReaderSettingsConnectedViewModel from a new CardReaderSettingsDataSource, which gets it from CardReaderSettingsResultsControllers (also new) which has a ResultsController
  • It is now available to CardReaderSettingsSearchingViewModel in the same manner
• SoftwareUpdateTypeProperty’s name string has been moved to WooAnalyticsEvents inside the InPersonPayments “namespace”
• Unit tests were added and updated to ensure the gatewayID property is set

Testing

• Ensure unit tests pass
• Test discovering, connecting to, and collecting payment with a reader using a store powered by the WooCommerce Payments extension. Look in Live Tracks (or add prints) and ensure that the events include the gatewayID in its properties.
• Repeat the test using the WooCommerce Stripe Gateway extension and ensure that the events include THAT gatewayID in its properties.

Tracks Hints

• In Tracks Live View, restrict event viewing to the logged in user name and then look for tracks events like:

woocommerceios_card_present_collect_payment_tapped woocommerceios_card_present_collect_payment_success

• Click on the > symbol to the left of the event to expand it. In the details, look for plugin_slug and ensure the correct gateway (e.g. “woocommerce-stripe”) is displayed for the event.

Known Issues

• I'm not yet sure why, but the gatewayID passed for cardReaderDisconnectTapped ( woocommerceios_card_reader_disconnect_tapped in tracks) is "unknown". Looking into it. FIXED WITH MOVING THE FRC INTO THE CONNECTION CONTROLLER

Screenshots

N/A

---

• [x] I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[peril-woocommerce[bot]]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[peril-woocommerce[bot]]

You can trigger an installable build for these changes by visiting CircleCI here.

[wpmobilebot]

You can test the changes on this Pull Request by downloading it from AppCenter here with build number: pr6123-b648c6c. IPA is available here. If you need access to this, you can ask a maintainer to add you.

[joshheald]

WCPay testing from settings

• [x] cardReaderDiscoveryFailed - logs unknown when tracked for The system is busy executing another command - please try again
• [x] cardReaderConnectionSuccess - (sometimes) logs unknown from settings
• [x] cardReaderConnectionFailed - logs unknown when tracked from settings with critically low battery (even with simulated reader)
• [x] cardReaderDisconnectTapped
• [x] cardReaderSoftwareUpdateTapped
• [x] cardReaderSoftwareUpdateStarted
• [x] cardReaderSoftwareUpdateFailed
• [x] cardReaderSoftwareUpdateSuccess
• [x] cardReaderSoftwareUpdateCancelTapped - logs unknown when tracked for tapping cancel on mandatory update
• [x] cardReaderSoftwareUpdateCanceled - logs unknown when tracked after tapping cancel on mandatory update

Stripe testing from settings

• [x] cardReaderDiscoveryFailed - logs unknown when tracked for The system is busy executing another command - please try again
• [x] cardReaderConnectionSuccess - (sometimes) logs unknown from settings
• [x] cardReaderConnectionFailed - logs unknown when tracked from settings with critically low battery
• [x] cardReaderDisconnectTapped
• [x] cardReaderSoftwareUpdateTapped
• [x] cardReaderSoftwareUpdateStarted
• [x] cardReaderSoftwareUpdateFailed
• [x] cardReaderSoftwareUpdateSuccess
• [x] cardReaderSoftwareUpdateCancelTapped - logs unknown when tracked for tapping cancel on mandatory update
• [x] cardReaderSoftwareUpdateCanceled - logs unknown when tracked after tapping cancel on mandatory update

N.B. most of the unknown plugin logs were only seen when using a physical reader. I found that the plugin type was quite reliably logged for a simulated reader, but I'm not sure why there's a difference there given the race condition I found seems more to do with our stack than Stripe's.

[joshheald]

Thanks @koke, I'll adjust my testing a bit based on your review being in already.

[koke]

I don't think it's caused by this PR, but when there's a mandatory update, cardReaderSoftwareUpdateStarted is not being fired. I think we currently only trigger that event from CardReaderSettingsConnectedViewModel, but mandatory updates happen while connecting (so before the "connected" screen shows up), and perhaps CardReaderConnectionController needs to also track that event.

EDIT: I'm seeing this only when connecting from an order, connecting from Settings shows cardReaderSoftwareUpdateStarted being triggered, but also unknown gateway as @joshheald noticed

[koke]

I'll do the same testing, but from the Order screen instead of settings.

Stripe testing

• [x] cardReaderDiscoveryFailed
• [x] cardReaderConnectionSuccess
• [x] cardReaderConnectionFailed
• [ ] cardReaderDisconnectTapped
• [ ] cardReaderSoftwareUpdateTapped
• [x] cardReaderSoftwareUpdateStarted - 🛑 missing, as noted before
• [x] cardReaderSoftwareUpdateFailed - 🛑 I connected to a reader that had a mandatory update but low battery. The update process seemed to start (progress at 0%), then it showed the low battery error. There was only a card_reader_connection_failed event
• [x] cardReaderSoftwareUpdateSuccess - 🛑 missing on a mandatory update, the first event I saw after the update was card_reader_connection_success then card_present_collect_payment_failed with a system busy error
• [x] cardReaderSoftwareUpdateCancelTapped
• [x] cardReaderSoftwareUpdateCanceled
• [x] collectPaymentTapped
• [x] collectPaymentFailed
• [x] collectPaymentCanceled - 🛑 tried this from a Simple Payments order, and it doesn't include the plugin_slug property
• [x] collectPaymentSuccess

[koke]

When testing connection from the order screen I got some weird sequence of events.

• I only see card_present_collect_payment_tapped fire after the reader has installed the software update and connected
• I see card_present_collect_payment_tapped firing twice, one with each gateway
• Then card_reader_connection_success with the right gateway
• Then card_present_collect_payment_tapped one more time with the right gateway (note that I was not tapping that button)
• Then card_present_collect_payment_failed twice, one with each gateway
• The app shows a "system is busy" payment error.

2022-02-16 11:30:13.875825+0100 WooCommerce[12805:13702274] 🔵 Tracked card_present_collect_payment_tapped, properties: [AnyHashable("is_wpcom_store"): false, AnyHashable("plugin_slug"): "woocommerce-stripe", AnyHashable("blog_id"): 202333381] 2022-02-16 11:30:13.894105+0100 WooCommerce[12805:13704222] 🔵 Tracked card_present_collect_payment_tapped, properties: [AnyHashable("blog_id"): 202333381, AnyHashable("is_wpcom_store"): false, AnyHashable("plugin_slug"): "woocommerce-payments"] 2022-02-16 11:30:13.911841+0100 WooCommerce[12805:13702274] 🔵 Tracked card_reader_connection_success, properties: [AnyHashable("blog_id"): 202333381, AnyHashable("battery_level"): "0.47", AnyHashable("is_wpcom_store"): false, AnyHashable("plugin_slug"): "woocommerce-stripe"] 2022-02-16 11:30:14.435077+0100 WooCommerce[12805:13704195] 🔵 Tracked card_present_collect_payment_tapped, properties: [AnyHashable("blog_id"): 202333381, AnyHashable("plugin_slug"): "woocommerce-stripe", AnyHashable("is_wpcom_store"): false] 2022-02-16 11:30:16.082429+0100 WooCommerce[12805:13704233] 🔵 Tracked card_present_collect_payment_failed, properties: [AnyHashable("is_wpcom_store"): false, AnyHashable("plugin_slug"): "woocommerce-payments", AnyHashable("error_description"): "The system is busy executing another command - please try again", AnyHashable("blog_id"): 202333381] 2022-02-16 11:30:16.299007+0100 WooCommerce[12805:13704231] 🔵 Tracked card_present_collect_payment_failed, properties: [AnyHashable("error_description"): "The system is busy executing another command - please try again", AnyHashable("is_wpcom_store"): false, AnyHashable("blog_id"): 202333381, AnyHashable("plugin_slug"): "woocommerce-stripe"]

When I try collecting again after the error, payment works and events seem fine:

2022-02-16 11:42:39.440332+0100 WooCommerce[12805:13707734] 🔵 Tracked card_present_collect_payment_tapped, properties: [AnyHashable("plugin_slug"): "woocommerce-stripe", AnyHashable("blog_id"): 202333381, AnyHashable("is_wpcom_store"): false] 2022-02-16 11:43:04.318126+0100 WooCommerce[12805:13707855] 🔵 Tracked card_present_collect_payment_success, properties: [AnyHashable("blog_id"): 202333381, AnyHashable("is_wpcom_store"): false, AnyHashable("plugin_slug"): "woocommerce-stripe"]

[joshheald]

I've had a look at the unknown plugin logs, and it looks like there's a race condition when connecting in settings.

On loading the Manage Card Reader screen, we refresh the account, which deletes the stale account.

We then make the viewModel, using the value of CardReaderSettingsDataSource.cardPresentPaymentGatewayID(). At this point, there's nothing in resultsControllers.paymentGatewayAccounts.filter { $0.isCardPresentEligible }, so we store the connectedGatewayID as unknown.

Then we make the CardReaderConnectionController, which takes the connectedGatewayID from the VM, which is still unknown. This is where most of the analytics will be logged from.

Shortly after this, the new account loads and cardPresentPaymentGatewayID() starts returning woocommerce-payments, and our viewModel.connectedGatewayId gets updated by the resultsControllers onReload closure but isn't updated on the CardReaderConnectionController. Although the connection controller's gatewayID property is a var, it's never mutated outside of init, and we don't recreate the connection controller.

This fits with the cardReaderSoftwareUpdateStarted getting logged (from the viewModel) with the correct plugin, while cancellation/failure events for the same update which are logged from the Controller get unknown

Probably the best way to solve this would be to get the gatewayID from the FRC on the CardReaderConnectionController, rather than pass it in to the init... but I've not really dug in to the implications of that.

[allendav]

@joshheald wrote:

Probably the best way to solve this would be to get the gatewayID from the FRC on the CardReaderConnectionController, rather than pass it in to the init... but I've not really dug in to the implications of that.

That's actually a capital idea. Implemented in https://github.com/woocommerce/woocommerce-ios/pull/6123/commits/55bcf16e5e7cf62a79aa7ffbca03960e6b387dc9 with unit tests updated in https://github.com/woocommerce/woocommerce-ios/pull/6123/commits/7eb5995337c1072590c1422ec8e27b749dc43805

@koke - I think this should also address the race condition. Since this is a big-ish change to the code, I'm thoroughly re-testing this now

[allendav]

I tested with a real card reader with connect-in-order flow and connect-in-settings flows and I'm getting the plugin in each tracks event now. I specifically confirmed:

woocommerceios_card_present_collect_payment_tapped woocommerceios_card_reader_connection_success woocommerceios_card_present_collect_payment_success woocommerceios_card_reader_disconnect_tapped

[joshheald]

I tested with a real card reader with connect-in-order flow and connect-in-settings flows and I'm getting the plugin in each tracks event now. I specifically confirmed:

woocommerceios_card_present_collect_payment_tapped woocommerceios_card_reader_connection_success woocommerceios_card_present_collect_payment_success woocommerceios_card_reader_disconnect_tapped

I retested this too, including the software update and connection failed events from the settings screen and a payment. I saw that the card_reader_software_update_started is not started in the payment flow, as Koke noted, so I'll make an issue for that.

Other than that missing event which is not related to this PR, it all tested well, and woocommerce-stripe/woocommerce-payments were included when I expected them to be.

[allendav]

Thank you all. Merging this now.