Skip to content
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

Cyber source rest store unstore #4709

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Sainterman
Copy link

@Sainterman Sainterman commented Feb 24, 2023

CyberSourceREST: Add Store and Unstore methods

To store a payment method in CyberSource the Token management system(TMS) provides
the customer, InstrumentIdentifier and PaymentInstrument endpoints.
Sending a purchase request can be done with the customer_id returned by the customer endpoint once a paymentInstrument and an instrumentIdentifier are created.

Heavyblade and others added 8 commits February 1, 2023 14:35
CybersourceRest: adding gateway

Summary:
------------------------------
Adding CybersourceRest gateway with authorize and purchase
calls.

GWI-474

Remote Test:
------------------------------

Finished in 3.6855 seconds.
6 tests, 17 assertions, 1 failures, 0 errors, 0 pendings,
0 omissions, 0 notifications
100% passed

Unit Tests:
------------------------------
Finished in 35.528692 seconds.
5441 tests, 77085 assertions, 0 failures, 0 errors, 0 pendings,
0 omissions, 0 notifications
100% passed

RuboCop:
------------------------------
760 files inspected, no offenses detected
…hant/active_merchant into CyberSourceREST_store_unstore
post[:buyerInformation][:merchantCustomerId] = options[:customer_id]
post[:buyerInformation][:email] = options[:email].presence || 'null@cybersource.com'
add_code(post, options)
post[:merchantDefinedInformation] = []
Copy link
Contributor

@sinourain sinourain Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that this is twice (82 and 86 have the same merchantDefinedInformation definition)

when /payments/
response['status'] == 'AUTHORIZED'
else
!response['id'].nil?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we use response['id'].present?

@Sainterman Sainterman marked this pull request as ready for review March 2, 2023 15:57
@gasb150 gasb150 marked this pull request as draft March 7, 2023 21:37
Copy link
Collaborator

@gasb150 gasb150 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments

else
return response['id'].present? unless http_method == :delete

return true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This will return true for any case.

def message_from(response)
return response['status'] if success_from(response)
def message_from(action, response, http_method)
return response['status'] if success_from(action, response, http_method)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of recalling the succes_from method, it is possible to use the call in the commit method.

def commit
# commit code
succeeded = success_from(action, response, http_method)
Response.new(
succeeded,
message_from(response, succeeded)
# other response fields
error_code: error_code_from(response, succeeded)
)
# other commit code
end

def message_from(response, succeeded)
        return response['status'] if succeeded

        response['errorInformation']['message']
      end

def error_code_from(response, succeeded)
        response['errorInformation']['reason'] unless succeeded
      end

@curiousepic
Copy link
Contributor

Marking this "of interest" before a cleanup of stale PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants