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
Cybersource: Update NT flow #5106
Conversation
de306da
to
ecbff31
Compare
case options[:stored_credential][:reason_type] | ||
when 'installment' then 'install' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anywhere in the docs indicating the keyword has changed from install
to installment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I got back from the CyberSource contact. I'll forward you the email
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh i hate when what they say does not match their docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I emailed them again just to confirm because like you said it says install in docs. I'll keep you updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aenand It's actually install
but I've decided to remove stored credentials from here and update it in a separate ticket to keep this as simple as possible.
xml.tag! 'paymentNetworkToken' do | ||
xml.tag!('transactionType', '1') | ||
xml.tag!('requestorID', options[:trid]) if transaction_type == '3' && options[:trid] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the transaction_type == '3'
guard in case an AP/GP has a trid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just to be extra cautious but I guess they wouldn't have a trid to begin with
CHANGELOG
Outdated
@@ -153,6 +153,7 @@ | |||
* PayTrace: Always send name in billing_address [almalee24] #5086 | |||
* StripePI: Update eci format [almalee24] #5097 | |||
* Paymentez: Remove reference_id flag [almalee24] #5081 | |||
* CheckoutV2: Update NT flow [almalee24] #5091 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changelog seems incorrect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, I need to update
commit(build_auth_request(money, payment_method, options), :authorize, money, options) | ||
else | ||
# this is for NetworkToken brands that aren't supported at CyberSource | ||
Response.new(false, "Payment method #{card_brand(payment_method)} is not supported, check https://developer.cybersource.com/docs/cybs/en-us/payments/developer/all/rest/payments/CreatingOnlineAuth/CreatingAuthReqPNT.html") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we clarify in this response message that it is for network tokens? Or will the return value from card_brand(payment_method)
make that clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea, I'll update the error message since I've added this to include ApplePay and GooglePay. I decided to fix this since currently we're getting The payment gateway settings are most likely invalid.
which is very misleading.
6f985e7
to
33ecd4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for that messaging change!
Update NT flow to send cryptogram in networkTokenCryptogram. If it's a regular transaction commerceIndicator should be internet. If it's a CIT or MIT commerceIndicator could be installment, recurring or internet for unscheduled. subsequentAuthStoredCredential should be sent for all subsequent transactions. Remote: 136 tests, 676 assertions, 7 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 94.8529% passed Unit: 159 tests, 943 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
33ecd4a
to
51252ce
Compare
Update NT flow to send cryptogram in networkTokenCryptogram. If it's
a regular transaction commerceIndicator should be internet. If it's a
CIT or MIT commerceIndicator could be installment, recurring or internet
for unscheduled. subsequentAuthStoredCredential should be sent for all
subsequent transactions.
Remote:
136 tests, 676 assertions, 7 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
94.8529% passed
Unit:
158 tests, 941 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed