Handle Wallet Onfido flow #8093

nkuoch · 2022-03-11T14:39:49Z

Tested with https://github.com/Expensify/Web-Secure/pull/2174

Details

Handle Onfido flow in P2P wallet activation

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/191962

Tests

Tests can be found in https://github.com/Expensify/Web-Secure/pull/2182

QA Steps

None

Screenshots

Web

nkuoch · 2022-03-21T10:04:57Z

Friendly Bump @marcaaron @ctkochan22 and @francoisl for review

francoisl · 2022-03-21T18:38:03Z

src/libs/actions/Wallet.js

@@ -212,7 +215,12 @@ function activateWallet(currentStep, parameters) {

            if (response.jsonCode !== 200) {
                if (currentStep === CONST.WALLET.STEP.ONFIDO) {
-                    Onyx.merge(ONYXKEYS.WALLET_ONFIDO, {error: response.message, loading: false});
+                    Onyx.merge(ONYXKEYS.WALLET_ONFIDO, {loading: false});
+                    if (response.title === CONST.WALLET.ERROR.ONFIDO_FIXABLE_ERROR) {


Can we use the UUID or any type of error code for this instead? As it stands, the error message sounds more likely to change in Web-Secure and break this flow.

It's the pattern we've been using everywhere else in this method. I agree with you, although I'm not a fan of using the uuid either.
I think the proper way would be either via another type of exception php (extending from ExpError), or via a key errorType in data? (Either way, let's do it in a follow up PR, so I can do it for all the other places too).

Would be good to try to start a wider conversation about this and propose a holistic fix. There's a lot of inconsistency in the new app in general WRT error handling - in many places we look at jsonCode in others the response.message etc.

francoisl · 2022-03-21T18:39:59Z

src/languages/en.js

@@ -567,6 +567,12 @@ export default {
        genericError: 'There was an error while processing this step. Please try again.',
        cameraPermissionsNotGranted: 'Camera permissions not granted',
        cameraRequestMessage: 'You have not granted us camera access. We need access to complete verification.',
+        originalDocumentNeeded: 'Please upload an original image of your ID rather than a screenshot or scanned image.',
+        documentNeedsBetterQuality: 'Your ID appears to be damaged or has missing security features. Please upload an original image of your undamaged ID that is entirely visible.',


nab but if their ID is damaged, how can the user upload an "image of [their] undamaged ID"? Would it make more sense to say: Please upload an original image of an undamaged ID ...?

francoisl · 2022-03-21T18:49:23Z

src/libs/actions/PersonalDetails.js

+ * @param {String} login
+ * @param {String} displayName
+ * @param {String} firstName
+ * @param {String} lastName


I can't really tell if we have a standard for this but looking at other functions like here or here, shouldn't be params be written:

Suggested change

* @param {String} login

* @param {String} displayName

* @param {String} firstName

* @param {String} lastName

* @param {Object} params

* @param {String} params.login

* @param {String} params.displayName

* @param {String} params.firstName

* @param {String} params.lastName

?

Maybe would also clearer to use a @param {Object} personalDetail. Seems this method is intended to be called with a row from personalDetails key in storage, but I wasn't sure on first glance.

marcaaron · 2022-03-21T20:50:28Z

src/libs/actions/PersonalDetails.js

+ * @param {String} login
+ * @param {String} displayName
+ * @param {String} firstName
+ * @param {String} lastName


Maybe would also clearer to use a @param {Object} personalDetail. Seems this method is intended to be called with a row from personalDetails key in storage, but I wasn't sure on first glance.

src/libs/actions/PersonalDetails.js

src/libs/actions/Wallet.js

src/pages/EnablePayments/AdditionalDetailsStep.js

src/pages/EnablePayments/EnablePaymentsPage.js

OSBotify · 2022-03-22T22:06:52Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-03-24T21:00:49Z

🚀 Deployed to staging by @nkuoch in version: 1.1.46-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2022-03-30T19:27:08Z

🚀 Deployed to production by @timszot in version: 1.1.46-3 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

ogumen · 2022-04-13T14:06:21Z

Hi @nkuoch, could you please let us know if there is an easy way to get to this screen in order to test it from the accessibility perspective? Can we use any testing account to get to it? Thanks!
CC @SumitDiyora @Stutikuls

nkuoch self-assigned this Mar 11, 2022

nkuoch force-pushed the nat-onfidokyc branch 3 times, most recently from 2540370 to bb6defc Compare March 14, 2022 13:00

nkuoch force-pushed the nat-p2pkycquestions branch 2 times, most recently from 0b9f587 to 42459e3 Compare March 14, 2022 14:33

Base automatically changed from nat-p2pkycquestions to main March 14, 2022 21:00

nkuoch force-pushed the nat-onfidokyc branch 12 times, most recently from 169efc5 to 94bb792 Compare March 17, 2022 13:35

Handle Wallet Onfido flow

7d67d8b

nkuoch force-pushed the nat-onfidokyc branch from 94bb792 to 7d67d8b Compare March 17, 2022 14:26

nkuoch changed the title ~~[WIP] Handle Wallet Onfido flow~~ Handle Wallet Onfido flow Mar 17, 2022

nkuoch marked this pull request as ready for review March 17, 2022 14:34

nkuoch requested a review from a team as a code owner March 17, 2022 14:34

nkuoch requested review from marcaaron and ctkochan22 March 17, 2022 14:35

MelvinBot requested review from Gonals and removed request for a team March 17, 2022 14:36

nkuoch requested review from francoisl and removed request for Gonals March 17, 2022 17:26

nkuoch added 2 commits March 17, 2022 18:42

Fix es copies

8711ad3

Update en copy

27afef7

francoisl reviewed Mar 21, 2022

View reviewed changes

marcaaron requested changes Mar 21, 2022

View reviewed changes

Fixes from Francois and Marc PR reviews

008c0bc

marcaaron approved these changes Mar 22, 2022

View reviewed changes

francoisl approved these changes Mar 22, 2022

View reviewed changes

nkuoch removed the request for review from ctkochan22 March 22, 2022 21:56

nkuoch merged commit 9c540f6 into main Mar 22, 2022

nkuoch deleted the nat-onfidokyc branch March 22, 2022 21:56

OSBotify mentioned this pull request Mar 24, 2022

Deploy Checklist: New Expensify 2022-03-24 #8305

Closed

69 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle Wallet Onfido flow #8093

Handle Wallet Onfido flow #8093

nkuoch commented Mar 11, 2022 •

edited

Loading

nkuoch commented Mar 21, 2022

francoisl Mar 21, 2022

nkuoch Mar 22, 2022

marcaaron Mar 22, 2022

francoisl Mar 21, 2022

francoisl Mar 21, 2022

marcaaron Mar 21, 2022 •

edited

Loading

marcaaron Mar 21, 2022 •

edited

Loading

OSBotify commented Mar 22, 2022

OSBotify commented Mar 24, 2022

OSBotify commented Mar 30, 2022

ogumen commented Apr 13, 2022

- * @param {String} login
- * @param {String} displayName
- * @param {String} firstName
- * @param {String} lastName
+ * @param {Object} params
+ * @param {String} params.login
+ * @param {String} params.displayName
+ * @param {String} params.firstName
+ * @param {String} params.lastName

Handle Wallet Onfido flow #8093

Handle Wallet Onfido flow #8093

Conversation

nkuoch commented Mar 11, 2022 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Screenshots

Web

nkuoch commented Mar 21, 2022

francoisl Mar 21, 2022

Choose a reason for hiding this comment

nkuoch Mar 22, 2022

Choose a reason for hiding this comment

marcaaron Mar 22, 2022

Choose a reason for hiding this comment

francoisl Mar 21, 2022

Choose a reason for hiding this comment

francoisl Mar 21, 2022

Choose a reason for hiding this comment

marcaaron Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

marcaaron Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

OSBotify commented Mar 22, 2022

OSBotify commented Mar 24, 2022

OSBotify commented Mar 30, 2022

ogumen commented Apr 13, 2022

nkuoch commented Mar 11, 2022 •

edited

Loading

marcaaron Mar 21, 2022 •

edited

Loading

marcaaron Mar 21, 2022 •

edited

Loading