Skip to content

Commit

Permalink
Change Styleguide around acronym capitalization, and fix recent Payme…
Browse files Browse the repository at this point in the history
…ntIntent code #1037

Fixes the styleguide to match the codebase, and updates recent code that was written
to follow the (incorrect) styleguide re: URL vs Url.

Only one public-facing property needed changing. Deprecated it, along with a fixit to
make the upgrade path easier for users. Everything else was private or test-only.
  • Loading branch information
danj-stripe authored Nov 13, 2018
2 parents 40f29e8 + 1a054f3 commit bd57e8d
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ - (void)pay {
STPAPIClient *stripeClient = [STPAPIClient sharedClient];
STPPaymentIntentParams *paymentIntentParams = [[STPPaymentIntentParams alloc] initWithClientSecret:clientSecret];
paymentIntentParams.sourceParams = [STPSourceParams cardParamsWithCard:self.paymentTextField.cardParams];
paymentIntentParams.returnUrl = @"payments-example://stripe-redirect";
paymentIntentParams.returnURL = @"payments-example://stripe-redirect";

[stripeClient confirmPaymentIntentWithParams:paymentIntentParams completion:^(STPPaymentIntent * _Nullable paymentIntent, NSError * _Nullable error) {
if (error) {
Expand Down
1 change: 1 addition & 0 deletions MIGRATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Changes to the `STPCardParams` object after setting `cardParams` no longer mutate the object held by the `STPPaymentCardTextField`
* Changes to the object returned by `STPPaymentCardTextField.cardParams` no longer mutate the object held by the `STPPaymentCardTextField`
* This is a breaking change for code like: `paymentCardTextField.cardParams.name = @"Jane Doe";`
* `STPPaymentIntentParams.returnUrl` has been renamed to `STPPaymentIntentParams.returnURL`. Xcode should offer a deprecation warning & fix-it to help you migrate.

### Migrating from versions < 13.1.0
* The SDK now supports PaymentIntents with `STPPaymentIntent`, which use `STPRedirectContext` in the same way that `STPSource` does
Expand Down
2 changes: 1 addition & 1 deletion STYLEGUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

- Avoid single letter variables. Try using `idx` / `jdx` instead of `i` / `j` in for loops.

- Prefer `urlString` over `URLString` (acronym prefix), `baseUrlString` over `baseURLString` (acronym infix), and `stripeId` over `stripeID` (acronym suffix)
- Acronyms should be all lowercase as a method prefix (ex:`url` or `urlString`). Otherwise, they should be all caps when occurring elsewhere in the method name, or as a class name (ex: `handleStripeURLCallbackWithURL`, `stripeID` or `STPAPIClient`)

### Control Flow

Expand Down
10 changes: 9 additions & 1 deletion Stripe/PublicHeaders/STPPaymentIntentParams.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,15 @@ NS_ASSUME_NONNULL_BEGIN
their payment on the payment method’s app or site.
This should probably be a URL that opens your iOS app.
*/
@property (nonatomic, copy, nullable, readwrite) NSString *returnUrl;
@property (nonatomic, copy, nullable, readwrite) NSString *returnURL;

/**
The URL to redirect your customer back to after they authenticate or cancel
their payment on the payment method’s app or site.
This property has been renamed to `returnURL` and deprecated.
*/
@property (nonatomic, copy, nullable, readwrite) NSString *returnUrl __attribute__((deprecated("returnUrl has been renamed to returnURL", "returnURL")));

@end

Expand Down
14 changes: 12 additions & 2 deletions Stripe/STPPaymentIntentParams.m
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ - (NSString *)description {
// PaymentIntentParams details (alphabetical)
[NSString stringWithFormat:@"clientSecret = %@", (self.clientSecret.length > 0) ? @"<redacted>" : @""],
[NSString stringWithFormat:@"receiptEmail = %@", self.receiptEmail],
[NSString stringWithFormat:@"returnUrl = %@", self.returnUrl],
[NSString stringWithFormat:@"returnURL = %@", self.returnURL],
[NSString stringWithFormat:@"saveSourceToCustomer = %@", (self.saveSourceToCustomer.boolValue) ? @"YES" : @"NO"],
[NSString stringWithFormat:@"sourceId = %@", self.sourceId],
[NSString stringWithFormat:@"sourceParams = %@", self.sourceParams],
Expand All @@ -54,6 +54,16 @@ - (NSString *)description {
return [NSString stringWithFormat:@"<%@>", [props componentsJoinedByString:@"; "]];
}

#pragma mark - Deprecated Properties

- (NSString *)returnUrl {
return self.returnURL;
}

- (void)setReturnUrl:(NSString *)returnUrl {
self.returnURL = returnUrl;
}

#pragma mark - STPFormEncodable

+ (nullable NSString *)rootObjectName {
Expand All @@ -67,7 +77,7 @@ + (nonnull NSDictionary *)propertyNamesToFormFieldNamesMapping {
NSStringFromSelector(@selector(sourceId)): @"source",
NSStringFromSelector(@selector(receiptEmail)): @"receipt_email",
NSStringFromSelector(@selector(saveSourceToCustomer)): @"save_source_to_customer",
NSStringFromSelector(@selector(returnUrl)): @"return_url",
NSStringFromSelector(@selector(returnURL)): @"return_url",
};
}

Expand Down
12 changes: 6 additions & 6 deletions Stripe/STPRedirectContext+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ NS_ASSUME_NONNULL_BEGIN

@interface STPRedirectContext()

/// Optional URL for a native app. This is passed directly to `UIApplication openURL:`, and if it fails this class falls back to `redirectUrl`
@property (nonatomic, nullable, copy) NSURL *nativeRedirectUrl;
/// The URL to redirect to, assuming `nativeRedirectUrl` is nil or fails to open. Cannot be nil if `nativeRedirectUrl` is.
@property (nonatomic, nullable, copy) NSURL *redirectUrl;
/// The expected `returnUrl`, passed to STPURLCallbackHandler
@property (nonatomic, copy) NSURL *returnUrl;
/// Optional URL for a native app. This is passed directly to `UIApplication openURL:`, and if it fails this class falls back to `redirectURL`
@property (nonatomic, nullable, copy) NSURL *nativeRedirectURL;
/// The URL to redirect to, assuming `nativeRedirectURL` is nil or fails to open. Cannot be nil if `nativeRedirectURL` is.
@property (nonatomic, nullable, copy) NSURL *redirectURL;
/// The expected `returnURL`, passed to STPURLCallbackHandler
@property (nonatomic, copy) NSURL *returnURL;
/// Completion block to execute when finished redirecting, with optional error parameter.
@property (nonatomic, copy) STPErrorBlock completion;

Expand Down
72 changes: 36 additions & 36 deletions Stripe/STPRedirectContext.m
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ @interface STPRedirectContext () <SFSafariViewControllerDelegate, STPURLCallback
@property (nonatomic, strong, nullable) SFSafariViewController *safariVC;
@property (nonatomic, assign, readwrite) STPRedirectContextState state;
/// If we're on iOS 11+ and in the SafariVC flow, this tracks the latest URL loaded/redirected to during the initial load
@property (nonatomic, strong, readwrite, nullable) NSURL *lastKnownSafariVCUrl;
@property (nonatomic, strong, readwrite, nullable) NSURL *lastKnownSafariVCURL;

@property (nonatomic, assign) BOOL subscribedToURLNotifications;
@property (nonatomic, assign) BOOL subscribedToForegroundNotifications;
Expand All @@ -45,9 +45,9 @@ - (nullable instancetype)initWithSource:(STPSource *)source
return nil;
}

self = [self initWithNativeRedirectUrl:[[self class] nativeRedirectURLForSource:source]
redirectUrl:source.redirect.url
returnUrl:source.redirect.returnURL
self = [self initWithNativeRedirectURL:[[self class] nativeRedirectURLForSource:source]
redirectURL:source.redirect.url
returnURL:source.redirect.returnURL
completion:^(NSError * _Nullable error) {
completion(source.stripeID, source.clientSecret, error);
}];
Expand All @@ -69,10 +69,10 @@ - (nullable instancetype)initWithPaymentIntent:(STPPaymentIntent *)paymentIntent
return nil;
}

NSString *redirectUrl = nextSourceAction[@"value"][@"url"];
return [self initWithNativeRedirectUrl:nil
redirectUrl:[NSURL URLWithString:redirectUrl]
returnUrl:paymentIntent.returnUrl
NSString *redirectURL = nextSourceAction[@"value"][@"url"];
return [self initWithNativeRedirectURL:nil
redirectURL:[NSURL URLWithString:redirectURL]
returnURL:paymentIntent.returnUrl
completion:^(NSError * _Nullable error) {
completion(paymentIntent.clientSecret, error);
}];
Expand All @@ -81,20 +81,20 @@ - (nullable instancetype)initWithPaymentIntent:(STPPaymentIntent *)paymentIntent
/**
Failable initializer for the general case of STPRedirectContext, some URLs and a completion block.
*/
- (nullable instancetype)initWithNativeRedirectUrl:(nullable NSURL *)nativeRedirectUrl
redirectUrl:(nullable NSURL *)redirectUrl
returnUrl:(NSURL *)returnUrl
- (nullable instancetype)initWithNativeRedirectURL:(nullable NSURL *)nativeRedirectURL
redirectURL:(nullable NSURL *)redirectURL
returnURL:(NSURL *)returnURL
completion:(STPErrorBlock)completion {
if ((nativeRedirectUrl == nil && redirectUrl == nil)
|| returnUrl == nil) {
if ((nativeRedirectURL == nil && redirectURL == nil)
|| returnURL == nil) {
return nil;
}

self = [super init];
if (self) {
_nativeRedirectUrl = nativeRedirectUrl;
_redirectUrl = redirectUrl;
_returnUrl = returnUrl;
_nativeRedirectURL = nativeRedirectURL;
_redirectURL = redirectURL;
_returnURL = returnURL;
_completion = completion;

_subscribedToURLNotifications = NO;
Expand All @@ -110,22 +110,22 @@ - (void)dealloc {
- (void)performAppRedirectIfPossibleWithCompletion:(STPBoolCompletionBlock)onCompletion {

if (self.state == STPRedirectContextStateNotStarted) {
NSURL *nativeUrl = self.nativeRedirectUrl;
if (!nativeUrl) {
NSURL *nativeURL = self.nativeRedirectURL;
if (!nativeURL) {
onCompletion(NO);
return;
}

// Optimistically start listening in case we get app switched away.
// If the app switch fails we'll undo this later
self.state = STPRedirectContextStateInProgress;
[self subscribeToUrlAndForegroundNotifications];
[self subscribeToURLAndForegroundNotifications];

UIApplication *application = [UIApplication sharedApplication];
if (@available(iOS 10, *)) {

WEAK(self);
[application openURL:nativeUrl options:@{} completionHandler:^(BOOL success) {
[application openURL:nativeURL options:@{} completionHandler:^(BOOL success) {
if (!success) {
STRONG(self);
self.state = STPRedirectContextStateNotStarted;
Expand All @@ -136,7 +136,7 @@ - (void)performAppRedirectIfPossibleWithCompletion:(STPBoolCompletionBlock)onCom
}
else {
_state = STPRedirectContextStateInProgress;
BOOL opened = [application openURL:nativeUrl];
BOOL opened = [application openURL:nativeURL];
if (!opened) {
self.state = STPRedirectContextStateNotStarted;
[self unsubscribeFromNotifications];
Expand Down Expand Up @@ -169,9 +169,9 @@ - (void)startSafariViewControllerRedirectFlowFromViewController:(UIViewControlle

if (self.state == STPRedirectContextStateNotStarted) {
_state = STPRedirectContextStateInProgress;
[self subscribeToUrlNotifications];
self.lastKnownSafariVCUrl = self.redirectUrl;
self.safariVC = [[SFSafariViewController alloc] initWithURL:self.lastKnownSafariVCUrl];
[self subscribeToURLNotifications];
self.lastKnownSafariVCURL = self.redirectURL;
self.safariVC = [[SFSafariViewController alloc] initWithURL:self.lastKnownSafariVCURL];
self.safariVC.delegate = self;
[presentingViewController presentViewController:self.safariVC
animated:YES
Expand All @@ -182,8 +182,8 @@ - (void)startSafariViewControllerRedirectFlowFromViewController:(UIViewControlle
- (void)startSafariAppRedirectFlow {
if (self.state == STPRedirectContextStateNotStarted) {
self.state = STPRedirectContextStateInProgress;
[self subscribeToUrlAndForegroundNotifications];
[[UIApplication sharedApplication] openURL:self.redirectUrl];
[self subscribeToURLAndForegroundNotifications];
[[UIApplication sharedApplication] openURL:self.redirectURL];
}
}

Expand Down Expand Up @@ -215,7 +215,7 @@ - (void)safariViewController:(__unused SFSafariViewController *)controller didCo
if (didLoadSuccessfully == NO) {
if (@available(iOS 11, *)) {
stpDispatchToMainThreadIfNecessary(^{
if ([self.lastKnownSafariVCUrl.host containsString:@"stripe.com"]) {
if ([self.lastKnownSafariVCURL.host containsString:@"stripe.com"]) {
[self handleRedirectCompletionWithError:[NSError stp_genericConnectionError]
shouldDismissViewController:YES];
}
Expand All @@ -233,7 +233,7 @@ - (void)safariViewController:(__unused SFSafariViewController *)controller initi
stpDispatchToMainThreadIfNecessary(^{
// This is only kept up to date during the "initial load", but we only need the value in
// `safariViewController:didCompleteInitialLoad:`, so that's fine.
self.lastKnownSafariVCUrl = URL;
self.lastKnownSafariVCURL = URL;
});
}

Expand Down Expand Up @@ -288,16 +288,16 @@ - (void)handleRedirectCompletionWithError:(nullable NSError *)error
self.completion(error);
}

- (void)subscribeToUrlNotifications {
- (void)subscribeToURLNotifications {
if (!self.subscribedToURLNotifications) {
self.subscribedToURLNotifications = YES;
[[STPURLCallbackHandler shared] registerListener:self
forURL:self.returnUrl];
forURL:self.returnURL];
}
}

- (void)subscribeToUrlAndForegroundNotifications {
[self subscribeToUrlNotifications];
- (void)subscribeToURLAndForegroundNotifications {
[self subscribeToURLNotifications];
if (!self.subscribedToForegroundNotifications) {
self.subscribedToForegroundNotifications = YES;
[[NSNotificationCenter defaultCenter] addObserver:self
Expand Down Expand Up @@ -329,18 +329,18 @@ - (void)dismissPresentedViewController {
}

+ (nullable NSURL *)nativeRedirectURLForSource:(STPSource *)source {
NSString *nativeUrlString = nil;
NSString *nativeURLString = nil;
switch (source.type) {
case STPSourceTypeAlipay:
nativeUrlString = source.details[@"native_url"];
nativeURLString = source.details[@"native_url"];
break;
default:
// All other sources currently have no native url support
break;
}

NSURL *nativeUrl = nativeUrlString ? [NSURL URLWithString:nativeUrlString] : nil;
return nativeUrl;
NSURL *nativeURL = nativeURLString ? [NSURL URLWithString:nativeURLString] : nil;
return nativeURL;
}

@end
Expand Down
2 changes: 1 addition & 1 deletion Tests/Tests/STPFixtures.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ extern NSString *const STPTestJSONSourceSOFORT;
/**
A Source object with type Alipay and a native redirect url
*/
+ (STPSource *)alipaySourceWithNativeUrl;
+ (STPSource *)alipaySourceWithNativeURL;

/**
A PaymentIntent object
Expand Down
2 changes: 1 addition & 1 deletion Tests/Tests/STPFixtures.m
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ + (STPSource *)alipaySource {
return [STPSource decodedObjectFromAPIResponse:[STPTestUtils jsonNamed:STPTestJSONSourceAlipay]];
}

+ (STPSource *)alipaySourceWithNativeUrl {
+ (STPSource *)alipaySourceWithNativeURL {
NSMutableDictionary *dictionary = [STPTestUtils jsonNamed:STPTestJSONSourceAlipay].mutableCopy;
NSMutableDictionary *detailsDictionary = ((NSDictionary *)dictionary[@"alipay"]).mutableCopy;
detailsDictionary[@"native_url"] = @"alipay://test";
Expand Down
22 changes: 21 additions & 1 deletion Tests/Tests/STPPaymentIntentParamsTest.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ - (void)testInit {
XCTAssertNil(params.sourceId);
XCTAssertNil(params.receiptEmail);
XCTAssertNil(params.saveSourceToCustomer);
XCTAssertNil(params.returnUrl);
XCTAssertNil(params.returnURL);
}
}

Expand All @@ -39,6 +39,26 @@ - (void)testDescription {
XCTAssertNotNil(params.description);
}

#pragma mark Deprecated Property

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated"

- (void)testReturnURLRenaming {
STPPaymentIntentParams *params = [[STPPaymentIntentParams alloc] init];

XCTAssertNil(params.returnURL);
XCTAssertNil(params.returnUrl);

params.returnURL = @"set via new name";
XCTAssertEqualObjects(params.returnUrl, @"set via new name");

params.returnUrl = @"set via old name";
XCTAssertEqualObjects(params.returnURL, @"set via old name");
}

#pragma clang diagnostic pop

#pragma mark STPFormEncodable Tests

- (void)testRootObjectName {
Expand Down
Loading

0 comments on commit bd57e8d

Please sign in to comment.