-
Notifications
You must be signed in to change notification settings - Fork 982
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
Update PaymentIntents support #1038
Conversation
Added TODOs to pull the replacement property in the right places.
It's confusing to print `type = authorize_with_url` if decoding the source action details as a `STPPaymentIntentSourceActionAuthorizeWithURL` object fails. Instead, log the value that the enum currently has.
…nURL exists as desired
…edirectContext` Also, since we're using our normal object parsing code, could simplify the whitebox test that tried unexpected types during deserialization - covered elsewhere.
This PR relies on #1037, so it is not using master as the base branch and the CI tests haven't run against it. It looked good locally, and worked when I used the example app to create & confirm PaymentIntents. I think the code's ready to review, but it'll probably require another 👍 after #1037 is merged and the base branch is changed. |
|
||
These are created & owned by the containing `STPPaymentIntent`. | ||
*/ | ||
@interface STPPaymentIntentSourceActionAuthorizeWithURL: NSObject<STPAPIResponseDecodable> |
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.
What do you think of renaming this something like STPPaymentIntentSourceActionURLAuthorizationData
? AuthorizeWithURL
is a bit confusing to me
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.
I figured consistency with the API (where this shows up as authorize_with_url
, plus the other stripe SDKs (which use essentially the same thing), was a good reason to go with this monstrosity of a name. It's my hope that users of the library never actually really interact with this class - I suspect they'll just access it's fields through the STPPaymentIntent
object
- https://github.com/stripe/stripe-go/blob/4f11ac96bdae9a119b13ba82a5cc0e95d96dd5cc/paymentintent.go#L115-L118
- https://github.com/stripe/stripe-dotnet/blob/a9d958bf4a1ddafc8446461a47e6d04eef9301a1/src/Stripe.net/Entities/PaymentIntents/PaymentIntentSourceActionAuthorizeWithUrl.cs#L5
- https://github.com/stripe/stripe-java/blob/a8ff01ec123f8d0770a6248cc20a61fdae6c3b1a/src/main/java/com/stripe/model/PaymentIntentSourceActionValueAuthorizeWithUrl.java#L10
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.
Makes sense
6916e76
to
09614b1
Compare
Summary
nextSourceAction
, with the only possible type beingAuthorizeWithURL
returnURL
from thenextSourceAction
instead of the top-levelreturnURL
inSTPRedirectContext
Motivation
IOS-1013
Testing
Updated automated tests