Priority: Refactor gateway integration, add additional fields to request #4383
Conversation
| @@ -207,11 +270,12 @@ def commit(action, params: '', iid: '', card_number: nil, jwt: '') | |||
| parse(ssl_post(url(action, params), post_data(params), request_headers)) | |||
| end | |||
| rescue ResponseError => e | |||
| parse(e.response.body) | |||
| # currently Priority returns a 404 with a message but an empty body on certain calls | |||
There was a problem hiding this comment.
:confused_parrot: where does the message come from if the body is empty????
There was a problem hiding this comment.
Good question.
Apparently the response.message attribute in ruby's net/http library is simply the reason phrase pulled out of the status line of the HTTP response.
So in this case the response has no body but the status line is HTTP/1.1 404 Not Found so response.message ends up being 'Not Found'.
Perhaps my comment would be more accurate if it read currently Priority returns a 404 with no body on certain calls. In those cases we will substitute the response status from response.message.
I will change it.
| response = @gateway.authorize(@amount_purchase, @credit_card, @option_spr) | ||
| assert_success response | ||
| assert_equal 'Approved', response.params['status'] | ||
| def test_successful_capture |
There was a problem hiding this comment.
This is a minor nitpick, but I think it's conventional to name tests that call multiple methods after both methods, e.g. test_successful_authorize_and_capture
There was a problem hiding this comment.
Thank you for this minor but helpful nitpick 😄 . I will change it.
CE-2491 Rubocop: 739 files inspected, no offenses dtected Unit: 5159 tests, 75564 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: Loaded suite test/remote/gateways/remote_priority_test 27 tests, 81 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passede
dsmcclain
force-pushed
the
CE-2491_priority_gsfs
branch
from
ef85c3c to
df971a5
Compare
CE-2491
This PR makes significant adjustments to the Priority gateway integration.
I will describe some of the changes below, but I also want to note two areas that warrant additional investigation:
verifyaction and its dependencycreate_jwt. These methods are using the gateway's BIN lookup service, which is not a standard approach to verifying a card.get_payment_statusandclose_batchactions. It's not clear why these actions were implemented, as they are currently being used exclusively to manipulate transactions within the test cases. A merchant going through Spreedly would have no way to take advantage of these actions.I will open some new tickets to investigate these two areas.
The changes made in this PR include:
pos_dataobject has been extracted to its own method and is now added line-by-line to the requestshipping_datafields have their own method now.verifymethod, as well as the auxiliaryget_payment_status,close_batch, andcreate_jwtmethods, have been simplified to conform to Active Merchant standards.authorization_frommethod has been updated to conform to Active Merchant standards. Instead of returning a ruby hash it now returns a string of format"<request_id>|<payment_token>"In addition to the above changes, the following fields were removed from requests to the gateway, because they are either not mentioned in the gateway documentation, not necessary for the request, or else described as gateway response fields (meaning that they do not belong in the request):
authCodecardAccount.codecardAccount.taxRatecardAccount.entryModecardAccount.cardIdcardAccount.hasContractcardAccount.isCorpcardAccount.isDebitcardAccount.tokencardPresentcardPresentTypeisTicketmxAdvantageEnabledsourceZippurchases.dateCreatedpurchases.iIdpurchases.transactionIIdpurchases.transactionIdThe following fields were removed from the gateway adapter because they were actually dead code, meaning they were never even added to requests in the first place (from the
add_risk_datamethod):cvvResponseCodecvvResponsecvvMatchavsResponseavsAddressMatchavsZipMatchThe following fields were removed from requests to the gateway because they were not correctly implemented according to the gateway's documentation and they represent features that are currently not supported by Spreedly's integration with the gateway:
shouldVaultCardRubocop:
739 files inspected, no offenses detected
Unit:
5147 tests, 75501 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
Loaded suite test/remote/gateways/remote_priority_test
27 tests, 81 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed