-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 methods to allow for recurring AP #4817
Conversation
CER-701 Remote Tests: 122 tests, 609 assertions, 6 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 95.082% passed Unit Tests: 132 tests, 628 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed * Didn’t add a unit test but I can if needed Local Tests: 5541 tests, 77545 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
# stored_credential_overrides is not documented on the gateway guide in docs | ||
# I think the easiest way to solve for apple pay recurring is to create a new field within the hash | ||
# commerce_indicator can be passed in as 'internet' but is assigned that value in an earlier method so will be overwritten by the card brand if there is not something done within this method | ||
commerce_indicator = 'internet' if options.dig(:stored_credential_overrides, :type) == 'apple_pay' |
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.
Are you envisioning the merchant passing in this GSF or adding when we build the params in the gateway class?
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 was a thought, but this was more of a means to an end to see if I could get a passing remote test. I'm open to other options.
@rachelkirk do we know if this also works with GP? If so would a solution of checking |
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.
At first I wasn't sure if using only the token but not cryptogram was accurate to how a digital wallet was intended to be used. But it looks like the token is meant to be used for recurring payments, or any payment not originating on the device (source). I just had a question about if we should be localizing when we remove the cryptogram.
case brand | ||
when :visa | ||
xml.tag! 'ccAuthService', { 'run' => 'true' } do | ||
xml.tag!('cavv', payment_method.payment_cryptogram) |
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.
Are we removing the cryptogram fields entirely or only for recurring transactions?
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.
Only for recurring Apple Pay transactions. That's why I said the guard clauses should probably be revisited, but this was more of a draft/means to get getting a passing test.
@aenand |
CER-701
According to CyberSource, the cryptogram should be eliminated from the request and stored credentials should be set to merchant and unscheduled. Also in order for this to work, commerce_indicator must be
internet
.Remote Tests:
122 tests, 609 assertions, 6 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 95.082% passed
Unit Tests:
132 tests, 628 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
Local Tests:
5541 tests, 77545 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed