Skip to content

[Wallet KYC] Fix Onfido and KBA flows #14988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
MonilBhavsar merged 9 commits into from
Feb 16, 2023
Merged

[Wallet KYC] Fix Onfido and KBA flows #14988

MonilBhavsar merged 9 commits into from
Feb 16, 2023

Conversation

@MariaHCD
Copy link
Contributor

@MariaHCD MariaHCD commented Feb 9, 2023 •
edited
Loading

Details

Fixed Issues

Part of #14477
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

Test steps:

  1. Have an account (under the wallet beta) with a silver/pending wallet. Note: every account under the wallet beta has a silver wallet created automatically.
  2. If this account does not have a personal bank account (PBA) set up, follow the below steps:
    • Log into NewDot and navigate to Settings > Payments > Add payment method > Bank account
    • Select Connect via plaid and use the following credentials:
Wells Fargo
Username: user_good
password: pass_good
Mobile verification code: credential_good
Plaid saving
  1. Have another user request money to the user you are testing with
  2. Attempt to pay the money request and verify the KYC wall is triggered
  3. In the additional details form, enter first name Alberta, last name Charleson and SSN as 3333. Enter any data for the other fields.
  4. Click Save & continue
  5. Verify you are able to proceed to the Verify Identity step
  6. Go back to the Additional details step
  7. Change the last name to Bobbeth
  8. Verify you get an SSN error. Without changing any data, click continue again
  9. Verify you are brought to the KBA flow
  10. To pass this step, select the first answer for every question (important otherwise the KBA check will fail)
  11. Verify you proceed to the Verify Identity step and can start the Onfido flow
  12. Once you've tested the above flow on all platforms
  13. Use any platform to finish the wallet KYC process and proceed to complete the Onfido flow. You can use a test document (example)
  14. Accept the wallet terms and verify you see the pending page:

Screenshot 2023-02-13 at 4 14 37 PM

Offline tests

None of the wallet KYC steps should be available offline

QA Steps

  • Same as test steps

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
New.Expensify.webm
Mobile Web - Chrome
XRecorder_Edited_13022023_174830.mp4
Mobile Web - Safari
Screen.Recording.2023-02-14.at.3.52.09.PM.mov
Desktop
Screen.Recording.2023-02-13.at.4.17.49.PM.mov
iOS
Screen.Recording.2023-02-14.at.4.01.00.PM.mov
Android
Screen.Recording.2023-02-13.at.5.55.37.PM.mov

@MariaHCD MariaHCD self-assigned this Feb 9, 2023
@MariaHCD MariaHCD mentioned this pull request Feb 13, 2023
Closed
1 task
@MariaHCD MariaHCD marked this pull request as ready for review February 14, 2023 12:09
@MariaHCD MariaHCD requested a review from a team as a code owner February 14, 2023 12:09
@melvin-bot melvin-bot bot requested review from MonilBhavsar and removed request for a team February 14, 2023 12:09
@MelvinBot
Copy link

MelvinBot commented Feb 14, 2023

