Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail & log payment attempt for a deferred intent with a next action #4026

Merged
merged 23 commits into from
Sep 23, 2024

Conversation

joyceqin-stripe
Copy link
Collaborator

Summary

Added an STPAnalyticsEvent paymentSheetIntentPaymentMethodIdMismatch and analytics log to deferred validator and analytics test

Motivation

https://jira.corp.stripe.com/browse/RUN_MOBILESDK-3540

Testing

Asserted that the log fired after deferred validation failure matched the paymentSheetIntentPaymentMethodIdMismatch event

Changelog

Copy link
Collaborator

@porter-stripe porter-stripe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, minor nit's

@@ -194,6 +194,7 @@ import Foundation
case paymentSheetFormShown = "mc_form_shown"
case paymentSheetFormInteracted = "mc_form_interacted"
case paymentSheetCardNumberCompleted = "mc_card_number_completed"
case paymentSheetIntentPaymentMethodIdMismatch = "mc_intent_payment_method_id_mismatch"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's rename this to mc_deferred_intent_payment_method_id_mismatch

@@ -53,6 +54,8 @@ struct PaymentSheetDeferredValidator {
return
}
guard intentPaymentMethod.stripeId == paymentMethod.stripeId else {
let errorAnalytic = ErrorAnalytic(event: .paymentSheetIntentPaymentMethodIdMismatch, error: PaymentSheetError.unknown(debugDescription: "Payment method ids don't match"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use the same error message in debugDescription as we do in the message below in the error we throw.

Copy link

⚠️ Public API changes detected:

StripePaymentSheet

- public init(mode: StripePaymentSheet.PaymentSheet.IntentConfiguration.Mode, paymentMethodTypes: [Swift.String]? = nil, onBehalfOf: Swift.String? = nil, paymentMethodConfigurationId: Swift.String? = nil, confirmHandler: @escaping StripePaymentSheet.PaymentSheet.IntentConfiguration.ConfirmHandler, requireCVCRecollection: Swift.Bool = false)
+ public init(mode: StripePaymentSheet.PaymentSheet.IntentConfiguration.Mode, paymentMethodTypes: [Swift.String]? = nil, onBehalfOf: Swift.String? = nil, paymentMethodConfigurationId: Swift.String? = nil, confirmHandler: @escaping StripePaymentSheet.PaymentSheet.IntentConfiguration.ConfirmHandler)
- public var requireCVCRecollection: Swift.Bool

If you are adding a new public API consider the following:

  • Do these APIs need to be public or can they be protected with @_spi(STP)?
  • If these APIs need to be public, assess whether they require an API review.

If you are modifying or removing a public API:

  • Does this require a breaking version change?
  • Do these changes require API review?

If you confirm these APIs need to be added/updated and have undergone necessary review, add the label modifies public API to this PR to acknowledge and bypass this check.

ℹ️ If this comment appears to be left in error, make sure your branch is up-to-date with master.

@joyceqin-stripe joyceqin-stripe enabled auto-merge (squash) September 23, 2024 16:49
@joyceqin-stripe joyceqin-stripe merged commit 757bb89 into master Sep 23, 2024
5 checks passed
@joyceqin-stripe joyceqin-stripe deleted the joyceqin-RUN_MOBILESDK-3540 branch September 23, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants