-
Notifications
You must be signed in to change notification settings - Fork 52
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
[WePay] implementation of all functions and their testcases #165
base: dev
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #165 +/- ##
==========================================
- Coverage 77.33% 68.55% -8.78%
==========================================
Files 14 15 +1
Lines 375 423 +48
==========================================
Hits 290 290
- Misses 85 133 +48
Continue to review full report at Codecov.
|
lib/gringotts/gateways/we_pay.ex
Outdated
|
||
{ | ||
:ok, | ||
Response.success(id: id, message: message, token: token, raw: parsed, status_code: code) |
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.
Don't use the functions from the Response
module, they are deprecated (slated for removal in 1.2.0).
lib/gringotts/gateways/we_pay.ex
Outdated
{:ok, card_token} <- extract_card_token(card_token_response) do | ||
body = | ||
Poison.encode!(%{ | ||
account_id: opts[:account_id], |
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.
The account_id
cannot be fetched like this because it should be part of the required config.
See https://developer.wepay.com/docs/faq/general#how-do-i-find-my-account-id
opts[:config][:account_id]
lib/gringotts/gateways/we_pay.ex
Outdated
body = | ||
Poison.encode!(%{ | ||
account_id: opts[:account_id], | ||
short_description: opts[:short_description], |
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.
- This map is so verbose! Most of the keys are not transformed. Please consider the following style to build this map:
opts |> Keyword.take(@required_or_allowed_keys) |> Enum.into(%{}) |> Map.merge(%{ currency: currency, value: value, payment_method: %{...}, ... })
lib/gringotts/gateways/we_pay.ex
Outdated
|
||
body = | ||
Poison.encode!(%{ | ||
account_id: opts[:account_id], |
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.
The maps are very similar in both clauses, please move the common parts into a private function.
iex> {:ok, void_result} = Gringotts.capture(Gringotts.Gateways.WePay, purchase_result.id, opts) | ||
``` | ||
""" | ||
@spec void(String.t(), keyword) :: {:ok | :error, Response} |
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.
The auto_release
setting comes into play here, when we cancel a checkout.
If it was true, the funds are auto-released back to the payer.
otherwise, an explicit call to checkout/release/
is needed.
Since we don't have any API call for release
, we must hard-code auto_release
to true. Hmm... too bad. 😞
defmodule Gringotts.Integration.Gateways.WePayTest do | ||
# Integration tests for the WePay | ||
|
||
use ExUnit.Case, async: false |
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.
async true pls
refund_reason: "the product was defective", | ||
cancel_reason: "the product was defective, i don't want", | ||
config: [ | ||
access_token: "STAGE_a24e062d0fc2d399412ea0ff1c1e748b04598b341769b3797d3bd207ff8cf6b2" |
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.
Expect some more fields here.
test "[Store] with CreditCard" do | ||
use_cassette "WePay/store_with_valid_card" do | ||
assert {:ok, response} = Gateway.store(@good_card, @opts) | ||
assert response.success == true |
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.
Please assert that the card ID is received in the :token
field
] | ||
|
||
describe "store" do | ||
test "[Store] with CreditCard" do |
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.
You don't need [Store]
in the test name.
end | ||
end | ||
|
||
describe "Void" do |
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.
Can you add a test to check auth
-> capture
-> void
?
Implementation of :
Authorize/3
Capture/3
Purchase/3
void/2
refund/3
store/2
unstore/2
and its private functions