Nuvei: Update Payouts request and some fixes #5327
Conversation
javierpedrozaing
force-pushed
the
SER-1494_Nuvei_payouts
branch
from
c9d14cd
to
525b1d8
Compare
There was a problem hiding this comment.
Looks pretty good @javierpedrozaing , left some minor comments
| payment_data = payment.is_a?(CreditCard) || payment.is_a?(NetworkTokenizationCreditCard) ? credit_card_hash(payment) : payment | ||
|
|
There was a problem hiding this comment.
💬 Comment/Style:
Seems that we hand enough payment method types to justify a case instead of if/elsif/end structure
There was a problem hiding this comment.
thanks, I think that all this method requires a code refactoring.
| @@ -475,7 +477,11 @@ def success_from(response) | |||
| end | |||
|
|
|||
| def authorization_from(action, response, post) | |||
| response.dig(:transactionId) | |||
| if post[:userTokenId].present? && post[:transactionType] == 'Auth' && post[:amount].to_i == 0 | |||
There was a problem hiding this comment.
💬 Comment:
maybe having an extra method that clarifies the reason for those '&', just by reading I'm understanding something like a zero auth request, perhaps?
There was a problem hiding this comment.
thanks, I will update it.
| if post[:userTokenId].present? && post[:transactionType] == 'Auth' && post[:amount].to_i == 0 | ||
| response.dig(:paymentOption, :userPaymentOptionId) | ||
| else | ||
| response.dig(:transactionId) |
There was a problem hiding this comment.
💬 COmment
Unless you are expecting to add more keys to the params a dig is an overkill over a simple []
javierpedrozaing
force-pushed
the
SER-1494_Nuvei_payouts
branch
2 times, most recently
from
3195e3c
to
d639494
Compare
There was a problem hiding this comment.
Generally looks pretty good, but I have a broad question. Since this PR is for Payouts, is there any transaction specific value that distinguishes the payout from a standard credit?
I'm wondering if there should be a specific test or if there is something that a user needs to specify which would make the payout different from an "unreferenced refund".
There was a problem hiding this comment.
Regarding my last comment, I think I see now what I might have missed previously.
Payouts should be treated as an alternate operation for credit, but not the default operation. So, we should only perform the /payout if there is some explicit indicator, whether it's a GSF or something else, that tells us to send the request to the /payout endpoint instead of the unreferenced refund.
Hi #naashton ahhh ok thanks, it's clearer to me now, I will working on it. |
javierpedrozaing
force-pushed
the
SER-1494_Nuvei_payouts
branch
5 times, most recently
from
0af2ee3
to
f97c039
Compare
Description ------------------------- [SER1494](https://spreedly.atlassian.net/browse/SER-1494) This commit include Google and Apple pay for payout request additionnal this commit add the next fixes - Remove the unnecessary savePM flag. By default, Nuvei stores the payment method (PM), so we don't need to override this default behavior. - Fix ACH transaction by updating the SECCode field - Use the user_token_id option only for the required test - Refactor add_payment_method Unit test ------------------------- Finished in 1.789372 seconds. 25 tests, 122 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed 13.97 tests/s, 68.18 assertions/s Remote test ------------------------- Finished in 102.124827 seconds. 36 tests, 117 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed 0.35 tests/s, 1.15 assertions/s Rubocop ------------------------- 804 files inspected, no offenses detected
javierpedrozaing
force-pushed
the
SER-1494_Nuvei_payouts
branch
from
9467b86
to
a95bcf3
Compare
Description
SER1494
This commit include Google and Apple pay for payout request additionnal this commit add the next fixes
Remove the unnecessary savePM flag. By default, Nuvei stores the payment method (PM), so we don't need to override this default behavior.
Fix ACH transaction by updating the SECCode field
Update authorization_from method
Use the user_token_id option only for the required test
Unit test
Finished in 1.789372 seconds.
25 tests, 122 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
13.97 tests/s, 68.18 assertions/s
Remote test
Finished in 102.124827 seconds.
36 tests, 117 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
0.35 tests/s, 1.15 assertions/s
Rubocop
804 files inspected, no offenses detected