From a2f7913fdbb9956660bcc92b4f0a35d87fc52ff8 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Mon, 27 Jan 2025 14:58:16 -0700 Subject: [PATCH 01/16] Distinguish between embedded in load events --- .../Source/Analytics/STPAnalyticEvent.swift | 14 ++++++++- .../PaymentSheetAnalyticsHelper.swift | 30 +++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift index baf099dddc3..99427f3f644 100644 --- a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift +++ b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift @@ -77,7 +77,19 @@ import Foundation // MARK: - Embedded Payment Element init case mcInitEmbedded = "mc_embedded_init" - + + // MARK: - Embedded Payment Element loading + case mcLoadStartedEmbedded = "mc_load_started_embedded" + case mcLoadSucceededEmbedded = "mc_load_succeeded_embedded" + case mcLoadFailedEmbedded = "mc_load_failed_embedded" + + // MARK: - Embedded Payment Element confirm + case mcConfirmEmbedded = "mc_embedded_confirm" + + // MARK: - Embedded Payment Element update + case mcUpdateStartedEmbedded = "mc_embedded_update_started" + case mcUpdateFinishedEmbedded = "mc_embedded_update_finished" + // MARK: - PaymentSheet Show case mcShowCustomNewPM = "mc_custom_sheet_newpm_show" case mcShowCustomSavedPM = "mc_custom_sheet_savedpm_show" diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 295d26a74ca..9d37484e759 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -70,7 +70,15 @@ final class PaymentSheetAnalyticsHelper { func logLoadStarted() { loadingStartDate = Date() - log(event: .paymentSheetLoadStarted) + let event: STPAnalyticEvent = { + switch integrationShape { + case .complete, .flowController: + return .paymentSheetLoadStarted + case .embedded: + return .mcLoadStartedEmbedded + } + }() + log(event: event) } func logLoadFailed(error: Error) { @@ -79,8 +87,16 @@ final class PaymentSheetAnalyticsHelper { guard let loadingStartDate else { return 0 } return Date().timeIntervalSince(loadingStartDate) }() + let event: STPAnalyticEvent = { + switch integrationShape { + case .complete, .flowController: + return .paymentSheetLoadFailed + case .embedded: + return .mcLoadFailedEmbedded + } + }() log( - event: .paymentSheetLoadFailed, + event: event, duration: duration, error: error ) @@ -124,8 +140,16 @@ final class PaymentSheetAnalyticsHelper { guard let loadingStartDate else { return 0 } return Date().timeIntervalSince(loadingStartDate) }() + let event: STPAnalyticEvent = { + switch integrationShape { + case .complete, .flowController: + return .paymentSheetLoadSucceeded + case .embedded: + return .mcLoadSucceededEmbedded + } + }() log( - event: .paymentSheetLoadSucceeded, + event: event, duration: duration, params: params ) From a17a8a4eb47b9ecb8d0b1c6912912612c0ead8ec Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Mon, 27 Jan 2025 15:32:15 -0700 Subject: [PATCH 02/16] Update tests for embedded load event --- .../PaymentSheetUITest/EmbeddedUITest.swift | 10 +++++----- .../PaymentSheet/EmbeddedPaymentElementTest.swift | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift index 42ff4d461db..c8fbbb8bdd9 100644 --- a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift +++ b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift @@ -24,7 +24,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { .filter({ !$0.starts(with: "luxe") }) XCTAssertEqual( startupLog, - ["mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_embedded_init"] + ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_init"] ) // Entering a card w/ deferred PaymentIntent... @@ -73,7 +73,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let aliPayAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( aliPayAnalytics, - ["mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_carousel_payment_method_tapped"] + ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_carousel_payment_method_tapped"] ) // ...and *updating* to a SetupIntent... @@ -122,7 +122,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let klarnaAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( klarnaAnalytics, - ["mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_carousel_payment_method_tapped", + ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_carousel_payment_method_tapped", "mc_form_shown", "mc_form_completed", "mc_confirm_button_tapped", ] ) @@ -250,7 +250,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { .prefix(5) XCTAssertEqual( presentEmbeddedLog, - ["mc_load_started", "mc_load_succeeded", "mc_embedded_init", "mc_carousel_payment_method_tapped", "mc_form_shown"] + ["mc_load_started_embedded", "mc_load_succeeded_embedded", "mc_embedded_init", "mc_carousel_payment_method_tapped", "mc_form_shown"] ) // Complete payment @@ -518,7 +518,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let loadEventsWithoutSPMSelection = analyticsLog.compactMap({ $0[string: "event"] }) .filter({ $0.starts(with: "mc_") }).prefix(3) - XCTAssertEqual(loadEventsWithoutSPMSelection, ["mc_load_started", "mc_load_succeeded", "mc_embedded_init"]) + XCTAssertEqual(loadEventsWithoutSPMSelection, ["mc_load_started_embedded", "mc_load_succeeded_embedded", "mc_embedded_init"]) let confirmationEvents = analyticsLog.compactMap({ $0[string: "event"] }) .filter({ $0.starts(with: "mc_") }).suffix(1) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift index c3584474b0c..60286331099 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift @@ -93,8 +93,8 @@ class EmbeddedPaymentElementTest: XCTestCase { // Sanity check that the analytics... let analytics = STPAnalyticsClient.sharedClient._testLogHistory - let loadStartedEvents = analytics.filter { $0["event"] as? String == "mc_load_started" } - let loadSucceededEvents = analytics.filter { $0["event"] as? String == "mc_load_succeeded" } + let loadStartedEvents = analytics.filter { $0["event"] as? String == "mc_load_started_embedded" } + let loadSucceededEvents = analytics.filter { $0["event"] as? String == "mc_load_succeeded_embedded" } // ...have the expected # of start and succeeded events... XCTAssertEqual(loadStartedEvents.count, 3) XCTAssertEqual(loadSucceededEvents.count, 3) From acf29abb40d094a68d390335ee4f7b9a3975db24 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Mon, 27 Jan 2025 16:00:17 -0700 Subject: [PATCH 03/16] Track update analytics --- .../Source/Analytics/STPAnalyticEvent.swift | 2 ++ .../PaymentSheetAnalyticsHelper.swift | 33 +++++++++++++++++++ .../Embedded/EmbeddedPaymentElement.swift | 18 +++++++--- 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift index 99427f3f644..4850d33e3ce 100644 --- a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift +++ b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift @@ -89,6 +89,8 @@ import Foundation // MARK: - Embedded Payment Element update case mcUpdateStartedEmbedded = "mc_embedded_update_started" case mcUpdateFinishedEmbedded = "mc_embedded_update_finished" + case mcUpdateFailedEmbedded = "mc_embedded_update_failed" + case mcUpdateCanceledEmbedded = "mc_embedded_update_canceled" // MARK: - PaymentSheet Show case mcShowCustomNewPM = "mc_custom_sheet_newpm_show" diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 9d37484e759..0bf50ec96cf 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -19,6 +19,7 @@ final class PaymentSheetAnalyticsHelper { var elementsSession: STPElementsSession? var loadingStartDate: Date? private var startTimes: [TimeMeasurement: Date] = [:] + private var updateStartDate: Date? enum IntegrationShape { case flowController @@ -344,6 +345,38 @@ final class PaymentSheetAnalyticsHelper { linkUI: paymentOption.linkUIAnalyticsValue ) } + + func logEmbeddedUpdateStarted() { + stpAssert(integrationShape == .embedded, "This function should only be used with embedded integration") + updateStartDate = Date() + log(event: .mcUpdateStartedEmbedded) + } + + func logEmbeddedUpdateFinished(result: EmbeddedPaymentElement.UpdateResult) { + stpAssert(integrationShape == .embedded, "This function should only be used with embedded integration") + let duration: TimeInterval? = { + guard let updateStartDate else { return nil } + return Date().timeIntervalSince(updateStartDate) + }() + + let event: STPAnalyticEvent = { + switch result { + case .succeeded: + return .mcUpdateFinishedEmbedded + case .failed: + return .mcUpdateFailedEmbedded + case .canceled: + return .mcUpdateCanceledEmbedded + } + }() + + var params: [String: Any] = [:] + if case .failed(let error) = result { + params["error"] = error.localizedDescription + } + + log(event: event, duration: duration, params: params) + } func log( event: STPAnalyticEvent, diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index 9e5b34fa8e1..6f460d20eb4 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -115,6 +115,12 @@ public final class EmbeddedPaymentElement { _ = await latestUpdateTask?.value // Start the new update task let currentUpdateTask = Task { @MainActor [weak self, configuration, analyticsHelper] in + analyticsHelper.logEmbeddedUpdateStarted() + var updateResult: UpdateResult = .canceled // Default to canceled if unhandled + defer { + analyticsHelper.logEmbeddedUpdateFinished(result: updateResult) + } + // ⚠️ Don't modify `self` until the end to avoid being canceled halfway through and leaving self in a partially updated state. // 1. Reload v1/elements/session. let loadResult: PaymentSheetLoader.LoadResult @@ -127,10 +133,12 @@ public final class EmbeddedPaymentElement { integrationShape: .embedded ) } catch { - return UpdateResult.failed(error: error) + updateResult = UpdateResult.failed(error: error) + return updateResult } guard !Task.isCancelled else { - return UpdateResult.canceled + updateResult = UpdateResult.canceled + return updateResult } // Store the old payment option before we update self.formViewController @@ -167,7 +175,8 @@ public final class EmbeddedPaymentElement { _ = await fetchPaymentOption.value guard let self, !Task.isCancelled else { - return .canceled + updateResult = UpdateResult.canceled + return updateResult } // At this point, we're still the latest update and update is successful - update self properties and inform our delegate. self.savedPaymentMethods = loadResult.savedPaymentMethods @@ -179,7 +188,8 @@ public final class EmbeddedPaymentElement { if oldPaymentOption != self.paymentOption { self.delegate?.embeddedPaymentElementDidUpdatePaymentOption(embeddedPaymentElement: self) } - return .succeeded + updateResult = .succeeded + return updateResult } self.latestUpdateTask = currentUpdateTask let updateResult = await currentUpdateTask.value From 9ac020eb5b99683cf707e601897cd5f3c12a9f6e Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Mon, 27 Jan 2025 16:32:28 -0700 Subject: [PATCH 04/16] Fix tests for update metric --- .../PaymentSheetUITest/EmbeddedUITest.swift | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift index c8fbbb8bdd9..7c58025eb7c 100644 --- a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift +++ b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift @@ -73,7 +73,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let aliPayAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( aliPayAnalytics, - ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_carousel_payment_method_tapped"] + ["mc_embedded_update_started", "mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped"] ) // ...and *updating* to a SetupIntent... @@ -122,9 +122,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let klarnaAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( klarnaAnalytics, - ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_carousel_payment_method_tapped", - "mc_form_shown", "mc_form_completed", "mc_confirm_button_tapped", - ] + ["mc_embedded_update_started", "mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped", "mc_form_shown", "mc_form_completed", "mc_confirm_button_tapped"] ) // ...switching back to payment should keep Klarna selected From b0e60bd502168b63cb9e8f4e8d429c224c8b21d8 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Tue, 28 Jan 2025 08:51:26 -0700 Subject: [PATCH 05/16] Add confirm analytic --- .../Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index 6f460d20eb4..cb83af85b90 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -212,6 +212,7 @@ public final class EmbeddedPaymentElement { stpAssertionFailure(errorMessage) return .failed(error: PaymentSheetError.integrationError(nonPIIDebugDescription: errorMessage)) } + analyticsHelper.log(event: .mcConfirmEmbedded) let authContext = STPAuthenticationContextWrapper(presentingViewController: presentingViewController) return await _confirm(paymentOption: paymentOption, authContext: authContext).result } From 231c52667b4989d8b813760cfaf4f5527bfa40ae Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Tue, 28 Jan 2025 09:45:01 -0700 Subject: [PATCH 06/16] Check the confirm analytic --- .../PaymentSheet/EmbeddedPaymentElementTest.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift index 60286331099..eefbf711648 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift @@ -257,6 +257,12 @@ class EmbeddedPaymentElementTest: XCTestCase { case .canceled: XCTFail("Expected confirm to succeed, but it was canceled") } + + // Check our confirm analytics + let analytics = STPAnalyticsClient.sharedClient._testLogHistory + let confirmEvents = analytics.filter { $0["event"] as? String == "mc_embedded_confirm" } + // ...have the expected # of confirm events... + XCTAssertEqual(confirmEvents.count, 1) } func testConfirmWithInvalidCard() async throws { From f88c15411214eb0913c62d06bea03aad231be92d Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Tue, 28 Jan 2025 09:51:49 -0700 Subject: [PATCH 07/16] Add test to helper class --- .../PaymentSheetAnalyticsHelperTest.swift | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index 80154cbfe57..a6dfacd0cd8 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -140,6 +140,36 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { XCTAssertEqual(loadSucceededPayload["intent_type"] as? String, "payment_intent") XCTAssertEqual(loadSucceededPayload["ordered_lpms"] as? String, "card,external_paypal") } + + func testLogLoadFailed_embedded() { + let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + // Load started -> failed + sut.logLoadStarted() + sut.logLoadFailed(error: NSError(domain: "domain", code: 1)) + XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started_embedded") + XCTAssertEqual(analyticsClient._testLogHistory[1]["event"] as? String, "mc_load_failed_embedded") + XCTAssertLessThan(analyticsClient._testLogHistory[1]["duration"] as! Double, 1.0) + } + + func testLogLoadSucceeded_embedded() { + let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + // Load started -> succeeded + sut.logLoadStarted() + sut.logLoadSucceeded( + intent: ._testValue(), + elementsSession: ._testCardValue(), + defaultPaymentMethod: .applePay, + orderedPaymentMethodTypes: [.stripe(.card), .external(._testPayPalValue())] + ) + XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started_embedded") + + let loadSucceededPayload = analyticsClient._testLogHistory[1] + XCTAssertEqual(loadSucceededPayload["event"] as? String, "mc_load_succeeded_embedded") + XCTAssertLessThan(loadSucceededPayload["duration"] as! Double, 1.0) + XCTAssertEqual(loadSucceededPayload["selected_lpm"] as? String, "apple_pay") + XCTAssertEqual(loadSucceededPayload["intent_type"] as? String, "payment_intent") + XCTAssertEqual(loadSucceededPayload["ordered_lpms"] as? String, "card,external_paypal") + } func testLogShow() { let paymentSheetHelper = PaymentSheetAnalyticsHelper(integrationShape: .complete, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) @@ -369,6 +399,33 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { ) XCTAssertEqual(analyticsClient._testLogHistory.last!["link_context"] as? String, "link_card_brand") } + + func testLogEmbeddedUpdate() { + let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + + // Test update started + sut.logEmbeddedUpdateStarted() + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_started_embedded") + + // Test successful update + sut.logEmbeddedUpdateFinished(result: .succeeded) + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_finished_embedded") + XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + + // Test failed update + sut.logEmbeddedUpdateStarted() + let error = NSError(domain: "test", code: 123, userInfo: [NSLocalizedDescriptionKey: "Test error"]) + sut.logEmbeddedUpdateFinished(result: .failed(error: error)) + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_failed_embedded") + XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertEqual(analyticsClient._testLogHistory.last!["error"] as? String, "Test error") + + // Test canceled update + sut.logEmbeddedUpdateStarted() + sut.logEmbeddedUpdateFinished(result: .canceled) + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_canceled_embedded") + XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + } // MARK: - Helpers From d3bcbdd8e20e66091c54f6dd46c55a6bfabab4f1 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Tue, 28 Jan 2025 10:39:47 -0700 Subject: [PATCH 08/16] Fix test --- .../PaymentSheet/PaymentSheetAnalyticsHelperTest.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index a6dfacd0cd8..7b2689a1519 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -405,25 +405,25 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { // Test update started sut.logEmbeddedUpdateStarted() - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_started_embedded") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_started") // Test successful update sut.logEmbeddedUpdateFinished(result: .succeeded) - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_finished_embedded") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) // Test failed update sut.logEmbeddedUpdateStarted() let error = NSError(domain: "test", code: 123, userInfo: [NSLocalizedDescriptionKey: "Test error"]) sut.logEmbeddedUpdateFinished(result: .failed(error: error)) - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_failed_embedded") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_failed") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) XCTAssertEqual(analyticsClient._testLogHistory.last!["error"] as? String, "Test error") // Test canceled update sut.logEmbeddedUpdateStarted() sut.logEmbeddedUpdateFinished(result: .canceled) - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_update_canceled_embedded") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_canceled") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) } From f99d074c6d36f2a64f79dc908fdd19a2d2d9fe82 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Wed, 29 Jan 2025 08:13:22 -0700 Subject: [PATCH 09/16] Reuse mc_load_started --- .../PaymentSheetUITest/EmbeddedUITest.swift | 10 +- .../Source/Analytics/STPAnalyticEvent.swift | 5 - .../PaymentSheetAnalyticsHelper.swift | 46 +++----- .../EmbeddedPaymentElementTest.swift | 4 +- .../PaymentSheetAnalyticsHelperTest.swift | 108 +++++++++--------- 5 files changed, 79 insertions(+), 94 deletions(-) diff --git a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift index 7c58025eb7c..1e482dd3ad0 100644 --- a/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift +++ b/Example/PaymentSheet Example/PaymentSheetUITest/EmbeddedUITest.swift @@ -24,7 +24,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { .filter({ !$0.starts(with: "luxe") }) XCTAssertEqual( startupLog, - ["mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_init"] + ["mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_embedded_init"] ) // Entering a card w/ deferred PaymentIntent... @@ -73,7 +73,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let aliPayAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( aliPayAnalytics, - ["mc_embedded_update_started", "mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped"] + ["mc_embedded_update_started", "mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped"] ) // ...and *updating* to a SetupIntent... @@ -122,7 +122,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let klarnaAnalytics = analyticsLog.compactMap({ $0[string: "event"] }) XCTAssertEqual( klarnaAnalytics, - ["mc_embedded_update_started", "mc_load_started_embedded", "link.account_lookup.complete", "mc_load_succeeded_embedded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped", "mc_form_shown", "mc_form_completed", "mc_confirm_button_tapped"] + ["mc_embedded_update_started", "mc_load_started", "link.account_lookup.complete", "mc_load_succeeded", "mc_embedded_update_finished", "mc_carousel_payment_method_tapped", "mc_form_shown", "mc_form_completed", "mc_confirm_button_tapped"] ) // ...switching back to payment should keep Klarna selected @@ -248,7 +248,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { .prefix(5) XCTAssertEqual( presentEmbeddedLog, - ["mc_load_started_embedded", "mc_load_succeeded_embedded", "mc_embedded_init", "mc_carousel_payment_method_tapped", "mc_form_shown"] + ["mc_load_started", "mc_load_succeeded", "mc_embedded_init", "mc_carousel_payment_method_tapped", "mc_form_shown"] ) // Complete payment @@ -516,7 +516,7 @@ class EmbeddedUITests: PaymentSheetUITestCase { let loadEventsWithoutSPMSelection = analyticsLog.compactMap({ $0[string: "event"] }) .filter({ $0.starts(with: "mc_") }).prefix(3) - XCTAssertEqual(loadEventsWithoutSPMSelection, ["mc_load_started_embedded", "mc_load_succeeded_embedded", "mc_embedded_init"]) + XCTAssertEqual(loadEventsWithoutSPMSelection, ["mc_load_started", "mc_load_succeeded", "mc_embedded_init"]) let confirmationEvents = analyticsLog.compactMap({ $0[string: "event"] }) .filter({ $0.starts(with: "mc_") }).suffix(1) diff --git a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift index 4850d33e3ce..c397614c867 100644 --- a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift +++ b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift @@ -78,11 +78,6 @@ import Foundation // MARK: - Embedded Payment Element init case mcInitEmbedded = "mc_embedded_init" - // MARK: - Embedded Payment Element loading - case mcLoadStartedEmbedded = "mc_load_started_embedded" - case mcLoadSucceededEmbedded = "mc_load_succeeded_embedded" - case mcLoadFailedEmbedded = "mc_load_failed_embedded" - // MARK: - Embedded Payment Element confirm case mcConfirmEmbedded = "mc_embedded_confirm" diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 0bf50ec96cf..03bcef375d0 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -25,6 +25,17 @@ final class PaymentSheetAnalyticsHelper { case flowController case complete case embedded + + var analyticsValue: String { + switch self { + case .flowController: + return "flow_controller" + case .complete: + return "paymentsheet" + case .embedded: + return "embedded" + } + } } init( @@ -71,15 +82,7 @@ final class PaymentSheetAnalyticsHelper { func logLoadStarted() { loadingStartDate = Date() - let event: STPAnalyticEvent = { - switch integrationShape { - case .complete, .flowController: - return .paymentSheetLoadStarted - case .embedded: - return .mcLoadStartedEmbedded - } - }() - log(event: event) + log(event: .paymentSheetLoadStarted, params: ["integration_shape": integrationShape.analyticsValue]) } func logLoadFailed(error: Error) { @@ -88,18 +91,11 @@ final class PaymentSheetAnalyticsHelper { guard let loadingStartDate else { return 0 } return Date().timeIntervalSince(loadingStartDate) }() - let event: STPAnalyticEvent = { - switch integrationShape { - case .complete, .flowController: - return .paymentSheetLoadFailed - case .embedded: - return .mcLoadFailedEmbedded - } - }() log( - event: event, + event: .paymentSheetLoadFailed, duration: duration, - error: error + error: error, + params: ["integration_shape": integrationShape.analyticsValue] ) } @@ -131,6 +127,7 @@ final class PaymentSheetAnalyticsHelper { "selected_lpm": defaultPaymentMethodAnalyticsValue, "intent_type": intent.analyticsValue, "ordered_lpms": orderedPaymentMethodTypes.map({ $0.identifier }).joined(separator: ","), + "integration_shape": integrationShape.analyticsValue ] let linkEnabled: Bool = PaymentSheet.isLinkEnabled(elementsSession: elementsSession, configuration: configuration) if linkEnabled { @@ -141,16 +138,9 @@ final class PaymentSheetAnalyticsHelper { guard let loadingStartDate else { return 0 } return Date().timeIntervalSince(loadingStartDate) }() - let event: STPAnalyticEvent = { - switch integrationShape { - case .complete, .flowController: - return .paymentSheetLoadSucceeded - case .embedded: - return .mcLoadSucceededEmbedded - } - }() + log( - event: event, + event: .paymentSheetLoadSucceeded, duration: duration, params: params ) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift index eefbf711648..293de0e41cb 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentElementTest.swift @@ -93,8 +93,8 @@ class EmbeddedPaymentElementTest: XCTestCase { // Sanity check that the analytics... let analytics = STPAnalyticsClient.sharedClient._testLogHistory - let loadStartedEvents = analytics.filter { $0["event"] as? String == "mc_load_started_embedded" } - let loadSucceededEvents = analytics.filter { $0["event"] as? String == "mc_load_succeeded_embedded" } + let loadStartedEvents = analytics.filter { $0["event"] as? String == "mc_load_started" && $0["integration_shape"] as? String == "embedded" } + let loadSucceededEvents = analytics.filter { $0["event"] as? String == "mc_load_succeeded" && $0["integration_shape"] as? String == "embedded" } // ...have the expected # of start and succeeded events... XCTAssertEqual(loadStartedEvents.count, 3) XCTAssertEqual(loadSucceededEvents.count, 3) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index 7b2689a1519..b40e193dd29 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -112,63 +112,63 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { } func testLogLoadFailed() { - let sut = PaymentSheetAnalyticsHelper(integrationShape: .complete, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) - // Load started -> failed - sut.logLoadStarted() - sut.logLoadFailed(error: NSError(domain: "domain", code: 1)) - XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started") - XCTAssertEqual(analyticsClient._testLogHistory[1]["event"] as? String, "mc_load_failed") - XCTAssertLessThan(analyticsClient._testLogHistory[1]["duration"] as! Double, 1.0) + let integrationShapes: [(PaymentSheetAnalyticsHelper.IntegrationShape, String)] = [ + (.complete, "paymentsheet"), + (.embedded, "embedded"), + (.flowController, "flow_controller") + ] + + for (shape, shapeString) in integrationShapes { + let sut = PaymentSheetAnalyticsHelper(integrationShape: shape, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + + // Reset the analytics client for each iteration + analyticsClient._testLogHistory.removeAll() + + // Load started -> failed + sut.logLoadStarted() + sut.logLoadFailed(error: NSError(domain: "domain", code: 1)) + + XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started") + XCTAssertEqual(analyticsClient._testLogHistory[0]["integration_shape"] as? String, shapeString) + XCTAssertEqual(analyticsClient._testLogHistory[1]["event"] as? String, "mc_load_failed") + XCTAssertLessThan(analyticsClient._testLogHistory[1]["duration"] as! Double, 1.0) + XCTAssertEqual(analyticsClient._testLogHistory[1]["integration_shape"] as? String, shapeString) + } } func testLogLoadSucceeded() { - let sut = PaymentSheetAnalyticsHelper(integrationShape: .complete, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) - // Load started -> succeeded - sut.logLoadStarted() - sut.logLoadSucceeded( - intent: ._testValue(), - elementsSession: ._testCardValue(), - defaultPaymentMethod: .applePay, - orderedPaymentMethodTypes: [.stripe(.card), .external(._testPayPalValue())] - ) - XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started") - - let loadSucceededPayload = analyticsClient._testLogHistory[1] - XCTAssertEqual(loadSucceededPayload["event"] as? String, "mc_load_succeeded") - XCTAssertLessThan(loadSucceededPayload["duration"] as! Double, 1.0) - XCTAssertEqual(loadSucceededPayload["selected_lpm"] as? String, "apple_pay") - XCTAssertEqual(loadSucceededPayload["intent_type"] as? String, "payment_intent") - XCTAssertEqual(loadSucceededPayload["ordered_lpms"] as? String, "card,external_paypal") - } - - func testLogLoadFailed_embedded() { - let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) - // Load started -> failed - sut.logLoadStarted() - sut.logLoadFailed(error: NSError(domain: "domain", code: 1)) - XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started_embedded") - XCTAssertEqual(analyticsClient._testLogHistory[1]["event"] as? String, "mc_load_failed_embedded") - XCTAssertLessThan(analyticsClient._testLogHistory[1]["duration"] as! Double, 1.0) - } - - func testLogLoadSucceeded_embedded() { - let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) - // Load started -> succeeded - sut.logLoadStarted() - sut.logLoadSucceeded( - intent: ._testValue(), - elementsSession: ._testCardValue(), - defaultPaymentMethod: .applePay, - orderedPaymentMethodTypes: [.stripe(.card), .external(._testPayPalValue())] - ) - XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started_embedded") - - let loadSucceededPayload = analyticsClient._testLogHistory[1] - XCTAssertEqual(loadSucceededPayload["event"] as? String, "mc_load_succeeded_embedded") - XCTAssertLessThan(loadSucceededPayload["duration"] as! Double, 1.0) - XCTAssertEqual(loadSucceededPayload["selected_lpm"] as? String, "apple_pay") - XCTAssertEqual(loadSucceededPayload["intent_type"] as? String, "payment_intent") - XCTAssertEqual(loadSucceededPayload["ordered_lpms"] as? String, "card,external_paypal") + let integrationShapes: [(PaymentSheetAnalyticsHelper.IntegrationShape, String)] = [ + (.complete, "paymentsheet"), + (.embedded, "embedded"), + (.flowController, "flow_controller") + ] + + for (shape, shapeString) in integrationShapes { + let sut = PaymentSheetAnalyticsHelper(integrationShape: shape, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + + // Reset the analytics client for each iteration + analyticsClient._testLogHistory.removeAll() + + // Load started -> succeeded + sut.logLoadStarted() + sut.logLoadSucceeded( + intent: ._testValue(), + elementsSession: ._testCardValue(), + defaultPaymentMethod: .applePay, + orderedPaymentMethodTypes: [.stripe(.card), .external(._testPayPalValue())] + ) + + XCTAssertEqual(analyticsClient._testLogHistory[0]["event"] as? String, "mc_load_started") + XCTAssertEqual(analyticsClient._testLogHistory[0]["integration_shape"] as? String, shapeString) + + let loadSucceededPayload = analyticsClient._testLogHistory[1] + XCTAssertEqual(loadSucceededPayload["event"] as? String, "mc_load_succeeded") + XCTAssertLessThan(loadSucceededPayload["duration"] as! Double, 1.0) + XCTAssertEqual(loadSucceededPayload["selected_lpm"] as? String, "apple_pay") + XCTAssertEqual(loadSucceededPayload["intent_type"] as? String, "payment_intent") + XCTAssertEqual(loadSucceededPayload["ordered_lpms"] as? String, "card,external_paypal") + XCTAssertEqual(loadSucceededPayload["integration_shape"] as? String, shapeString) + } } func testLogShow() { From 6a932c6baa737349d61a6eac6830fcd06b9db45a Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Wed, 29 Jan 2025 08:24:00 -0700 Subject: [PATCH 10/16] Consolidate into mc_embedded_update_finished --- .../Source/Analytics/STPAnalyticEvent.swift | 2 -- .../PaymentSheetAnalyticsHelper.swift | 33 +++++++++++-------- .../PaymentSheetAnalyticsHelperTest.swift | 10 ++++-- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift index c397614c867..b0e786d581d 100644 --- a/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift +++ b/StripeCore/StripeCore/Source/Analytics/STPAnalyticEvent.swift @@ -84,8 +84,6 @@ import Foundation // MARK: - Embedded Payment Element update case mcUpdateStartedEmbedded = "mc_embedded_update_started" case mcUpdateFinishedEmbedded = "mc_embedded_update_finished" - case mcUpdateFailedEmbedded = "mc_embedded_update_failed" - case mcUpdateCanceledEmbedded = "mc_embedded_update_canceled" // MARK: - PaymentSheet Show case mcShowCustomNewPM = "mc_custom_sheet_newpm_show" diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 03bcef375d0..54be780417d 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -349,23 +349,15 @@ final class PaymentSheetAnalyticsHelper { return Date().timeIntervalSince(updateStartDate) }() - let event: STPAnalyticEvent = { + let error: Error? = { switch result { - case .succeeded: - return .mcUpdateFinishedEmbedded - case .failed: - return .mcUpdateFailedEmbedded - case .canceled: - return .mcUpdateCanceledEmbedded + case .failed(let error): + return error + default: + return nil } }() - - var params: [String: Any] = [:] - if case .failed(let error) = result { - params["error"] = error.localizedDescription - } - - log(event: event, duration: duration, params: params) + log(event: .mcUpdateFinishedEmbedded, duration: duration, error: error, params: ["status": result.analyticValue]) } func log( @@ -478,3 +470,16 @@ extension PaymentElementConfiguration { return payload } } + +extension EmbeddedPaymentElement.UpdateResult { + var analyticValue: String { + switch self { + case .succeeded: + return "succeeded" + case .canceled: + return "canceled" + case .failed(_): + return "failed" + } + } +} diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index b40e193dd29..4cb671bdce6 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -410,20 +410,24 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { // Test successful update sut.logEmbeddedUpdateFinished(result: .succeeded) XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") + XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "succeeded") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) // Test failed update sut.logEmbeddedUpdateStarted() let error = NSError(domain: "test", code: 123, userInfo: [NSLocalizedDescriptionKey: "Test error"]) sut.logEmbeddedUpdateFinished(result: .failed(error: error)) - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_failed") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") + XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "failed") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) - XCTAssertEqual(analyticsClient._testLogHistory.last!["error"] as? String, "Test error") + XCTAssertEqual(analyticsClient._testLogHistory.last!["error_type"] as? String, "test") + XCTAssertEqual(analyticsClient._testLogHistory.last!["error_code"] as? String, "123") // Test canceled update sut.logEmbeddedUpdateStarted() sut.logEmbeddedUpdateFinished(result: .canceled) - XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_canceled") + XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") + XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "canceled") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) } From 5b915a2411d97a133234272f27c88dd96c7b97d7 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Wed, 29 Jan 2025 08:25:22 -0700 Subject: [PATCH 11/16] Assert we don't send errors for non-failed updates --- .../PaymentSheet/PaymentSheetAnalyticsHelperTest.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index 4cb671bdce6..89b330038b5 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -412,6 +412,8 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "succeeded") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertNil(analyticsClient._testLogHistory.last!["error_type"]) + XCTAssertNil(analyticsClient._testLogHistory.last!["error_code"]) // Test failed update sut.logEmbeddedUpdateStarted() @@ -429,6 +431,8 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "canceled") XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertNil(analyticsClient._testLogHistory.last!["error_type"]) + XCTAssertNil(analyticsClient._testLogHistory.last!["error_code"]) } // MARK: - Helpers From efdcc2d816ee62584fb1c096bf71954f215948a1 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Wed, 29 Jan 2025 08:25:48 -0700 Subject: [PATCH 12/16] Remove extra code --- .../PaymentSheet/PaymentSheetAnalyticsHelperTest.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index 89b330038b5..f890ce8a9cc 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -417,7 +417,7 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { // Test failed update sut.logEmbeddedUpdateStarted() - let error = NSError(domain: "test", code: 123, userInfo: [NSLocalizedDescriptionKey: "Test error"]) + let error = NSError(domain: "test", code: 123) sut.logEmbeddedUpdateFinished(result: .failed(error: error)) XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "failed") From a144ac344ef19aef2eab85d92d655f086fca12da Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Thu, 30 Jan 2025 07:05:03 -0700 Subject: [PATCH 13/16] PR feedback --- .../Source/Analytics/PaymentSheetAnalyticsHelper.swift | 2 +- .../Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift | 2 +- .../PaymentSheet/PaymentSheetAnalyticsHelperTest.swift | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 54be780417d..6c8d89ca1b4 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -29,7 +29,7 @@ final class PaymentSheetAnalyticsHelper { var analyticsValue: String { switch self { case .flowController: - return "flow_controller" + return "flowcontroller" case .complete: return "paymentsheet" case .embedded: diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index cb83af85b90..6f78e82e8f5 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -202,6 +202,7 @@ public final class EmbeddedPaymentElement { /// - Note: This method presents authentication screens on the instance's `presentingViewController` property. /// - Note: This method requires that the last call to `update` succeeded. If the last `update` call failed, this call will fail. If this method is called while a call to `update` is in progress, it waits until the `update` call completes. public func confirm() async -> EmbeddedPaymentElementResult { + analyticsHelper.log(event: .mcConfirmEmbedded) guard let presentingViewController else { let errorMessage = "Presenting view controller is nil. Please set EmbeddedPaymentElement.presentingViewController." stpAssertionFailure(errorMessage) @@ -212,7 +213,6 @@ public final class EmbeddedPaymentElement { stpAssertionFailure(errorMessage) return .failed(error: PaymentSheetError.integrationError(nonPIIDebugDescription: errorMessage)) } - analyticsHelper.log(event: .mcConfirmEmbedded) let authContext = STPAuthenticationContextWrapper(presentingViewController: presentingViewController) return await _confirm(paymentOption: paymentOption, authContext: authContext).result } diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index f890ce8a9cc..028320849c2 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -115,7 +115,7 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { let integrationShapes: [(PaymentSheetAnalyticsHelper.IntegrationShape, String)] = [ (.complete, "paymentsheet"), (.embedded, "embedded"), - (.flowController, "flow_controller") + (.flowController, "flowcontroller") ] for (shape, shapeString) in integrationShapes { @@ -140,7 +140,7 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { let integrationShapes: [(PaymentSheetAnalyticsHelper.IntegrationShape, String)] = [ (.complete, "paymentsheet"), (.embedded, "embedded"), - (.flowController, "flow_controller") + (.flowController, "flowcontroller") ] for (shape, shapeString) in integrationShapes { From e95c8979eba45fc28b97c23a0c937ec792209eee Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Thu, 30 Jan 2025 13:37:43 -0700 Subject: [PATCH 14/16] Move update analytics to top and bottom of function --- .../PaymentSheetAnalyticsHelper.swift | 8 +------- .../Embedded/EmbeddedPaymentElement.swift | 19 +++++++------------ .../PaymentSheetAnalyticsHelperTest.swift | 13 +++++++------ 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift index 6c8d89ca1b4..c227ab83ad5 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/Analytics/PaymentSheetAnalyticsHelper.swift @@ -19,7 +19,6 @@ final class PaymentSheetAnalyticsHelper { var elementsSession: STPElementsSession? var loadingStartDate: Date? private var startTimes: [TimeMeasurement: Date] = [:] - private var updateStartDate: Date? enum IntegrationShape { case flowController @@ -338,16 +337,11 @@ final class PaymentSheetAnalyticsHelper { func logEmbeddedUpdateStarted() { stpAssert(integrationShape == .embedded, "This function should only be used with embedded integration") - updateStartDate = Date() log(event: .mcUpdateStartedEmbedded) } - func logEmbeddedUpdateFinished(result: EmbeddedPaymentElement.UpdateResult) { + func logEmbeddedUpdateFinished(result: EmbeddedPaymentElement.UpdateResult, duration: TimeInterval) { stpAssert(integrationShape == .embedded, "This function should only be used with embedded integration") - let duration: TimeInterval? = { - guard let updateStartDate else { return nil } - return Date().timeIntervalSince(updateStartDate) - }() let error: Error? = { switch result { diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index 6f78e82e8f5..fe18adc2bcb 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -104,9 +104,13 @@ public final class EmbeddedPaymentElement { public func update( intentConfiguration: IntentConfiguration ) async -> UpdateResult { + let startTime = Date() + analyticsHelper.logEmbeddedUpdateStarted() // Do not process any update calls if we have already successfully confirmed an intent guard !hasConfirmedIntent else { - return .failed(error: PaymentSheetError.embeddedPaymentElementAlreadyConfirmedIntent) + let result: EmbeddedPaymentElement.UpdateResult = .failed(error: PaymentSheetError.embeddedPaymentElementAlreadyConfirmedIntent) + analyticsHelper.logEmbeddedUpdateFinished(result: result, duration: Date().timeIntervalSince(startTime)) + return result } embeddedPaymentMethodsView.isUserInteractionEnabled = false @@ -115,12 +119,6 @@ public final class EmbeddedPaymentElement { _ = await latestUpdateTask?.value // Start the new update task let currentUpdateTask = Task { @MainActor [weak self, configuration, analyticsHelper] in - analyticsHelper.logEmbeddedUpdateStarted() - var updateResult: UpdateResult = .canceled // Default to canceled if unhandled - defer { - analyticsHelper.logEmbeddedUpdateFinished(result: updateResult) - } - // ⚠️ Don't modify `self` until the end to avoid being canceled halfway through and leaving self in a partially updated state. // 1. Reload v1/elements/session. let loadResult: PaymentSheetLoader.LoadResult @@ -133,12 +131,9 @@ public final class EmbeddedPaymentElement { integrationShape: .embedded ) } catch { - updateResult = UpdateResult.failed(error: error) - return updateResult + return UpdateResult.failed(error: error) } guard !Task.isCancelled else { - updateResult = UpdateResult.canceled - return updateResult } // Store the old payment option before we update self.formViewController @@ -177,6 +172,7 @@ public final class EmbeddedPaymentElement { guard let self, !Task.isCancelled else { updateResult = UpdateResult.canceled return updateResult + return .canceled } // At this point, we're still the latest update and update is successful - update self properties and inform our delegate. self.savedPaymentMethods = loadResult.savedPaymentMethods @@ -202,7 +198,6 @@ public final class EmbeddedPaymentElement { /// - Note: This method presents authentication screens on the instance's `presentingViewController` property. /// - Note: This method requires that the last call to `update` succeeded. If the last `update` call failed, this call will fail. If this method is called while a call to `update` is in progress, it waits until the `update` call completes. public func confirm() async -> EmbeddedPaymentElementResult { - analyticsHelper.log(event: .mcConfirmEmbedded) guard let presentingViewController else { let errorMessage = "Presenting view controller is nil. Please set EmbeddedPaymentElement.presentingViewController." stpAssertionFailure(errorMessage) diff --git a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift index 028320849c2..b2df51d34f5 100644 --- a/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift +++ b/StripePaymentSheet/StripePaymentSheetTests/PaymentSheet/PaymentSheetAnalyticsHelperTest.swift @@ -402,35 +402,36 @@ final class PaymentSheetAnalyticsHelperTest: XCTestCase { func testLogEmbeddedUpdate() { let sut = PaymentSheetAnalyticsHelper(integrationShape: .embedded, configuration: PaymentSheet.Configuration(), analyticsClient: analyticsClient) + let testDuration: TimeInterval = 10.5 // Test update started sut.logEmbeddedUpdateStarted() XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_started") // Test successful update - sut.logEmbeddedUpdateFinished(result: .succeeded) + sut.logEmbeddedUpdateFinished(result: .succeeded, duration: testDuration) XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "succeeded") - XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertEqual(analyticsClient._testLogHistory.last!["duration"] as? TimeInterval, testDuration) XCTAssertNil(analyticsClient._testLogHistory.last!["error_type"]) XCTAssertNil(analyticsClient._testLogHistory.last!["error_code"]) // Test failed update sut.logEmbeddedUpdateStarted() let error = NSError(domain: "test", code: 123) - sut.logEmbeddedUpdateFinished(result: .failed(error: error)) + sut.logEmbeddedUpdateFinished(result: .failed(error: error), duration: testDuration) XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "failed") - XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertEqual(analyticsClient._testLogHistory.last!["duration"] as? TimeInterval, testDuration) XCTAssertEqual(analyticsClient._testLogHistory.last!["error_type"] as? String, "test") XCTAssertEqual(analyticsClient._testLogHistory.last!["error_code"] as? String, "123") // Test canceled update sut.logEmbeddedUpdateStarted() - sut.logEmbeddedUpdateFinished(result: .canceled) + sut.logEmbeddedUpdateFinished(result: .canceled, duration: testDuration) XCTAssertEqual(analyticsClient._testLogHistory.last!["event"] as? String, "mc_embedded_update_finished") XCTAssertEqual(analyticsClient._testLogHistory.last!["status"] as? String, "canceled") - XCTAssertNotNil(analyticsClient._testLogHistory.last!["duration"]) + XCTAssertEqual(analyticsClient._testLogHistory.last!["duration"] as? TimeInterval, testDuration) XCTAssertNil(analyticsClient._testLogHistory.last!["error_type"]) XCTAssertNil(analyticsClient._testLogHistory.last!["error_code"]) } From 7b433eb3176476e44ff9c2911cf0a7ada7477457 Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Thu, 30 Jan 2025 13:39:40 -0700 Subject: [PATCH 15/16] Add back lines --- .../PaymentSheet/Embedded/EmbeddedPaymentElement.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index fe18adc2bcb..61a4070bcb5 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -134,6 +134,7 @@ public final class EmbeddedPaymentElement { return UpdateResult.failed(error: error) } guard !Task.isCancelled else { + return UpdateResult.canceled } // Store the old payment option before we update self.formViewController @@ -170,8 +171,6 @@ public final class EmbeddedPaymentElement { _ = await fetchPaymentOption.value guard let self, !Task.isCancelled else { - updateResult = UpdateResult.canceled - return updateResult return .canceled } // At this point, we're still the latest update and update is successful - update self properties and inform our delegate. @@ -184,12 +183,12 @@ public final class EmbeddedPaymentElement { if oldPaymentOption != self.paymentOption { self.delegate?.embeddedPaymentElementDidUpdatePaymentOption(embeddedPaymentElement: self) } - updateResult = .succeeded - return updateResult + return .succeeded } self.latestUpdateTask = currentUpdateTask let updateResult = await currentUpdateTask.value embeddedPaymentMethodsView.isUserInteractionEnabled = true + analyticsHelper.logEmbeddedUpdateFinished(result: updateResult, duration: Date().timeIntervalSince(startTime)) return updateResult } From 96027477cc782222f74d8dc4433b92c6d2255dad Mon Sep 17 00:00:00 2001 From: Nick Porter Date: Thu, 30 Jan 2025 13:41:08 -0700 Subject: [PATCH 16/16] Add confirm metric --- .../Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift index 61a4070bcb5..ac7b8f293e7 100644 --- a/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift +++ b/StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement.swift @@ -197,6 +197,7 @@ public final class EmbeddedPaymentElement { /// - Note: This method presents authentication screens on the instance's `presentingViewController` property. /// - Note: This method requires that the last call to `update` succeeded. If the last `update` call failed, this call will fail. If this method is called while a call to `update` is in progress, it waits until the `update` call completes. public func confirm() async -> EmbeddedPaymentElementResult { + analyticsHelper.log(event: .mcConfirmEmbedded) guard let presentingViewController else { let errorMessage = "Presenting view controller is nil. Please set EmbeddedPaymentElement.presentingViewController." stpAssertionFailure(errorMessage)