-
Notifications
You must be signed in to change notification settings - Fork 53
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
[CAMS] Adds money protocol #89
Conversation
gopalshimpi
commented
Jan 19, 2018
•
edited by pkrawat1
Loading
edited by pkrawat1
- Modified methods according to money protocol
- Fixes Fix "space_after_commas" issue in lib/gringotts/gateways/cams.ex #21, Fixes Fix "parentheses_in_condition" issue in lib/gringotts/gateways/cams.ex #26 and Fixes Fix "unused_keyword_operation" issue in lib/gringotts/gateways/cams.ex #28
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.
There are some issues with the examples in docs, the opts
list has a :currency
key inside. We no longer accept that.
Also, all bindings in the examples should have an iex>
in front:
iex> amount = 100
lib/gringotts/gateways/cams.ex
Outdated
@@ -118,7 +117,7 @@ defmodule Gringotts.Gateways.Cams do | |||
|
|||
iex> Gringotts.purchase(Gringotts.Gateways.Cams, money, payment, options) | |||
""" | |||
@spec purchase(number, CreditCard.t, Keyword) :: Response | |||
@spec purchase(Money.t, CreditCard.t, Keyword) :: 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.
Your specs
are wrong, this should be:
@spec purchase(Money.t(), CreditCard.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.
@oyeb CreditCard.t() why brackets here?
lib/gringotts/gateways/cams.ex
Outdated
@headers [{"Content-Type", "application/x-www-form-urlencoded"}] | ||
use Gringotts.Gateways.Base | ||
use Gringotts.Adapter, | ||
required_config: [:username, :password, :default_currency] | ||
alias Gringotts.{CreditCard, Response} | ||
alias Gringotts.{CreditCard, Response, Money} |
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.
Follow the style guide on the order of use, alias, import and @module_attribute
lib/gringotts/gateways/cams.ex
Outdated
@@ -87,14 +87,13 @@ defmodule Gringotts.Gateways.Cams do | |||
- Refund | |||
""" | |||
@live_url "https://secure.centralams.com/gw/api/transact.php" | |||
@default_currency "USD" | |||
@headers [{"Content-Type", "application/x-www-form-urlencoded"}] |
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 mention that the unit of amount is not cents, in the "Scope of this module" section on line 52. See how it is done in MONEI docs.
lib/gringotts/gateways/cams.ex
Outdated
optional arguments for transactions with the CAMS gateway. The following keys | ||
are supported: | ||
|
||
| Key | Remark | Status | |
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 fill this table. AFAICS billing_address
and order_id
are already implemented, but there is no explanation what their schema or types are. Either place a link to the official docs or document the schema somewhere.
[post: fn(_url, _body, _headers) -> MockResponse.failed_purchase_with_bad_money end] do | ||
{:ok, %Response{message: result}} = Gateway.purchase(@bad_money, @payment, @options) | ||
assert String.contains?(result, "Invalid amount") | ||
assert String.contains?(result, "Invalid Credit Card Number") | ||
end | ||
end | ||
|
||
test "with invalid currency" 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.
@money
does not have currency INR
, so this mock test is misleading.
This PR can be merged after merging #92. |
c2283af
to
68d3d65
Compare
dc1393c
to
c2283af
Compare
16b222a
to
dc3597a
Compare
1. Modified methods according to money protocal. 2. Modified test data as per money protocal.
* Reworded all docs, fixed numerous typos * Fixed (#) critical bugs in capture and refund (money was not being used) * Cleaned up bad whitespace
* Added patterns in function clauses on type * Reduced arity and name of add_invoice -> add_amount * Corrected HTTPoison.post in commit * Improved add_address
* Corrected mock tests, no direct calls to module. * Removed test on bad_amount
* Added money protocol for CAMS gateway. * Modified methods according to money protocol. * Modified test data as per money protocol. * Corrected protocol usage, docs and some bugs * Updated docs