@MonilBhavsar Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@MariaHCD MariaHCD requested a review from nkuoch February 14, 2023 12:10
MariaHCD
MariaHCD commented Feb 14, 2023
View reviewed changes
src/libs/actions/Wallet.js
* @param {String} dob
*/
function openOnfidoFlow(firstName, lastName, dob) {
API.read('OpenOnfidoFlow', {firstName, lastName, dob}, {
Copy link
Contributor Author

@MariaHCD MariaHCD Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For context, we're now fetching these params in the API instead of relying on the frontend to send them: https://github.com/Expensify/Web-Expensify/pull/36435

MonilBhavsar
MonilBhavsar reviewed Feb 15, 2023
View reviewed changes
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Code looks good. Testing now...

src/pages/EnablePayments/IdologyQuestions.js
message={errorMessage}
isLoading={this.props.walletAdditionalDetails.isLoading}
buttonText={this.props.translate('common.saveAndContinue')}
containerStyles={[styles.mh0, styles.mv0, styles.mb0]}
Copy link
Contributor

@MonilBhavsar MonilBhavsar Feb 15, 2023

Choose a reason for hiding this comment

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

NAB, these style rules can be shifted to a class, as they're used on multiple pages

@MonilBhavsar
Copy link
Contributor

MonilBhavsar commented Feb 15, 2023 •
edited
Loading

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified tests pass on all platforms & I tested again on:
    • Android / native
    • Android / Chrome
    • iOS / native
    • iOS / Safari
    • MacOS / Chrome / Safari
    • MacOS / Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Screen.Recording.2023-02-16.at.4.28.49.PM.mov
Screen.Recording.2023-02-16.at.5.53.31.PM.mov
Mobile Web - Chrome
Screen.Recording.2023-02-16.at.5.45.38.PM.mov
Mobile Web - Safari
Screen.Recording.2023-02-16.at.4.48.09.PM.mov
Desktop
Screen.Recording.2023-02-16.at.4.35.43.PM.mov
iOS
Screen.Recording.2023-02-16.at.4.43.21.PM.mov
Android
Screen.Recording.2023-02-16.at.5.20.15.PM.mov

@MariaHCD
Copy link
Contributor Author

MariaHCD commented Feb 16, 2023

Bump @MonilBhavsar :)

MonilBhavsar
MonilBhavsar approved these changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@MonilBhavsar MonilBhavsar left a comment

Choose a reason for hiding this comment

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

Works fine!
Sorry, I ran into some issues with my Android simulator

@MonilBhavsar MonilBhavsar merged commit 7d7fc3d into main Feb 16, 2023
@MonilBhavsar MonilBhavsar deleted the maria-fix-wallet-onfido-flow branch February 16, 2023 12:28
@OSBotify
Copy link
Contributor

OSBotify commented Feb 16, 2023

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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 16, 2023

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries
Name Duration
App start TTI 748.049 ms → 780.002 ms (+31.953 ms, +4.3%)
App start runJsBundle 200.844 ms → 220.656 ms (+19.813 ms, +9.9%)
App start nativeLaunch 20.161 ms → 20.742 ms (+0.581 ms, +2.9%)
App start regularAppStart 0.016 ms → 0.019 ms (+0.003 ms, +17.4%) 🟡
Open Search Page TTI 600.538 ms → 592.883 ms (-7.655 ms, -1.3%)
Show details
Name Duration
App start TTI Baseline
Mean: 748.049 ms
Stdev: 26.494 ms (3.5%)
Runs: 699.6603890005499 706.1331689991057 713.3925039991736 713.5458179991692 714.4664130005985 719.2696109991521 720.3472300004214 723.2963820006698 730.9286419991404 736.463546000421 740.733932999894 743.0914740003645 744.7047780007124 745.1872010007501 747.9857320003211 751.1345220003277 751.6653760001063 753.8966259993613 757.354240000248 761.1066830009222 763.5337700005621 764.1100660003722 764.8279539998621 765.5057679992169 766.3624360002577 767.4044569991529 783.1541520003229 793.4095820002258 795.173071000725 803.6201050002128

Current
Mean: 780.002 ms
Stdev: 24.294 ms (3.1%)
Runs: 727.388513000682 748.7995120007545 749.4627149999142 755.0322520006448 756.0731109995395 759.5990139991045 762.0764160007238 762.4158110003918 763.3978300001472 765.4012550003827 766.5154160000384 769.8155160006136 772.2326900009066 773.3885500002652 777.4619100000709 777.4925970006734 777.532919999212 780.6919330004603 783.925232000649 785.2231530006975 790.364280000329 793.4137130007148 795.413465000689 798.5766889993101 810.7499289996922 813.8256080001593 815.4376330003142 817.0765569992363 818.7592029999942 832.5079829990864
App start runJsBundle Baseline
Mean: 200.844 ms
Stdev: 14.390 ms (7.2%)
Runs: 169 170 171 182 184 185 191 193 193 195 196 198 199 200 201 204 204 205 205 207 208 209 211 211 213 215 215 215 216 219 219 224

Current
Mean: 220.656 ms
Stdev: 17.010 ms (7.7%)
Runs: 195 197 197 199 200 201 203 205 206 207 211 214 216 219 219 220 220 223 225 226 226 228 228 229 232 232 241 242 242 247 248 263
App start nativeLaunch Baseline
Mean: 20.161 ms
Stdev: 2.034 ms (10.1%)
Runs: 17 17 18 18 18 18 18 19 19 19 19 19 19 19 20 20 20 20 20 21 21 21 21 22 22 22 23 23 23 24 25

Current
Mean: 20.742 ms
Stdev: 1.934 ms (9.3%)
Runs: 18 18 18 19 19 19 19 19 19 19 20 20 20 20 20 21 21 21 21 21 21 21 22 22 22 22 23 24 24 25 25
App start regularAppStart Baseline
Mean: 0.016 ms
Stdev: 0.001 ms (8.2%)
Runs: 0.013753000646829605 0.0138349998742342 0.014038000255823135 0.014322999864816666 0.015095999464392662 0.015258001163601875 0.015258999541401863 0.015300000086426735 0.015340998768806458 0.01574699953198433 0.015828000381588936 0.015869000926613808 0.015951000154018402 0.015990998595952988 0.016154000535607338 0.016235001385211945 0.01631700061261654 0.016479000449180603 0.016520000994205475 0.01656000129878521 0.016683001071214676 0.016804998740553856 0.017048999667167664 0.017130998894572258 0.017170999199151993 0.01737399958074093 0.017903000116348267 0.018310001119971275 0.018432999029755592 0.019409000873565674

Current
Mean: 0.019 ms
Stdev: 0.001 ms (6.3%)
Runs: 0.016764000058174133 0.016927000135183334 0.01700899936258793 0.0172520000487566 0.017619000747799873 0.017741000279784203 0.01798499934375286 0.01810700073838234 0.018350999802350998 0.018432999029755592 0.0185139998793602 0.01855500042438507 0.018636001273989677 0.01892099902033806 0.019164999946951866 0.01932700164616108 0.01936899870634079 0.019450001418590546 0.019491000100970268 0.019652999937534332 0.019652999937534332 0.019734999164938927 0.0197759997099638 0.020020000636577606 0.02006000094115734 0.020100999623537064 0.020142000168561935 0.02018200047314167 0.020262999460101128 0.020263999700546265 0.022135000675916672
Open Search Page TTI Baseline
Mean: 600.538 ms
Stdev: 29.169 ms (4.9%)
Runs: 555.1443290002644 555.5882169995457 559.6872969996184 562.7993170004338 564.8048500008881 575.7840579990298 576.4427899997681 580.6803389992565 581.1073410008103 583.0526529997587 585.550699999556 586.3981929998845 587.2174069993198 589.9899100009352 596.0364580005407 597.343628000468 600.950154999271 601.4078379999846 603.9523519985378 604.9523520004004 607.8107099998742 613.106282999739 617.6475419998169 618.1018059998751 622.9421800002456 623.3382580000907 629.0759280007333 629.2633870001882 633.6015620008111 637.9311120007187 648.6663409993052 686.8343109991401

Current
Mean: 592.883 ms
Stdev: 31.683 ms (5.3%)
Runs: 546.8220620006323 556.482137998566 558.7877200003713 559.6479090005159 560.8918859995902 561.0551350004971 563.2353109996766 567.3825279995799 570.3682460002601 570.5209149997681 576.5095220003277 578.6841230001301 580.7395019996911 581.125854998827 583.0759690012783 583.8052159994841 584.6246750000864 588.40633200109 590.1494950000197 596.9279380012304 597.0002449993044 598.0203040000051 599.6823740005493 605.1714679989964 610.4750569984317 612.4195150006562 618.4047040008008 620.1378169991076 623.375447999686 625.7206619996578 630.9560139998794 678.1654460001737 686.3586829993874

@OSBotify OSBotify mentioned this pull request Feb 16, 2023
Closed
11 tasks
@mvtglobally
Copy link

mvtglobally commented Feb 16, 2023

@MariaHCD Can we use Expensifail account for this PR? or any specific account to validate it?

@OSBotify
Copy link
Contributor

OSBotify commented Feb 16, 2023

🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 1.2.73-0 🚀

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

@MariaHCD
Copy link
Contributor Author

MariaHCD commented Feb 17, 2023

@mvtglobally You can use any account under the wallet beta. An expensifail account should work for this. Let me know if you run into any issues.

This was referenced Feb 17, 2023
Closed
Closed
@OSBotify
Copy link
Contributor

OSBotify commented Feb 17, 2023

🚀 Deployed to production by https://github.com/francoisl in version: 1.2.73-1 🚀

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

1 similar comment
@OSBotify
Copy link
Contributor

OSBotify commented Feb 17, 2023

🚀 Deployed to production by https://github.com/francoisl in version: 1.2.73-1 🚀

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

@fedirjh fedirjh mentioned this pull request May 29, 2023
Merged
57 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nkuoch nkuoch nkuoch approved these changes

@MonilBhavsar MonilBhavsar MonilBhavsar approved these changes

Assignees

@MariaHCD MariaHCD

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@MariaHCD @MelvinBot @MonilBhavsar @OSBotify @mvtglobally @nkuoch