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

Concatenate card and customer token #3144

Closed

Conversation

therufs
Copy link
Contributor

@therufs therufs commented Feb 18, 2019

Pinpayments handles stored cards by assigning them to a user. It seems that if
a card is associated with a user the card must be used with the customer ID
rather than the card's, and since a card can only be associated with one user,
it's not clear that the card ID generated when a card is stored will ever
actually be reusable (see the Customer API docs since cards are irretrievable once they are replaced.

The possible use case for stored card IDs is that customers may have multiple
cards if one is set as primary. Other stored cards may not be able to be used
until/unless they are set as primary.

Remote tests: 16 tests, 46 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
Unit tests: 37 tests, 112 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

@therufs therufs force-pushed the ECS-52-pinpayment-store branch from 1d4591e to f12aff6 Compare February 18, 2019 16:17
@therufs therufs requested a review from a team February 18, 2019 16:17
@therufs therufs self-assigned this Feb 18, 2019
@@ -206,7 +215,11 @@ def unparsable_response(raw_response)
end

def token(response)
response['token']
if response['token'].start_with?('cus')
"#{response.dig('card', 'token')}+#{response['token']}"
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no real standardization, but unless you think + is particularly suited for this gateway, let's use one of the more common separators, either ; or |.

Copy link
Contributor

@curiousepic curiousepic Feb 18, 2019

Choose a reason for hiding this comment

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

Also, since prior authorizations only consisted of the customer token, that should be the first element so that splitting old tokens will still parse them correctly. Maybe that's not actually applicable for this gateway but should be followed generally when adding these things.

Copy link
Member

Choose a reason for hiding this comment

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

+1 for using a common separator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@therufs therufs force-pushed the ECS-52-pinpayment-store branch 5 times, most recently from cab09f9 to ad8ee2d Compare February 19, 2019 20:03
Copy link
Member

@jknipp jknipp left a comment

Choose a reason for hiding this comment

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

👍

@therufs therufs closed this Feb 20, 2019
@therufs therufs force-pushed the ECS-52-pinpayment-store branch from ad8ee2d to 997c0a8 Compare February 20, 2019 18:20
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.

3 participants