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

Support new Checkout flow #466

Merged
merged 7 commits into from
Mar 5, 2019
Merged

Support new Checkout flow #466

merged 7 commits into from
Mar 5, 2019

Conversation

dnsbty
Copy link
Contributor

@dnsbty dnsbty commented Mar 4, 2019

Stripe is launching a new flow for Checkout: https://stripe.com/docs/payments/checkout

This PR adds the backend Session functionality that is required for this new flow. Because this is my first major PR to this library, I have tried to maintain the style of existing modules, but I would love feedback on anything I could be doing better.

@coveralls
Copy link

coveralls commented Mar 4, 2019

Coverage Status

Coverage decreased (-0.05%) to 84.848% when pulling a4eb82d on dnsbty:master into c4e0ec8 on code-corps:master.

Copy link
Collaborator

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

This looks pretty close! Just a few comments and we can merge. Thank you!

optional(:state) => String.t()
}

@type shipping_info :: %{
Copy link
Collaborator

Choose a reason for hiding this comment

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

in stripe/types.ex, we have a shipping type


@type capture_method :: :automatic | :manual

@type address :: %{
Copy link
Collaborator

Choose a reason for hiding this comment

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

in stripe/types.ex, we have a shipping type

optional(:application_fee_amount) => integer(),
optional(:capture_method) => capture_method,
optional(:description) => String.t(),
optional(:metadata) => map(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

r/String.t()/Stripe.Types.metadata()

use Stripe.Entity
import Stripe.Request

@type line_item :: %{
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a Stripe.LineItem object. Has everything except images. Don't see it here though 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that might be a new feature for the new Stripe Checkout: https://stripe.com/docs/api/checkout/sessions/create#create_checkout_session-line_items

My personal thinking is that the Stripe.LineItem type has a lot of fields on it that won't be accepted by this endpoint, so it would be better to have a second local line_item type that is specific to this use case. If you prefer though, I'm happy to add an images field to Stripe.LineItem and use that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

na we can leave it as is 👍

result = Converter.convert_result(fixture)

assert result == expected_result
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

optional(:subscription_data) => subscription_data
}

@type t :: %{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need %__MODULE__{ here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch!

Copy link
Collaborator

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

Woooo hoooo! Lmk how it works on your end!

@snewcomer snewcomer merged commit 2d037d3 into beam-community:master Mar 5, 2019
@dnsbty
Copy link
Contributor Author

dnsbty commented Mar 6, 2019

Works great! The new checkout flow also uses a new PaymentIntent object, so we'll probably send you another PR to get that working sometime in the next couple days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants