Merge pull request #6495 from woocommerce/feat/5984-add-payment-method-to-collect-payment-events

jaclync · web-flow · commit 6f79e8d03dfe · 2022-03-31T08:59:42.000+08:00
IPP analytics: add `payment_method_type` property to `card_present_collect_payment_success` and `card_present_collect_payment_failed` events
diff --git a/Hardware/Hardware/CardReader/CardReaderServiceError.swift b/Hardware/Hardware/CardReader/CardReaderServiceError.swift
@@ -12,7 +12,7 @@ public enum CardReaderServiceError: Error {
     /// Error thrown while connecting to a reader
     case connection(underlyingError: UnderlyingError = .internalServiceError)
 
-    /// Error thrown while disonnecting from a reader
+    /// Error thrown while disconnecting from a reader
     case disconnection(underlyingError: UnderlyingError = .internalServiceError)
 
     /// Error thrown while creating a payment intent
@@ -24,6 +24,10 @@ public enum CardReaderServiceError: Error {
     /// Error thrown while capturing a payment
     case paymentCapture(underlyingError: UnderlyingError = .internalServiceError)
 
+    /// Error thrown when the order payment fails to be captured with a known payment method.
+    /// The payment method is currently used for analytics.
+    case paymentCaptureWithPaymentMethod(underlyingError: Error, paymentMethod: PaymentMethod)
+
     /// Error thrown while cancelling a payment
     case paymentCancellation(underlyingError: UnderlyingError = .internalServiceError)
 
@@ -58,6 +62,8 @@ extension CardReaderServiceError: LocalizedError {
                 .refundCancellation(let underlyingError),
                 .softwareUpdate(let underlyingError, _):
             return underlyingError.errorDescription
+        case .paymentCaptureWithPaymentMethod(underlyingError: let underlyingError, paymentMethod: _):
+            return (underlyingError as? UnderlyingError)?.errorDescription ?? underlyingError.localizedDescription
         case .bluetoothDenied:
             return NSLocalizedString(
                 "This app needs permission to access Bluetooth to connect to a card reader, please change the privacy settings if you wish to allow this.",
diff --git a/Hardware/Hardware/CardReader/PaymentIntent.swift b/Hardware/Hardware/CardReader/PaymentIntent.swift
@@ -105,3 +105,12 @@ public extension PaymentIntent {
         return metadata
     }
 }
+
+public extension PaymentIntent {
+    /// Returns the payment method from a PaymentIntent if available, typically after the payment is processed.
+    /// Before the payment is processed, `nil` is returned.
+    /// - Returns: an optional payment method that is set after the payment is processed.
+    func paymentMethod() -> PaymentMethod? {
+        charges.first?.paymentMethod
+    }
+}
diff --git a/Hardware/Hardware/CardReader/PaymentMethod.swift b/Hardware/Hardware/CardReader/PaymentMethod.swift
@@ -1,5 +1,5 @@
 /// The type of the PaymentMethod.
-public enum PaymentMethod {
+public enum PaymentMethod: Equatable {
     /// A card payment method.
     case card
 
diff --git a/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift b/Hardware/Hardware/CardReader/StripeCardReader/StripeCardReaderService.swift
@@ -461,7 +461,12 @@ private extension StripeCardReaderService {
             Terminal.shared.processPayment(intent) { (intent, error) in
                 if let error = error {
                     let underlyingError = UnderlyingError(with: error)
-                    promise(.failure(CardReaderServiceError.paymentCapture(underlyingError: underlyingError)))
+                    if let paymentMethod = error.paymentIntent.map({ PaymentIntent(intent: $0) })?.paymentMethod() {
+                        promise(.failure(CardReaderServiceError.paymentCaptureWithPaymentMethod(underlyingError: underlyingError,
+                                                                                                paymentMethod: paymentMethod)))
+                    } else {
+                        promise(.failure(CardReaderServiceError.paymentCapture(underlyingError: underlyingError)))
+                    }
                 }
 
                 if let intent = intent {
diff --git a/Hardware/HardwareTests/PaymentIntentTests.swift b/Hardware/HardwareTests/PaymentIntentTests.swift
@@ -48,8 +48,43 @@ final class PaymentIntentTests: XCTestCase {
         // It is not possible to instantiate a SCPCharge, which is what
         // would be needed to instantiate a mock intent.
         // For now, we will rely on counting charges as a way
-        // to chek that at least both SCPPaymentIntent and
+        // to check that at least both SCPPaymentIntent and
         // PaymentIntent reference the same number of charges 🤷
         XCTAssertEqual(intent.charges.count, mockIntent.charges.count)
     }
+
+    func test_paymentMethod_is_nil_when_there_are_no_charges() {
+        // When
+        let intent = PaymentIntent(intent: mockIntent)
+
+        // Then
+        XCTAssertNil(intent.paymentMethod())
+    }
+
+    func test_paymentMethod_is_set_by_the_first_charge_when_there_are_two_charges() {
+        // When
+        let intent = PaymentIntent(id: "",
+                                   status: .processing,
+                                   created: .init(),
+                                   amount: 1201,
+                                   currency: "cad",
+                                   metadata: nil,
+                                   charges: [.init(id: "",
+                                                   amount: 201,
+                                                   currency: "cad",
+                                                   status: .failed,
+                                                   description: nil,
+                                                   metadata: nil,
+                                                   paymentMethod: .card),
+                                             .init(id: "",
+                                                   amount: 1000,
+                                                   currency: "cad",
+                                                   status: .failed,
+                                                   description: nil,
+                                                   metadata: nil,
+                                                   paymentMethod: .unknown)])
+
+        // Then
+        XCTAssertEqual(intent.paymentMethod(), .card)
+    }
 }
diff --git a/WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift b/WooCommerce/Classes/Analytics/WooAnalyticsEvent.swift
@@ -39,8 +39,15 @@ import Yosemite
 /// ~~~
 ///
 public struct WooAnalyticsEvent {
+    init(statName: WooAnalyticsStat, properties: [String: WooAnalyticsEventPropertyType], error: Error? = nil) {
+        self.statName = statName
+        self.properties = properties
+        self.error = error
+    }
+
     let statName: WooAnalyticsStat
     let properties: [String: WooAnalyticsEventPropertyType]
+    let error: Error?
 }
 
 // MARK: - In-app Feedback and Survey
@@ -569,6 +576,7 @@ extension WooAnalyticsEvent {
             static let countryCode = "country"
             static let gatewayID = "plugin_slug"
             static let errorDescription = "error_description"
+            static let paymentMethodType = "payment_method_type"
             static let softwareUpdateType = "software_update_type"
         }
 
@@ -779,13 +787,39 @@ extension WooAnalyticsEvent {
         ///   - countryCode: the country code of the store.
         ///
         static func collectPaymentFailed(forGatewayID: String?, error: Error, countryCode: String) -> WooAnalyticsEvent {
-            WooAnalyticsEvent(statName: .collectPaymentFailed,
-                              properties: [
-                                Keys.countryCode: countryCode,
-                                Keys.gatewayID: gatewayID(forGatewayID: forGatewayID),
-                                Keys.errorDescription: error.localizedDescription
-                              ]
-            )
+            let paymentMethod: PaymentMethod? = {
+                guard case let CardReaderServiceError.paymentCaptureWithPaymentMethod(_, paymentMethod) = error else {
+                    return nil
+                }
+                return paymentMethod
+            }()
+            let errorDescription: String? = {
+                guard case let CardReaderServiceError.paymentCaptureWithPaymentMethod(underlyingError, paymentMethod) = error else {
+                    return error.localizedDescription
+                }
+                switch paymentMethod {
+                case let .cardPresent(details):
+                    return [
+                        "underlyingError": underlyingError,
+                        "cardBrand": details.brand
+                    ].description
+                case let .interacPresent(details):
+                    return [
+                        "underlyingError": underlyingError,
+                        "cardBrand": details.brand
+                    ].description
+                default:
+                    return underlyingError.localizedDescription
+                }
+            }()
+            let properties: [String: WooAnalyticsEventPropertyType] = [
+                Keys.countryCode: countryCode,
+                Keys.gatewayID: gatewayID(forGatewayID: forGatewayID),
+                Keys.paymentMethodType: paymentMethod?.analyticsValue,
+                Keys.errorDescription: errorDescription
+            ].compactMapValues { $0 }
+            return WooAnalyticsEvent(statName: .collectPaymentFailed,
+                                     properties: properties)
         }
 
         /// Tracked when the payment collection is cancelled
@@ -808,12 +842,14 @@ extension WooAnalyticsEvent {
         /// - Parameters:
         ///   - forGatewayID: the plugin (e.g. "woocommerce-payments" or "woocommerce-gateway-stripe") to be included in the event properties in Tracks.
         ///   - countryCode: the country code of the store.
+        ///   - paymentMethod: the payment method of the captured payment.
         ///
-        static func collectPaymentSuccess(forGatewayID: String?, countryCode: String) -> WooAnalyticsEvent {
+        static func collectPaymentSuccess(forGatewayID: String?, countryCode: String, paymentMethod: PaymentMethod) -> WooAnalyticsEvent {
             WooAnalyticsEvent(statName: .collectPaymentSuccess,
                               properties: [
                                 Keys.countryCode: countryCode,
-                                Keys.gatewayID: gatewayID(forGatewayID: forGatewayID)
+                                Keys.gatewayID: gatewayID(forGatewayID: forGatewayID),
+                                Keys.paymentMethodType: paymentMethod.analyticsValue
                               ]
             )
         }
@@ -842,3 +878,16 @@ extension WooAnalyticsEvent {
         }
     }
 }
+
+private extension PaymentMethod {
+    var analyticsValue: String {
+        switch self {
+        case .card, .cardPresent:
+            return "card"
+        case .interacPresent:
+            return "card_interac"
+        case .unknown:
+            return "unknown"
+        }
+    }
+}
diff --git a/WooCommerce/Classes/ServiceLocator/Analytics.swift b/WooCommerce/Classes/ServiceLocator/Analytics.swift
@@ -65,6 +65,6 @@ extension Analytics {
     /// - Parameter event: The event to track along with its properties.
     ///
     func track(event: WooAnalyticsEvent) {
-        track(event.statName, withProperties: event.properties)
+        track(event.statName, properties: event.properties, error: event.error)
     }
 }
diff --git a/WooCommerce/Classes/ViewModels/CardPresentPayments/PaymentCaptureOrchestrator.swift b/WooCommerce/Classes/ViewModels/CardPresentPayments/PaymentCaptureOrchestrator.swift
@@ -1,6 +1,15 @@
 import Yosemite
 import PassKit
 
+/// Contains data associated with a payment that has been collected, processed, and captured.
+struct CardPresentCapturedPaymentData {
+    /// Currently used for analytics.
+    let paymentMethod: PaymentMethod
+
+    /// Used for receipt generation for display in the app.
+    let receiptParameters: CardPresentReceiptParameters
+}
+
 /// Orchestrates the sequence of actions required to capture a payment:
 /// 1. Check if there is a card reader connected
 /// 2. Launch the reader discovering and pairing UI if there is no reader connected
@@ -21,7 +30,7 @@ final class PaymentCaptureOrchestrator {
                         onWaitingForInput: @escaping () -> Void,
                         onProcessingMessage: @escaping () -> Void,
                         onDisplayMessage: @escaping (String) -> Void,
-                        onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> Void) {
+                        onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> Void) {
         /// Bail out if the order amount is below the minimum allowed:
         /// https://stripe.com/docs/currencies#minimum-and-maximum-charge-amounts
         guard isTotalAmountValid(order: order) else {
@@ -119,7 +128,7 @@ final class PaymentCaptureOrchestrator {
 }
 
 private extension PaymentCaptureOrchestrator {
-    /// Supress wallet presentation. This requires a special entitlement from Apple:
+    /// Suppress wallet presentation. This requires a special entitlement from Apple:
     /// `com.apple.developer.passkit.pass-presentation-suppression`
     /// See Woo-*.entitlements in WooCommerce/Resources
     ///
@@ -169,7 +178,7 @@ private extension PaymentCaptureOrchestrator {
 private extension PaymentCaptureOrchestrator {
     func completePaymentIntentCapture(order: Order,
                                     captureResult: Result<PaymentIntent, Error>,
-                                    onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> Void) {
+                                    onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> Void) {
         switch captureResult {
         case .failure(let error):
             onCompletion(.failure(error))
@@ -184,15 +193,16 @@ private extension PaymentCaptureOrchestrator {
     func submitPaymentIntent(siteID: Int64,
                              order: Order,
                              paymentIntent: PaymentIntent,
-                             onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> Void) {
+                             onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> Void) {
         let action = CardPresentPaymentAction.captureOrderPayment(siteID: siteID,
                                                                      orderID: order.orderID,
                                                                      paymentIntentID: paymentIntent.id) { [weak self] result in
             guard let self = self else {
                 return
             }
 
-            guard let receiptParameters = paymentIntent.receiptParameters() else {
+            guard let paymentMethod = paymentIntent.paymentMethod(),
+                  let receiptParameters = paymentIntent.receiptParameters() else {
                 let error = CardReaderServiceError.paymentCapture()
 
                 DDLogError("⛔️ Payment completed without required metadata: \(error)")
@@ -205,9 +215,10 @@ private extension PaymentCaptureOrchestrator {
             case .success:
                 self.celebrate() // plays a sound, haptic
                 self.saveReceipt(for: order, params: receiptParameters)
-                onCompletion(.success(receiptParameters))
+                onCompletion(.success(.init(paymentMethod: paymentMethod,
+                                            receiptParameters: receiptParameters)))
             case .failure(let error):
-                onCompletion(.failure(error))
+                onCompletion(.failure(CardReaderServiceError.paymentCaptureWithPaymentMethod(underlyingError: error, paymentMethod: paymentMethod)))
                 return
             }
         }
diff --git a/WooCommerce/Classes/ViewRelated/Orders/Collect Payments/CollectOrderPaymentUseCase.swift b/WooCommerce/Classes/ViewRelated/Orders/Collect Payments/CollectOrderPaymentUseCase.swift
@@ -116,10 +116,10 @@ final class CollectOrderPaymentUseCase: NSObject, CollectOrderPaymentProtocol {
                 onCollect(result.map { _ in () }) // Transforms Result<CardPresentReceiptParameters, Error> to Result<Void, Error>
 
                 // Handle payment receipt
-                guard let receiptParameters = try? result.get() else {
+                guard let paymentData = try? result.get() else {
                     return
                 }
-                self?.presentReceiptAlert(receiptParameters: receiptParameters, backButtonTitle: backButtonTitle, onCompleted: onCompleted)
+                self?.presentReceiptAlert(receiptParameters: paymentData.receiptParameters, backButtonTitle: backButtonTitle, onCompleted: onCompleted)
             })
         }
     }
@@ -169,7 +169,7 @@ private extension CollectOrderPaymentUseCase {
 
     /// Attempts to collect payment for an order.
     ///
-    func attemptPayment(onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> ()) {
+    func attemptPayment(onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> ()) {
         // Track tapped event
         analytics.track(event: WooAnalyticsEvent.InPersonPayments.collectPaymentTapped(forGatewayID: paymentGatewayAccount.gatewayID,
                                                                                        countryCode: configurationLoader.configuration.countryCode))
@@ -198,8 +198,8 @@ private extension CollectOrderPaymentUseCase {
 
                }, onCompletion: { [weak self] result in
                    switch result {
-                   case .success(let receiptParameters):
-                       self?.handleSuccessfulPayment(receipt: receiptParameters, onCompletion: onCompletion)
+                   case .success(let capturedPaymentData):
+                       self?.handleSuccessfulPayment(capturedPaymentData: capturedPaymentData, onCompletion: onCompletion)
                    case .failure(let error):
                        self?.handlePaymentFailureAndRetryPayment(error, onCompletion: onCompletion)
                    }
@@ -209,18 +209,21 @@ private extension CollectOrderPaymentUseCase {
 
     /// Tracks the successful payments
     ///
-    func handleSuccessfulPayment(receipt: CardPresentReceiptParameters, onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> ()) {
+    func handleSuccessfulPayment(capturedPaymentData: CardPresentCapturedPaymentData,
+                                 onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> ()) {
         // Record success
-        analytics.track(event: WooAnalyticsEvent.InPersonPayments.collectPaymentSuccess(forGatewayID: paymentGatewayAccount.gatewayID,
-                                                                                        countryCode: configurationLoader.configuration.countryCode))
+        analytics.track(event: WooAnalyticsEvent.InPersonPayments
+                            .collectPaymentSuccess(forGatewayID: paymentGatewayAccount.gatewayID,
+                                                   countryCode: configurationLoader.configuration.countryCode,
+                                                   paymentMethod: capturedPaymentData.paymentMethod))
 
         // Success Callback
-        onCompletion(.success(receipt))
+        onCompletion(.success(capturedPaymentData))
     }
 
     /// Log the failure reason, cancel the current payment and retry it if possible.
     ///
-    func handlePaymentFailureAndRetryPayment(_ error: Error, onCompletion: @escaping (Result<CardPresentReceiptParameters, Error>) -> ()) {
+    func handlePaymentFailureAndRetryPayment(_ error: Error, onCompletion: @escaping (Result<CardPresentCapturedPaymentData, Error>) -> ()) {
         // Record error
         analytics.track(event: WooAnalyticsEvent.InPersonPayments.collectPaymentFailed(forGatewayID: paymentGatewayAccount.gatewayID,
                                                                                        error: error,
diff --git a/Yosemite/Yosemite/Model/Model.swift b/Yosemite/Yosemite/Model/Model.swift
@@ -135,6 +135,7 @@ public typealias CardReaderServiceDiscoveryStatus = Hardware.CardReaderServiceDi
 public typealias CardReaderServiceError = Hardware.CardReaderServiceError
 public typealias CardReaderType = Hardware.CardReaderType
 public typealias CardReaderConfigError = Hardware.CardReaderConfigError
+public typealias PaymentMethod = Hardware.PaymentMethod
 public typealias PaymentParameters = Hardware.PaymentIntentParameters
 public typealias RefundParameters = Hardware.RefundParameters
 public typealias PaymentIntent = Hardware.PaymentIntent
diff --git a/Yosemite/Yosemite/Model/Payments/PaymentIntent+ReceiptParameters.swift b/Yosemite/Yosemite/Model/Payments/PaymentIntent+ReceiptParameters.swift
@@ -5,7 +5,7 @@ public extension PaymentIntent {
     /// render a receipt.
     /// - Returns: an optional struct containing all the data that needs to go into a receipt
     func receiptParameters() -> CardPresentReceiptParameters? {
-        guard let cardDetails = self.charges.first?.paymentMethod?.cardPresentDetails else {
+        guard let cardDetails = paymentMethod()?.cardPresentDetails else {
             return nil
         }
 

Original file line number	Diff line number	Diff line change
`@@ -105,3 +105,12 @@ public extension PaymentIntent {`
`105`	`105`	`return metadata`
`106`	`106`	`}`
`107`	`107`	`}`
	`108`	`+`
	`109`	`+public extension PaymentIntent {`
	`110`	`+ /// Returns the payment method from a PaymentIntent if available, typically after the payment is processed.`
	`111`	+ /// Before the payment is processed, `nil` is returned.
	`112`	`+ /// - Returns: an optional payment method that is set after the payment is processed.`
	`113`	`+ func paymentMethod() -> PaymentMethod? {`
	`114`	`+ charges.first?.paymentMethod`
	`115`	`+ }`
	`116`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,6 @@ extension Analytics {`
`65`	`65`	`/// - Parameter event: The event to track along with its properties.`
`66`	`66`	`///`
`67`	`67`	`func track(event: WooAnalyticsEvent) {`
`68`		`- track(event.statName, withProperties: event.properties)`
	`68`	`+ track(event.statName, properties: event.properties, error: event.error)`
`69`	`69`	`}`
`70`	`70`	`}`