Trust Commerce: Support void after purchase #3265
Conversation
|
@jknipp wasn't sure if this was ready for review or not (didn't see Connect tagged), but I've started a review and code-wise things seem to be good to go to me. I read the related tickets, but didn't go deep to confirm what the exact logic should be (in terms of the Gateway's documented requirements). Please let me know if this is ready for final review/approval or if there's something in particular you'd like a closer look at. Thanks! |
|
@bayprogrammer Waiting on feedback from the gateway for remote testing this functionality. |
jknipp
force-pushed
the
ECS-356-trust-commerce-voids
branch
from
acd0c9c to
db6f8fe
Compare
|
@bayprogrammer This one should be ready for review now. |
| # NOTE: AMEX preauth's cannot be reversed. If you want to clear it more | ||
| # quickly than the automatic expiration (7-10 days), you will have to | ||
| # capture it and then immediately issue a credit for the same amount | ||
| # which should clear the customers credit card with 48 hours according to | ||
| # TC. | ||
| def void(authorization, options = {}) | ||
| transaction_id, original_action = split_authorization(authorization) | ||
| action = (VOIDABLE_ACTIONS - ['preauth']).include?(original_action) ? 'void' : 'reversal' |
There was a problem hiding this comment.
Just want to make sure I understand what's going on here. We must use 'void' for voidable actions except 'preauth', which should still use 'reversal' instead? I'm guessing this was the nuance we're trying to update for. And thus, even though there's a difference when voiding, we needed to append the original action for all action listed in VOIDABLE_ACTIONS so we can do this sort of check? Thus the need to use a subset of VOIDABLE_ACTIONS to determine which action to actually use for this void request?
Since ['preauth'] is a constant value, and is the exception in the case of void vs. reversal, maybe it would make sense to move it into a named constant like VOIDABLE_VIA_REVERSAL_ACTIONS or something similar? (Would also avoid allocating a new array on every call to void).
There was a problem hiding this comment.
I don't care for the single element array with preauth, but opted for explicitness because there is only a single action that uses reversal and it is only used in a single place.
Moving the value to a constant would only prevent a new array for void if we were using a singleton instance of the gateway. If we instantiate a new gateway adapter for each transaction, a by-product of moving to a constant is that we would create this new constant array for every adapter, not just for void calls.
There was a problem hiding this comment.
If we instantiate a new gateway adapter for each transaction, a by-product of moving to a constant is that we would create this new constant array for every adapter...
I won't insist on the constant, but I will point out that the constant reference is not re-initialized on class instancing:
irb(main):001:0> class Foo; BAR = ['baz']; def bar; BAR.object_id; end; end
=> :bar
irb(main):002:0> f1 = Foo.new
=> #<Foo:0x00007fd9d424c448>
irb(main):003:0> f2 = Foo.new
=> #<Foo:0x00007fd9d4254e18>
irb(main):004:0> f1.bar == f2.bar
=> true
There was a problem hiding this comment.
I thought about that when I went for a walk. You are right on the constant reference initialization, I was mixing languages.
jknipp
force-pushed
the
ECS-356-trust-commerce-voids
branch
from
db6f8fe to
6556803
Compare
Trust Commerce allows voids on purchases, refunds and captures, and preauths. Preauth uses the 'reversal' function, while purchase, refund, and capture use the Trust Commerce 'void' function'. Expand void support for additional transaction types by tracking the original action associated with the authorization transaction id. The action take at Trust Commerce (reversal or void) is based on the original action. ECS-356 Unit: 12 tests, 41 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 17 tests, 64 assertions, 3 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 82.3529% passed closes activemerchant#3265
jknipp
force-pushed
the
ECS-356-trust-commerce-voids
branch
from
6556803 to
ac192a1
Compare
Trust Commerce allows voids on purchases, refunds and captures, and preauths. Preauth uses the 'reversal' function, while purchase, refund, and capture use the Trust Commerce 'void' function'. Expand void support for additional transaction types by tracking the original action associated with the authorization transaction id. The action take at Trust Commerce (reversal or void) is based on the original action. ECS-356 Unit: 12 tests, 41 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 17 tests, 64 assertions, 3 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 82.3529% passed closes activemerchant#3265
I was not able to test this remotely. I'm not sure if this functionality is enabled via settings or if it cannot be tested in the sandbox.
Trust Commerce allows voids on purchases, refunds and captures, and
preauths. Preauth uses the 'reversal' function, while purchase, refund,
and capture use the Trust Commerce 'void' function'.
Expand void support for additional transaction types by tracking the
original action associated with the authorization transaction
id. The action take at Trust Commerce (reversal or void) is based on the
original action.
ECS-356
Unit:
12 tests, 41 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
16 tests, 59 assertions, 3 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
81.25% passed
Three unrelated, existing remote test failures.