Skip to content

Save correct cardholder name in Spree::CreditCard#69

Merged
spaghetticode merged 1 commit into
solidusio:masterfrom
spaghetticode:spaghetticode/card-holder-name
Jun 29, 2020
Merged

Save correct cardholder name in Spree::CreditCard#69
spaghetticode merged 1 commit into
solidusio:masterfrom
spaghetticode:spaghetticode/card-holder-name

Conversation

@spaghetticode

Copy link
Copy Markdown
Member

When paying with Payment Intents either using the HTML form or the payment request button, we need to properly detect the cardholder name in order to save the correct information in the credit card DB record.

Fixes #68

When paying with Payment Intents either using the HTML form or the
payment request button, we need to properly detect the cardholder
name in order to save the correct information in the credit card
DB record.
@spaghetticode spaghetticode self-assigned this Jun 26, 2020
@spaghetticode spaghetticode changed the title Save correct card holder name in Spree::CreditCard Save correct cardholder name in Spree::CreditCard Jun 26, 2020

@aldesantis aldesantis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@spaghetticode the change itself looks good, but with all these methods and conditionals that extract data from Stripe's response I feel like there's a lot of indirection going on which makes it hard to figure out what's happening. At some point (or even in this PR if you have time) we may want to streamline them by moving more stuff to payment_params — it may make the method longer, but it will also make it easier to understand.

@spaghetticode

Copy link
Copy Markdown
Member Author

@aldesantis yes, I was not 100% comfortable with the added logic as well, but I think ATM fixing the bug would be the priority - after all, it's private methods. I am not sure about complicating payment_params, I would like to explore also the idea of extracting all that logic into another class.

@aldesantis

Copy link
Copy Markdown
Member

Sounds good, this is GTM as far as I'm concerned!

@spaghetticode spaghetticode merged commit 13de335 into solidusio:master Jun 29, 2020
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.

Card name ignored when adding new card to an order during checkout

2 participants