Support installments #70

Merged
merged 5 commits into from Mar 2, 2014

Conversation

Projects
None yet
3 participants
Contributor

lucashungaro commented Feb 24, 2014

Credit card payments in installments are pretty common in South America, so this is intended to support that use case.

@alloy alloy and 1 other commented on an outdated diff Feb 25, 2014

spec/functional/api_spec.rb
@@ -45,7 +45,7 @@ def perform_payment_request
end
it "performs a one-click payment request" do
- detail = Adyen::API.list_recurring_details(@user_id).references.last
+ detail = Adyen::API.list_recurring_details(@user_id).references.last || 'recurring-reference'
@alloy

alloy Feb 25, 2014

Collaborator

A request is made from the before filter and its response should be used, so why is this fallback to 'recurring-reference' being added?

@lucashungaro

lucashungaro Feb 25, 2014

Contributor

I tried running this spec and it failed because detail was nil (even before my changes), so I did that. Any tips? I'm surely missing something important here.

@alloy

alloy Feb 25, 2014

Collaborator

Then maybe the functional test is broken. (I haven’t had to use Adyen and thus this lib for a long time.) But that should then be fixed or at least left untouched if your code is not related to this in any way.

@lucashungaro

lucashungaro Feb 25, 2014

Contributor

You're right, I'll revert this.

@alloy alloy and 1 other commented on an outdated diff Feb 25, 2014

spec/api/payment_service_spec.rb
@@ -10,7 +10,7 @@
text('./payment:reference').should == 'order-id'
end
- it "includes the given amount of `currency'" do
+ it "includes the given amount of 'currency'" do
@alloy

alloy Feb 25, 2014

Collaborator

Please don’t change code-style in patches, instead you should follow them yourself (below).

@lucashungaro

lucashungaro Feb 25, 2014

Contributor

Sorry, I thought it was a typo.

@alloy alloy commented on an outdated diff Feb 25, 2014

spec/api/payment_service_spec.rb
it "does not include the fraud offset if none is given" do
@payment.params.delete(:fraud_offset)
xpath('./payment:fraudOffset').should be_empty
end
+ it "includes the given amount of 'installments'" do
@alloy

alloy Feb 25, 2014

Collaborator

This is where you should follow the existing code-style, as mentioned above.

Collaborator

alloy commented Feb 25, 2014

@wvanbergen Apart from those code-style comments and the question why the existing functional test is being changed, it looks good to me.

Owner

wvanbergen commented Mar 2, 2014

Merging. Thanks for your contribution!

wvanbergen merged commit 59f5394 into wvanbergen:master Mar 2, 2014

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment