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.

Sign up for GitHub

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

Jump to bottom

Fix createsCardSources <> didCreatePaymentResult , Apple Pay #893

Merged
merged 5 commits into from
Feb 2, 2018
Merged

Fix createsCardSources <> didCreatePaymentResult , Apple Pay #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
09d1d31
allow creating source when apple pay is selected
bg-stripe Feb 2, 2018
06b63f0
create apple pay source when createCardSources is enabled
bg-stripe Feb 2, 2018
c724928
fix issue where didCreatePAymentResult was not called for sources
bg-stripe Feb 2, 2018
c8ed039
always createCardSources in example app
bg-stripe Feb 2, 2018
eb09444
fix analyzer warning
bg-stripe Feb 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Example/Standard Integration (Swift)/CheckoutViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class CheckoutViewController: UIViewController, STPPaymentContextDelegate {
config.shippingType = settings.shippingType
config.additionalPaymentMethods = settings.additionalPaymentMethods

// Create card sources instead of card tokens
config.createCardSources = true;

let customerContext = STPCustomerContext(keyProvider: MyAPIClient.sharedClient)
let paymentContext = STPPaymentContext(customerContext: customerContext,
configuration: config,
Expand Down
5 changes: 3 additions & 2 deletions Stripe/PKPaymentAuthorizationViewController+Stripe_Blocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

NS_ASSUME_NONNULL_BEGIN

typedef void(^STPApplePayTokenHandlerBlock)(STPToken *token, STPErrorBlock completion);
typedef void(^STPApplePaySourceHandlerBlock)(id<STPSourceProtocol> source, STPErrorBlock completion);
typedef void (^STPPaymentCompletionBlock)(STPPaymentStatus status, NSError * __nullable error);
typedef void (^STPPaymentSummaryItemCompletionBlock)(NSArray<PKPaymentSummaryItem*> *summaryItems);
typedef void (^STPShippingMethodSelectionBlock)(PKShippingMethod *selectedMethod, STPPaymentSummaryItemCompletionBlock completion);
Expand All @@ -26,10 +26,11 @@ typedef void (^STPPaymentAuthorizationBlock)(PKPayment *payment);

+ (instancetype)stp_controllerWithPaymentRequest:(PKPaymentRequest *)paymentRequest
apiClient:(STPAPIClient *)apiClient
createSource:(BOOL)createSource
onShippingAddressSelection:(STPShippingAddressSelectionBlock)onShippingAddressSelection
onShippingMethodSelection:(STPShippingMethodSelectionBlock)onShippingMethodSelection
onPaymentAuthorization:(STPPaymentAuthorizationBlock)onPaymentAuthorization
onTokenCreation:(STPApplePayTokenHandlerBlock)onTokenCreation
onTokenCreation:(STPApplePaySourceHandlerBlock)onTokenCreation
onFinish:(STPPaymentCompletionBlock)onFinish;


Expand Down
38 changes: 29 additions & 9 deletions Stripe/PKPaymentAuthorizationViewController+Stripe_Blocks.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

#import "PKPaymentAuthorizationViewController+Stripe_Blocks.h"
#import "STPAPIClient+ApplePay.h"

#import "STPCard.h"
#import "STPSource.h"
#import "STPToken.h"

static char kSTPBlockBasedApplePayDelegateAssociatedObjectKey;

Expand All @@ -22,10 +24,11 @@ @interface STPBlockBasedApplePayDelegate : NSObject <PKPaymentAuthorizationViewC
@property (nonatomic, copy) STPShippingAddressSelectionBlock onShippingAddressSelection;
@property (nonatomic, copy) STPShippingMethodSelectionBlock onShippingMethodSelection;
@property (nonatomic, copy) STPPaymentAuthorizationBlock onPaymentAuthorization;
@property (nonatomic, copy) STPApplePayTokenHandlerBlock onTokenCreation;
@property (nonatomic, copy) STPApplePaySourceHandlerBlock onSourceCreation;
@property (nonatomic, copy) STPPaymentCompletionBlock onFinish;
@property (nonatomic) NSError *lastError;
@property (nonatomic) BOOL didSucceed;
@property (nonatomic) BOOL createSource;
@end

typedef void (^STPPaymentAuthorizationStatusCallback)(PKPaymentAuthorizationStatus status);
Expand All @@ -35,22 +38,37 @@ @implementation STPBlockBasedApplePayDelegate
- (void)paymentAuthorizationViewController:(__unused PKPaymentAuthorizationViewController *)controller
didAuthorizePayment:(PKPayment *)payment completion:(STPPaymentAuthorizationStatusCallback)completion {
self.onPaymentAuthorization(payment);
[self.apiClient createTokenWithPayment:payment completion:^(STPToken * _Nullable token, NSError * _Nullable error) {

void(^tokenOrSourceCompletion)(id<STPSourceProtocol>, NSError *) = ^(id<STPSourceProtocol> result, NSError *error) {
if (error) {
self.lastError = error;
completion(PKPaymentAuthorizationStatusFailure);
return;
}
self.onTokenCreation(token, ^(NSError *tokenCreationError){
if (tokenCreationError) {
self.lastError = tokenCreationError;
id<STPSourceProtocol> source;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is source used for? It kind of looks like you meant to pass source to the onSourceCreation() call, but it's getting result right now, so I think this is dead code.

edit: neat, static analyzer agrees with me and failed the build 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm more confident that you meant to pass source instead of result: down in the definition of applePaySourceHandler, the code used to be constructing a STPPaymentResult with token.card, and now it just passes source directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh derp, good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so i think i understand why this worked (returning STPToken instead of the child STPCard for an apple pay source). we don't display apple pay sources in the UI, so it didn't matter that STPToken doesn't conform to <STPPaymentMethod> (the protocol defining how a payment method is displayed). i'll still fix for consistency with how we handle non-apple pay sources.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in eb09444, re-r?

// return the child card, not the STPToken
if ([result isKindOfClass:[STPToken class]]) {
source = ((STPToken *)result).card;
}
else if ([result isKindOfClass:[STPSource class]]) {
source = (STPSource *)result;
}
self.onSourceCreation(source, ^(NSError *sourceCreation){
if (sourceCreation) {
self.lastError = sourceCreation;
completion(PKPaymentAuthorizationStatusFailure);
return;
}
self.didSucceed = YES;
completion(PKPaymentAuthorizationStatusSuccess);
});
}];
};
if (self.createSource) {
[self.apiClient createSourceWithPayment:payment completion:(STPSourceCompletionBlock)tokenOrSourceCompletion];
}
else {
[self.apiClient createTokenWithPayment:payment completion:(STPTokenCompletionBlock)tokenOrSourceCompletion];
}
}

- (void)paymentAuthorizationViewController:(__unused PKPaymentAuthorizationViewController *)controller
Expand Down Expand Up @@ -97,17 +115,19 @@ @implementation PKPaymentAuthorizationViewController (Stripe_Blocks)

+ (instancetype)stp_controllerWithPaymentRequest:(PKPaymentRequest *)paymentRequest
apiClient:(STPAPIClient *)apiClient
createSource:(BOOL)createSource
onShippingAddressSelection:(STPShippingAddressSelectionBlock)onShippingAddressSelection
onShippingMethodSelection:(STPShippingMethodSelectionBlock)onShippingMethodSelection
onPaymentAuthorization:(STPPaymentAuthorizationBlock)onPaymentAuthorization
onTokenCreation:(STPApplePayTokenHandlerBlock)onTokenCreation
onTokenCreation:(STPApplePaySourceHandlerBlock)onTokenCreation
onFinish:(STPPaymentCompletionBlock)onFinish {
STPBlockBasedApplePayDelegate *delegate = [STPBlockBasedApplePayDelegate new];
delegate.apiClient = apiClient;
delegate.createSource = createSource;
delegate.onShippingAddressSelection = onShippingAddressSelection;
delegate.onShippingMethodSelection = onShippingMethodSelection;
delegate.onPaymentAuthorization = onPaymentAuthorization;
delegate.onTokenCreation = onTokenCreation;
delegate.onSourceCreation = onTokenCreation;
delegate.onFinish = onFinish;
PKPaymentAuthorizationViewController *viewController = [[self alloc] initWithPaymentRequest:paymentRequest];
viewController.delegate = delegate;
Expand Down
22 changes: 12 additions & 10 deletions Stripe/STPPaymentContext.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,10 @@ - (void)requestPayment {
else if ([self requestPaymentShouldPresentShippingViewController]) {
[self presentShippingViewControllerWithNewState:STPPaymentContextStateRequestingPayment];
}
else if ([self.selectedPaymentMethod isKindOfClass:[STPCard class]]) {
else if ([self.selectedPaymentMethod isKindOfClass:[STPCard class]] ||
[self.selectedPaymentMethod isKindOfClass:[STPSource class]]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about checking [self.selectedPaymentMethod conformsToProtocol:@protocol(STPSourceProtocol)] instead?

I don't know:

  • whether there are other classes that conform to the protocol at we want excluded from this branch (making the check invalid)
  • how likely it is that we'll add new classes that do conform to the protocol that should take this branch (updated check would future-proof us)
  • how likely that we'll remove conformance to the protocol from these classes (which would break the check)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mm, interesting idea. I think it's unlikely that we'll create other class that conform to this protocol (the protocol is mostly a compatibility shim for card tokens to work with sources). I think we should leave it this way and explicitly reference STPCard, which we should start treating as a legacy class (card tokens will continue to be supported for a long time, but sources are the preferred integration).

self.state = STPPaymentContextStateRequestingPayment;
STPPaymentResult *result = [[STPPaymentResult alloc] initWithSource:(STPCard *)self.selectedPaymentMethod];
STPPaymentResult *result = [[STPPaymentResult alloc] initWithSource:(id<STPSourceProtocol>)self.selectedPaymentMethod];
[self.delegate paymentContext:self didCreatePaymentResult:result completion:^(NSError * _Nullable error) {
stpDispatchToMainThreadIfNecessary(^{
if (error) {
Expand Down Expand Up @@ -625,19 +626,19 @@ - (void)requestPayment {
[customerContext updateCustomerWithShippingAddress:self.shippingAddress completion:nil];
}
};
STPApplePayTokenHandlerBlock applePayTokenHandler = ^(STPToken *token, STPErrorBlock tokenCompletion) {
[self.apiAdapter attachSourceToCustomer:token completion:^(NSError *tokenError) {
STPApplePaySourceHandlerBlock applePaySourceHandler = ^(id<STPSourceProtocol> source, STPErrorBlock completion) {
[self.apiAdapter attachSourceToCustomer:source completion:^(NSError *attachSourceError) {
stpDispatchToMainThreadIfNecessary(^{
if (tokenError) {
tokenCompletion(tokenError);
if (attachSourceError) {
completion(attachSourceError);
} else {
STPPaymentResult *result = [[STPPaymentResult alloc] initWithSource:token.card];
STPPaymentResult *result = [[STPPaymentResult alloc] initWithSource:source];
[self.delegate paymentContext:self didCreatePaymentResult:result completion:^(NSError * error) {
// for Apple Pay, the didFinishWithStatus callback is fired later when Apple Pay VC finishes
if (error) {
tokenCompletion(error);
completion(error);
} else {
tokenCompletion(nil);
completion(nil);
}
}];
}
Expand All @@ -648,10 +649,11 @@ - (void)requestPayment {
paymentAuthVC = [PKPaymentAuthorizationViewController
stp_controllerWithPaymentRequest:paymentRequest
apiClient:self.apiClient
createSource:self.configuration.createCardSources
onShippingAddressSelection:shippingAddressHandler
onShippingMethodSelection:shippingMethodHandler
onPaymentAuthorization:paymentHandler
onTokenCreation:applePayTokenHandler
onTokenCreation:applePaySourceHandler
onFinish:^(STPPaymentStatus status, NSError * _Nullable error) {
[self.hostViewController dismissViewControllerAnimated:[self transitionAnimationsEnabled]
completion:^{
Expand Down