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

Changes to the Payment Intent APIs for the next API version #670

Merged
merged 1 commit into from Feb 12, 2019

Conversation

remi-stripe
Copy link
Contributor

We are going to ship some changes to the Payment Intent APIs in the next API version. Though are fairly self-contained and pave the way for future changes. Changes are:

  • status: remove require_source and require_source_action and add requires_payment_method and requires_action
  • next_source_action renamed to next_action
  • allowed_source_types renamed to payment_method_types
  • authorize_with_url renamed to redirect_to_url both in the type of next action and in the hash that describes that next action.

cc @stripe/api-libraries @danwang-stripe

cc @mickjermsurawong-stripe To confirm naming for the new classes make sense (now that they are inline)

@mickjermsurawong-stripe
Copy link
Contributor

@remi-stripe

  • the payment NextAction, NextActionRedirectToUrl nested in PaymentIntent looks great with me.
    I see that in the the latest OpenAPI spec. (But client metadata is wrong, I will go and update it to reflect similarly this PR)
  • Haven't seen the change on allowed_source_types in pay-server yet.
  • We currently don't have enum on the status, so not sure if it's worth specifying that in comments. But that means we have to maintain the comment too which is inconsistent. I'm okay with what it is now.

@remi-stripe
Copy link
Contributor Author

@mickjermsurawong-stripe thanks a lot for the review, this definitely helps vet the approach!

@remi-stripe remi-stripe changed the title [WIP] Changes to the Payment Intent APIs for the next API version Changes to the Payment Intent APIs for the next API version Feb 12, 2019
@remi-stripe
Copy link
Contributor Author

@mickjermsurawong-stripe API version is going out tomorrow so marking this as ready to merge now that stripe-mock is updated and assigning to you for a full review.

@EqualsAndHashCode(callSuper = false)
public static class NextAction extends StripeObject {
NextActionRedirectToUrl redirectToUrl;
String type;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need the additional field? At least it shows up in my autogen PR https://github.com/stripe/stripe-java/pull/676/files.. Also confirm that it's present in public OpenAPI

    Map<String, Object> useStripeSdk;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mickjermsurawong-stripe This is undocumented/secret right now though so not something we'd want to ship for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed, we want the parameter/enum value but this property should not be in the libraries since it's opaque and specific to stripe.js

@mickjermsurawong-stripe
Copy link
Contributor

ptal @remi-stripe, maybe we are missing a field on NextAction

@mickjermsurawong-stripe
Copy link
Contributor

ah good to know.. Will make a PR to undocument that in pay-server.
LGTM

@remi-stripe remi-stripe merged commit b4cf361 into master Feb 12, 2019
@remi-stripe remi-stripe deleted the remi-payment-intents-changes branch February 12, 2019 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants