solidusio / solidus_stripe

💳 Integrate Solidus with Stripe
https://stripe.com
BSD 3-Clause "New" or "Revised" License
36 stars 61 forks source link

Visa credit card type is blank #36

Closed stuffmatic closed 4 years ago

stuffmatic commented 4 years ago

I just noticed the mapCC function seems to expect the string Visa for Visa cards, whereas the Stripe API seems to return visa, which causes the cc_type of the Spree::CreditCard to be empty.

The example response in this section of the Stripe API has brand set to visa, which is what I'm seeing. Intrestingly, this section seems to suggest it should be Visa.

I'm using the test Visa card 4242 4242 4242 4242 and have v3_intents set to true. I haven't yet been able to test this for other card types, so I can't suggest a general solution at this point.

Finally, a huge thanks for your work on this gem!

spaghetticode commented 4 years ago

mapCC is a rather old function, so it's probably not up-to-date with all Stripe library changes. I think that problem can be solved by converting the equality checks on strings to checks on regexp, @stuffmatic what do you think?

stuffmatic commented 4 years ago

@spaghetticode I haven't been able to find any clear documentation on this yet. However, I found a clue in the official Android SDK, where CardBrand.tk is an enum class describing credit card brands. The members have both displayName and code properties, where the latter seems to correspond to the lower case visa I saw.

Here are the displayName - code pairs from CardBrand.tk

The list above covers all valid card brands given here.

Also, I tried some test cards of different brands, and the ones I was allowed to use resulted in a brand value matching the lower case values in the list above.

So it looks like the safest fix would be to do double string comparisons in mapCC, so instead of

if (ccType === 'American Express')

do

if (ccType === 'American Express' || ccType === 'amex')

Edit: I created a PR with my suggested fix.

aldesantis commented 4 years ago

Closing this, thanks for the fix in #38. 🙂