-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
MBL-1285: Integrate ApplePay into Post-Campaign Checkout #2010
Conversation
a753a68
to
eaffab0
Compare
PostCampaignCheckoutApplePayError.missingPaymentIntent("Missing PaymentIntent") | ||
) | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stripe assumes you'll request the payment intent asynchronously from inside this method, but in our case, we request it immediately when you tap the ApplePay button.
transactionId | ||
)) | ||
// Tokens considered 'legacy' from Stripe's side, but we still store the token ourselves. | ||
STPAPIClient.shared.createToken(with: payment) { [weak self] token, _ in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the one manual call we make to Stripe, and it's because our backend requires the token.
self.viewModel.inputs.applePayContextDidComplete() | ||
case .error: | ||
self.messageBannerViewController? | ||
.showBanner(with: .error, message: Strings.Something_went_wrong_please_try_again()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that isn't completely clear to me - if Stripe errors at this point, has the payment been confirmed? I assume not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. have you tried testing with a failure test card and then looking in the stripe dashboard (with Bessie's help) to see if it went through?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way, we'll need to run our validations/checkout ID calls and reconfirm again, but it would be good to know.
|
||
let newPaymentIntentForApplePay: Signal<String, Never> = self.configureWithDataProperty.signal | ||
.skipNil() | ||
.takeWhen(self.applePayButtonTappedSignal) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a fast follow, I'll create a ticket to make sure the Apple Pay button can't be tapped twice.
.configureWithDataProperty | ||
.signal | ||
.skipNil() | ||
.map { (data: PostCampaignCheckoutData) -> PostCampaignPaymentAuthorizationData? in | ||
.combineLatest(with: newPaymentIntentForApplePay) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This signal shouldn't fire until you have created a payment intent, which only happens after tapping the ApplePay button.
} | ||
|
||
private let (paymentAuthorizationDidFinishSignal, paymentAuthorizationDidFinishObserver) | ||
private let (applePayContextDidCompleteSignal, applePayContextDidCompleteObserver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Significantly simpler!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
self.viewModel.inputs.applePayContextDidComplete() | ||
case .error: | ||
self.messageBannerViewController? | ||
.showBanner(with: .error, message: Strings.Something_went_wrong_please_try_again()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. have you tried testing with a failure test card and then looking in the stripe dashboard (with Bessie's help) to see if it went through?
Order of operations: | ||
1) Apple pay button tapped | ||
2) Generate a new payment intent | ||
3) Present the payment authorization form | ||
4) Payment authorization form calls applePayContextDidCreatePayment with ApplePay params | ||
5) Payment authorization form calls paymentAuthorizationDidFinish | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
self.viewModel.inputs.applePayContextDidComplete() | ||
case .error: | ||
self.messageBannerViewController? | ||
.showBanner(with: .error, message: Strings.Something_went_wrong_please_try_again()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way, we'll need to run our validations/checkout ID calls and reconfirm again, but it would be good to know.
📲 What
MBL-1285: Integrate ApplePay into Post-Campaign Checkout, using
STPApplePayContext
.🤔 Why
This is the last remaining piece to complete the "happy path" for late-pledge payments.
🛠 How
I chose to swap from directly using
PKPaymentAuthorizationViewController
toSTPApplePayContext
, which is a Stripe wrapper aroundPKPaymentAuthorizationViewController
. The advantage ofSTPApplePayContext
is that it handles creating the payment method, associating it with the payment intent, and confirming the payment, saving us from making extra calls to Stripe and adding extra complexity to our view model. See their documentation here.