Translate credit card brand so that active merchand code handles the payment correctly #5070
Conversation
app/assets/javascripts/admin/payments/services/stripe_elements.js.coffee
Outdated
Show resolved
Hide resolved
|
Hi @luisramos0, Tested the console logging as you suggested, it works fine! This is great. In what concerns the #5068 issue: I first reproduced it, and before staging this PR observed that payment with the mastercard 5555555555554444 shows the error the error reported in #5068:
and the console shows the error:
After staging this PR, both the regular Visa and Mastercard now yielded the reported error: 4242424242424242 | Visa | Any 3 digits | Any future date
and the console shows the same error:
Also, this PR appears to break SCA payments. Using SCA payment method and SCA Card: 4000002500003155 results in Snail, console pic below:
Moving back to In Dev 💪 |
|
I think you just experienced some issue in the stg FR. What stripe config is there? This issue 5068 is not replicable with 5555555555554444, payments with this card should work in both stripe and stripeSCA. Before and after this PR. We need to test this again. |
|
Thank you @luisramos0 , |
|
I tested this further, this PR is not correct. I'll add a new commit in a few minutes. |
luisramos0
force-pushed
the
master_card
branch
2 times, most recently
from
ed26bb5 to
96cbad6
Compare
…payment correctly Adds a simple console.log statement in case there is an error adding the card
…s a problem with their card Add paymentMethodsAPI specific mapping function, we had some errors in production with mastercards probably caused by ActiveMerchant not handling the card type correctly
|
Hi @luisramos0 , I am having a look in AU-staging, before staging this PR. Some observations: a) In this shop (Tapaskoop), both SCA and Stripe connect are set, and SCA-authentication is working fine, payments are successful with 3D cards, also with the simple 4242424242424242 Visa-card. So, I think SCA-settings are fine. b) With Stripe-Connect, both 4242424242424242 and 5555555555554444 work fine too. However, while attempting to use the mastercard 5555555555554444 with the Stripe-SCA payment method I observe the error message in #5068, as before on the FR-staging. c) after attempting to checkout with Stripe-SCA and the 5555555555554444 and getting the error "The checkout failed. Please let us know so that we can process your order." other payment methods seem to get "blocked" (i.e. fail), on that checkout session. d) after seeing this error, all other payment methods fail - even simple cash payment - or the 4242424242424242 or 5555555555554444 cards, which normally work on Stripe-Connect (as in b)). e) The workaround is to use Stripe-SCA with 4242424242424242 or a 3D-secure card, which works. f) Another workaround is to go back to shop and back to cart, avoiding to use the combination 5555555555554444+StripeSCA. Doing this, allows one to use some other method/card, like, a) or b) or cash. Summarizing, 5555555555554444+StripeSCA does not work and kind of "blocks" the checkout for other payment methods, so I can't verify your previous statement: "with 5555555555554444, payments with this card should work in both stripe and stripeSCA. Before and after this PR." I'm staging the PR, and see what happens :-) |
|
nice testing!!! |
|
Awsome @luisramos0 !
I think it closes #5068 🎉 and I guess what I observed in the FR-Staging was the "blocking" of the order. Moving to ready to go! ✌️ |
|
nice, I am not sure about the failure of the mastercard before this PR (maybe a replication of 5068, maybe not) but anyway, it's great it's working and we should move forward because this change is needed anyway. But I made a process mistake here because I have changed the code and didnt wait for a re-review. I pinged devs on slack but I forgot to ask you (Filipe) to wait for the review :-( |
| when 'MasterCard' then return 'master' | ||
| when 'Visa' then return 'visa' | ||
| when 'American Express' then return 'american_express' | ||
| when 'Discover' then return 'discover' | ||
| when 'JCB' then return 'jcb' | ||
| when 'Diners Club' then return 'diners_club' | ||
|
|
||
| # Maps the brand returned by Stripe's paymentMethodsAPI to that required by activemerchant | ||
| mapPaymentMethodsApiCardBrand: (cardBrand) -> |
There was a problem hiding this comment.
I understand this PR pretty needed that is why I approve, but we'll need to remove this duplication right after.
There was a problem hiding this comment.
ah, I think we have discussed this before. we dont have a way to share code between darkswarm and admin!
What? Why?
I believe it closes #5068
We convert card types so that active merchant handles the payment correctly. For some reason I didnt copy this logic from the old stripe payment method. We need this.
This is done in checkout and backend payments collection.
Additionally, I added a simple console.log statement to LOG errors on JS calls to Stripe. We will have the console log in the browser available to see what went wrong. This was discussed here: #5003 (comment)
This is how it will look like in the user browser console:
What should we test?
Verify stripeSCA payments are working as before.
We can test the logging by checking out with a test stripe card but inserting a invalid expiry date. It shows a red message but you can click checkout anyway and check the console log.
Release notes
Changelog Category: Fixed
Fix a problem in stripeSCA for some card types like mastercard.