Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.32k stars 2.76k forks source link

[HOLD for payment 2024-08-29] [$250] [Payment card / Subscription] add UI for SCA error banner and re-authentication flow #46064

Closed blimpich closed 2 weeks ago

blimpich commented 1 month ago

If you don't know what SCA is, it stands for strong customer authentication, and is an extra authentication step that we use for users who add billing cards with GBP as their payment currency. You don't need to know about SCA in depth in order to work on this issue.

https://github.com/Expensify/App/issues/42432 added the most straightforward use case of adding a payment card via the SCA flow and having the authentication succeed, but we are stilling lacking an error banner when authentication doesn't succeed. We also need to give the user a way to authenticate again if authentication fails.

When a user goes through the SCA authentication in new dot right now and purposely fails the authentication this is what happens:

https://github.com/user-attachments/assets/bf26e4f8-bf42-4a73-9a09-dcba336e9f09

This is wrong. The UI makes it seem like the payment card is ready to be used and is fine, when its actually not fine. We should be showing the following UI with the error banner and "Authenticate payment" button.

Image

Clicking "Authenticate payment" should open up the right-docked add payment card pane with and also open up the iframe authentication model (src/pages/settings/Subscription/CardAuthenticationModal/index.tsx), restarting the authentication process.

The tricky thing about this issue is that its not possible for non-expensify engineers to test end to end, so you will be working closely with @blimpich to ensure this is working as intended.

The error banner should show when the onyx key nvp_private_stripeCustomerID has its status attribute equal to "authentication_required".

Relevant section of design doc. This doc also has the approved Spanish translations necessary to complete this. Please message @blimpich if you do not have access to this document and would like access, and make sure to include your email address.

Issue OwnerCurrent Issue Owner: @twisterdotcom
waterim commented 1 month ago

Hello Im Artem from Callstack and would like to work on this issue

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @twisterdotcom (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

blimpich commented 1 month ago

Added @mananjadhav because they are the C+ review the PR. Chose them since they have good context with subscription page.

melvin-bot[bot] commented 1 month ago

@mananjadhav, @twisterdotcom, @blimpich, @waterim Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 1 month ago

@mananjadhav, @twisterdotcom, @blimpich, @waterim Still overdue 6 days?! Let's take care of this!

twisterdotcom commented 1 month ago

Is this still waiting on @waterim?

waterim commented 1 month ago

This one is under development already

blimpich commented 1 month ago

Yeah this one is kind of special because the feature can't be fully tested by @waterim locally, so I'm testing it and providing feedback while @waterim works on it. This flow slows us down but I'm simultaneously working on code that will make @waterim and all contributors able to test this flow with just an engineer's ngrok credentials.

blimpich commented 1 month ago

@waterim I think you should be able to use my ngrok and test the full flow for 3ds verification. Let me know if you have any issues 🙂

As always though it probably won't work unless I'm online.

melvin-bot[bot] commented 1 month ago

@mananjadhav, @twisterdotcom, @blimpich, @waterim Eep! 4 days overdue now. Issues have feelings too...

blimpich commented 1 month ago

@waterim is this ready for review by a C+ contributor yet?

melvin-bot[bot] commented 1 month ago

@mananjadhav, @twisterdotcom, @blimpich, @waterim 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

mananjadhav commented 4 weeks ago

I'll start reviewing this soon.

melvin-bot[bot] commented 3 weeks ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 3 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.23-0 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-08-29. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 3 weeks ago

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

blimpich commented 2 weeks ago

No regression tests needed, its a new feature, we have manual tests already written for this.

melvin-bot[bot] commented 2 weeks ago

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

twisterdotcom commented 2 weeks ago

Payment Summary:

garrettmknight commented 2 weeks ago

$250 approved for @mananjadhav