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

Litle: Add Support for Echeck #2776

Merged
merged 1 commit into from Mar 8, 2018
Merged

Litle: Add Support for Echeck #2776

merged 1 commit into from Mar 8, 2018

Conversation

nfarve
Copy link

@nfarve nfarve commented Mar 6, 2018

Adds support for Echeck and tests for certification.

Loaded suite test/remote/gateways/remote_litle_test
...................................

35 tests, 136 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Loaded suite test/unit/gateways/litle_test
Started
...................................

35 tests, 151 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications

elsif check?(payment_method)
doc.echeck do
doc.accType(payment_method.account_type)
doc.accNum(payment_method.account_number)
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to add this field to the scrub method

Copy link
Author

Choose a reason for hiding this comment

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

good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

And the scrub tests too, preferably.

Copy link
Author

Choose a reason for hiding this comment

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

👍 check and check

order_id: '38',
order_source: 'telephone'
})
assert response = @gateway.authorize(3002, @authorize_check, options)
assert_success response
assert_equal 'Approved', response.message
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I know Github can do weird things to how these diffs are displayed, but wanted to make sure that these lines 113 and 114 aren't missing from the above test, and same for 202/203

Copy link
Author

Choose a reason for hiding this comment

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

Ooh look like it is. I think it was a merge conflict auto-merging thing. Will fix that.

doc.litleTxnId(transaction_id)
doc.amount(money) if money
doc.send(refund_type(payment), transaction_attributes(options)) do
if payment.is_a?(String) || payment.is_a?(Numeric)
Copy link

Choose a reason for hiding this comment

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

Why the need to check for a numeric type now?

Copy link
Author

Choose a reason for hiding this comment

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

@dtykocki in the tests it was giving tokens/auth codes as numbers and not strings

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't authorization_from stringify them?

Copy link

Choose a reason for hiding this comment

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

Yeah authorization_from should turn them into strings so I'm a bit 😕 here.

Copy link
Author

Choose a reason for hiding this comment

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

@dtykocki and @curiousepic So I can change remote tests so they pass only strings and not numbers. Cases like this:

assert capture_response = @gateway.capture(10010, 123456789012345360)

Copy link

Choose a reason for hiding this comment

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

@nfarve - Try that and maybe we'll be able to remove that number check.

add_echeck_purchase_params(doc, money, payment, options)
else
add_auth_purchase_params(doc, money, payment, options)
end
Copy link

Choose a reason for hiding this comment

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

Maybe it's the naming of the methods being used in here in refund (add_echeck_purchase_params and add_auth_purchase_params) but do refunds require the exact same params as echeck and auth purchases?

Copy link
Author

Choose a reason for hiding this comment

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

@dtykocki Litle allows for "credits" of non-vantiv (litle) transactions. In those cases you give it the same info as a purchase and they "credit" the amount instead.

@nfarve
Copy link
Author

nfarve commented Mar 7, 2018

@dtykocki and @curiousepic change all auth tokens to strings in remote test and got rid of test for numeric.

def test_successful_purchase_with_echeck
response = stub_comms do
@gateway.purchase(2004, @check)
end.respond_with(successful_purchase_with_echeck_response)
Copy link

Choose a reason for hiding this comment

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

Nitpick: indentation is slightly off here.

Copy link

@dtykocki dtykocki left a comment

Choose a reason for hiding this comment

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

🎉 👏 Nice job here @nfarve :shipit:

@nfarve nfarve force-pushed the enrg_7183_2 branch 2 times, most recently from e445660 to 701ba48 Compare March 8, 2018 13:59
Adds support for Echeck and tests for certification.

Loaded suite test/remote/gateways/remote_litle_test
...................................

35 tests, 136 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Loaded suite test/unit/gateways/litle_test
Started
...................................

35 tests, 151 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@nfarve nfarve merged commit 941adb6 into master Mar 8, 2018
@nfarve nfarve deleted the enrg_7183_2 branch March 8, 2018 16:26
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