-
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: Add merchant_id #4844
Conversation
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 looks good! Can you also make this change to the CyberSource REST gateway?
Yes, I'll make those changes. I just realized that @options[:merchant_id] was different than this new field |
37e6cce
to
8cf9338
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.
Looks good!
@@ -556,7 +556,7 @@ def add_line_item_data(xml, options) | |||
end | |||
|
|||
def add_merchant_data(xml, options) | |||
xml.tag! 'merchantID', @options[:login] | |||
xml.tag! 'merchantID', options[:merchant_id] || @options[:login] |
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.
options
vs @options
here is interesting. This makes sense, since it looks like @options
is a hash for "gateway options" and options
is a hash for "request options", so merchant_id
is passed after the gateway was originally created.
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.
Nice work!
string_to_sign = { | ||
host: host, | ||
date: gmtdatetime, | ||
"(request-target)": "#{http_method} /pts/v2/#{resource}", | ||
digest: digest, | ||
"v-c-merchant-id": @options[:merchant_id] | ||
"v-c-merchant-id": options[:merchant_id] || @options[:merchant_id] |
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.
After reviewing the REST docs I made a mistake in the requirements. I believe we should only override it for the HTTP header, not the Signature.
8cf9338
to
fc83329
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.
Nice work!
fc83329
to
7f9cfca
Compare
Set the merchantId filed to options[:merchant_id] instead of @options[:login] if available. Cybersource Unit: 136 tests, 641 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed CybersourceRest Unit: 30 tests, 144 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed
7f9cfca
to
21d987d
Compare
Set the merchantId filed to options[:merchant_id] instead of @options[:login] if available.
Unit: 136 tests, 641 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed