CyberSource Rest: Add the mcc field #5253
Conversation
There was a problem hiding this comment.
@yunnydang I think we will need to add this to cyber_source as well as cyber_source_rest
It also may require some additional wrangling on the Spreedly side if this adapter is expecting mcc to be nested in the three_d_secure block rather than options but we can discuss that further in another thread
yunnydang
force-pushed
the
cybersource_rest_add_mcc
branch
from
ac91603
to
67ac270
Compare
| @@ -138,6 +138,7 @@ def add_three_ds(post, payment_method, options) | |||
| post[:consumerAuthenticationInformation][:paSpecificationVersion] = three_d_secure[:version] if three_d_secure[:version] | |||
| post[:consumerAuthenticationInformation][:directoryServerTransactionID] = three_d_secure[:ds_transaction_id] if three_d_secure[:ds_transaction_id] | |||
| post[:consumerAuthenticationInformation][:eciRaw] = three_d_secure[:eci] if three_d_secure[:eci] | |||
| post[:consumerAuthenticationInformation][:mcc] = options[:mcc] if options[:mcc] | |||
There was a problem hiding this comment.
hey @jcreiff after our discussion I changed it to be be pulling the mcc from the options object however would it pose any issue or concern if line 128 is still there? im thinking theres no point to send the mcc by itself if none of the rest of the threeds fields are there anyways but wanted to run it by you, thanks!
There was a problem hiding this comment.
I think that guard clause should stay as is - the 3DS info is the main reason to enter this block of logic, and it doesn't seem like we want to entertain the option of only entering this method to add MCC.
However, this brings up a question about the requested use case that initiated this work, which I will follow up on in a separate thread
yunnydang
force-pushed
the
cybersource_rest_add_mcc
branch
from
67ac270
to
c5b76bf
Compare
| if options[:payer_auth_validate_service] | ||
| xml.tag! 'payerAuthValidateService', { 'run' => 'true' } do | ||
| xml.tag! 'signedPARes', options[:pares] | ||
| xml.tag! 'signedPARes', options[:pares] if options[:pares] |
There was a problem hiding this comment.
Any particular reason to add this? I think it should be reliably included in options based on the way we communicate these details from the Spreedly side
| @@ -138,6 +138,7 @@ def add_three_ds(post, payment_method, options) | |||
| post[:consumerAuthenticationInformation][:paSpecificationVersion] = three_d_secure[:version] if three_d_secure[:version] | |||
| post[:consumerAuthenticationInformation][:directoryServerTransactionID] = three_d_secure[:ds_transaction_id] if three_d_secure[:ds_transaction_id] | |||
| post[:consumerAuthenticationInformation][:eciRaw] = three_d_secure[:eci] if three_d_secure[:eci] | |||
| post[:consumerAuthenticationInformation][:mcc] = options[:mcc] if options[:mcc] | |||
There was a problem hiding this comment.
I think that guard clause should stay as is - the 3DS info is the main reason to enter this block of logic, and it doesn't seem like we want to entertain the option of only entering this method to add MCC.
However, this brings up a question about the requested use case that initiated this work, which I will follow up on in a separate thread
|
To provide a cleaner slate for the maintenance of the library, this PR/Issue is being labeled stale after 60 days without activity. It will be closed in 14 days unless you comment with an update regarding its applicability to the current build. Thank you! |
Adds the optional mcc field
Local:
6016 tests, 80320 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Unit:
40 tests, 217 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Remote:
54 tests, 177 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed