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

Reorganize Payment Gateway Structure and Fix Payment Method Creation #24

Merged
merged 6 commits into from Apr 19, 2017

Conversation

BenMorganIO
Copy link
Contributor

@BenMorganIO BenMorganIO commented Apr 18, 2017

This PR has a lot of changes in it.

  1. I've gone and changed up how payment gateways are developed. We want to have them split off by their type and not their domain logic. This way we can isolate all of that per payment gateway.

  2. I've added some work for creating credit cards. There's still lots more to do, but hey, at least we can create credit cards now. They are also tied to a specific customer.

  3. When configuring fallback gateways, we don't need to explicitly ask for it anymore. We can now just ask the person for a list of keys and values.

  4. Payment methods weren't fully working. They're now fully working with the embeds.

assert Proxy.changeset(%Proxy{}, %CreditCard{}) == expectation
changeset = CreditCard.changeset(%CreditCard{}, params)
expectation = cast(%Proxy{}, %{data: params}, [:data])
assert Proxy.changeset(%Proxy{}, %CreditCard{}) == Map.merge(expectation, %{

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 82).

|> Repo.insert!

gateway = %PaymentGateway{}
|> PaymentGateway.changeset(%{type: "stripe", external_id: "cus_123", user_id: user.id})

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 96).

|> Repo.insert!

gateway = %PaymentGateway{}
|> PaymentGateway.changeset(%{type: "stripe", external_id: "cus_123", user_id: user.id})

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 96).

})
|> Ryal.repo.insert!

result = Stripe.create(:credit_card, credit_card.proxy.data, bypass_endpoint(bypass))

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 91).


defp random_id do
:rand.uniform * 10_000_000_000
|> round

Choose a reason for hiding this comment

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

Pipe chain should start with a raw value.

We now pattern match based on domain, not by gateway provider. This way, when developing payment gateways, we can keep private functions and configuration specific to a single module.
@BenMorganIO BenMorganIO changed the title Credit cards and Stripe Reorganize Payment Gateway Structure and Fix Payment Method Creation Apr 18, 2017
end

%PaymentGateway{}
|> PaymentGateway.changeset(%{type: "stripe", external_id: "cus_123", user_id: user.id})

Choose a reason for hiding this comment

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

Line is too long (max is 80, was 94).

@stripe_api_key Map.get(Ryal.payment_gateways(), :stripe)
@stripe_base "https://#{@stripe_api_key}:@api.stripe.com"

@spec create(atom, Ecto.Schema.t, String.t) :: {:ok, String.t}
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 need to remove the need for an Ecto.Schema.t and use either multiple args or a map instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

At this level I agree. We shouldn't tie gateway usage to Ecto dependencies....Unless we are comitted to building on the Phoenix stack instead of a more general approach.

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'm good with sticking to Ecto. However, we don't really need Phoenix so much as we need Plug. To me, Ryal, ATM, will always require storage. It's up to the user if they want to mount the API. Ryal, however, would expect to be able to store data.

Unless, of course, we move the logic for Ryal into another dependency. But that's a longer-term goal. We need Ryal working first.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok let's ship it :shipit: and keep an eye on these dependencies as the code base grows.

@BenMorganIO BenMorganIO requested a review from dhonig April 19, 2017 20:50
@dhonig dhonig merged commit 5dde38a into master Apr 19, 2017
@BenMorganIO BenMorganIO deleted the credit-cards-and-stripe branch April 19, 2017 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants