Add VBA flow Part 1 #3459

marcaaron · 2021-06-09T00:31:22Z

Details

Adds the Verified Bank Account flow to E.cash

For this PR I am just cleaning things up a bit and laying down some scaffolding. We will:

Create a new route /bank-account/new for adding the VBA
Extract the Plaid logic we need from the PBA flow so it can be used in both views
Add the view for manually adding a bank account

We will not be hooking up to any APIs just yet...

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/166041

Tests

Navigate to /bank-account/new
Verify the add method selection options are shown
Select manual
Verify you can only press the button when valid account number + routing number are entered
Select Log Into Bank
Verify the Plaid widget appears
Enter chase bank info user_good pass_good + Continue
Verify the options are shown in a Picker
Select an option and enter password
Repeat Plaid steps for /add-personal-bank-account route and verify the selection screen appears + an account can be added (confirm via Web-Expensify).

Note: Testing on native/desktop is tricky atm but can be done by using a deep link for native and/or putting the navigate() method on window like this:

diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js
index 7fe26fcf3..0abf6ddee 100644
--- a/src/libs/Navigation/Navigation.js
+++ b/src/libs/Navigation/Navigation.js
@@ -66,6 +66,8 @@ function navigate(route = ROUTES.HOME) {
     linkTo(navigationRef.current, route);
 }

+window.ecashNavigate = navigate;
+
 /**
  * Dismisses a screen presented modally and returns us back to the previous view.
  *

then calling window.ecashNavigate('/bank-account/new') from console / native debugger session

QA Steps

No QA - We will test the entire flow together at the end

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

marcaaron · 2021-06-09T20:27:03Z

src/components/AddPlaidBankAccount.js

+                            {!_.isEmpty(this.props.text) && (
+                                <Text style={[styles.mb5]}>{this.props.text}</Text>
+                            )}
+                            {/* @TODO there are a bunch of logos to incorporate here to replace this name


Created an issue here to follow up on this eventually https://github.com/Expensify/Expensify/issues/166782

src/languages/en.js

marcaaron · 2021-06-09T20:31:05Z

src/pages/BusinessBankAccount/NewPage.js

+    }
+
+    canSubmitManually() {
+        // @TODO add better validation


I'm thinking at least check for a 9 digit routing number + that both are all numbers? I'll look at how web-secure is handling this.

marcaaron · 2021-06-09T21:03:05Z

src/pages/BusinessBankAccount/NewPage.js

+    }
+
+    addManualAccount() {
+        // @TODO call API to add account manually


Will do in the next PR.

marcaaron · 2021-06-09T22:43:53Z

~~Still waiting on some assets but this should be ready for an initial review.~~ assets added

roryabraham

Sorry, just a preliminary review for now. Didn't have time to finish this right now.

src/pages/BusinessBankAccount/NewPage.js

src/ROUTES.js

src/components/TextLink.js

src/components/AddPlaidBankAccount.js

src/pages/BusinessBankAccount/NewPage.js

MonilBhavsar · 2021-06-10T14:35:53Z

src/components/CheckboxWithLabel.js

+};
+
+const CheckboxWithLabel = ({
+    LabelComponent, isChecked, onPress, style,


NAB, Should be moved to separate lines?

Not sure we enforce this so if eslint doesn't mind I don't mind 😄

marcaaron · 2021-06-10T19:05:17Z

This is ready for review.

roryabraham

Also NAB but if you wanted to Checkbox and CheckboxWithLabel would probably be good candidates to include in storybook

src/pages/BusinessBankAccount/NewPage.js

src/components/AddPlaidBankAccount.js

src/components/CheckboxWithLabel.js

src/pages/BusinessBankAccount/NewPage.js

src/pages/EnablePayments/TermsStep.js

roryabraham · 2021-06-10T22:37:07Z

src/pages/EnablePayments/TermsStep.js

+                        style={styles.mb4}
+                        isChecked={this.state.hasAcceptedDisclosure}
+                        onPress={this.toggleDisclosure}
+                        LabelComponent={() => (
                            <Text>
                                {`${this.props.translate('termsStep.haveReadAndAgree')} `}

                                <TextLink href="https://use.expensify.com/fees">


Not sure if it's safe to put a Pressable inside a Text, but I don't think you're supposed to.

I think this can (and should) be un-nested

Ok I'm not entirely sure either. But this flow isn't being used yet so if it's broken because of this. We'll find out eventually 😂

marcaaron · 2021-06-10T23:14:14Z

Thanks for the review @roryabraham !

Also NAB but if you wanted to Checkbox and CheckboxWithLabel would probably be good candidates to include in storybook

Fixed up a few things but I'm going to create a follow up issue for this one and a few others you've mentioned. Great stuff overall.

jasperhuangg · 2021-06-11T00:49:33Z

src/components/TextInputWithLabel.js

    errorText: '',
 };

 const TextInputWithLabel = props => (
    <>
-        <Text style={[styles.formLabel]}>{props.label}</Text>
+        {!_.isEmpty(props.label) && <Text style={[styles.formLabel]}>{props.label}</Text>}


NAB: Why are we making props.label optional? Seems strange to make a label optional on a component called TextInputWithLabel. I also don't see you using the errorText prop anywhere where you're using TextInputWithLabel without the label prop, so I'm wondering if this change is necessary.

Sorta answered this in the other comment. But this component is likely going to change in the future when we apply some standardization to TextInputs across the app as it's kind of a mess. But we gotta keep rolling.

jasperhuangg · 2021-06-11T00:54:04Z

src/pages/BusinessBankAccount/NewPage.js

+                                    style={[styles.exampleCheckImage, styles.mb5]}
+                                    source={exampleCheckImage}
+                                />
+                                <TextInputWithLabel


Followup of my comment from earlier, can't you just use vanilla TextInputs here? I'm probably missing something.

I think this decision will make more sense once...

We add error handling + take advantage of the errorText (this is just a scaffolding step)

all the inputs get refactored and we use TextInputWithLabel as a base to build off of to create the text inputs that @shawnborton would prefer we build out eventually.

jasperhuangg · 2021-06-11T00:55:59Z

src/pages/EnablePayments/TermsStep.js

@@ -78,48 +71,42 @@ class TermsStep extends React.Component {
                            Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.


wat, this is definitely a placeholder I assume

marcaaron · 2021-06-11T01:44:29Z

@MonilBhavsar gonna move forward with this one but lemme know if you have any additional comments :)

OSBotify · 2021-06-11T01:45:16Z

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

MonilBhavsar · 2021-06-11T04:18:21Z

Thanks! looks good and tests well for me considering the scope of the PR

OSBotify · 2021-06-11T17:27:41Z

🚀 Cherry-picked to staging in version: 1.0.68-0🚀

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

OSBotify · 2021-06-15T16:52:52Z

🚀 Deployed to production in version: 1.0.68-4🚀

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

marcaaron added 4 commits June 8, 2021 11:48

add business bank account new page

ecb9990

add create manual form

c128cfe

Move logic out of AddBankAccountPage into AddPlaidBankAccount

5db6177

refactor plaid link stuff

176b380

marcaaron self-assigned this Jun 9, 2021

fix conflicts

225ef6f

marcaaron changed the title ~~[WIP] Add VBA flow~~ [WIP] Add VBA flow Part 1 Jun 9, 2021

marcaaron added 4 commits June 8, 2021 14:37

fix style

35e91bf

Refactor Plaid module so it can be easily used in the the two flows

a84e2fd

fix conflicts

78ebe95

update import and clear plaid token on init

8ae125e

marcaaron commented Jun 9, 2021

View reviewed changes

src/languages/en.js Show resolved Hide resolved

marcaaron commented Jun 9, 2021

View reviewed changes

src/languages/en.js Outdated Show resolved Hide resolved

marcaaron commented Jun 9, 2021

View reviewed changes

marcaaron added 5 commits June 9, 2021 10:35

clean up beta logic

ff91605

use CheckboxWithLabel

84cfe10

Fix up checkbox usages

d9ed59a

update spanish

6fbbf5f

fix style

5cdaec2

marcaaron commented Jun 9, 2021

View reviewed changes

marcaaron added 4 commits June 9, 2021 11:08

add checks for number

d5d1d99

Use same validation that Web-Secure is using for now

f985e1a

remove null checks

5cfa72d

translate

857ac9a

marcaaron changed the title ~~[WIP] Add VBA flow Part 1~~ [HOLD] Add VBA flow Part 1 Jun 9, 2021

marcaaron marked this pull request as ready for review June 9, 2021 22:43

marcaaron requested a review from a team as a code owner June 9, 2021 22:43

marcaaron requested a review from roryabraham June 9, 2021 22:44

MelvinBot requested a review from MonilBhavsar June 9, 2021 22:44

marcaaron added 2 commits June 9, 2021 12:53

use func for propTypes

7d4a9c8

add assets

a098b66

marcaaron changed the title ~~[HOLD] Add VBA flow Part 1~~ Add VBA flow Part 1 Jun 9, 2021

marcaaron added 3 commits June 9, 2021 13:38

fix checkbox text flow on iOS

7e96346

remove returnKeyType

ac39938

fix up misaligned lock icon

45b86d0

roryabraham requested changes Jun 10, 2021

View reviewed changes

rename check image. fix up text link and consts

8a19a67

marcaaron requested a review from roryabraham June 10, 2021 00:32

do not pass array to TextLink

d04ed46

MonilBhavsar reviewed Jun 10, 2021

View reviewed changes

use higher quality image

108674f

roryabraham requested changes Jun 10, 2021

View reviewed changes

make requested changes

6b3d5f5

roryabraham approved these changes Jun 10, 2021

View reviewed changes

jasperhuangg approved these changes Jun 11, 2021

View reviewed changes

jasperhuangg requested a review from MonilBhavsar June 11, 2021 01:42

marcaaron merged commit 157db96 into main Jun 11, 2021

marcaaron deleted the marcaaron-addBusinessBankAccountPage branch June 11, 2021 01:44

roryabraham mentioned this pull request Jun 11, 2021

Deploy Checklist: Expensify.cash 2021-06-08 #3450

Closed

49 tasks

eVoloshchak mentioned this pull request May 24, 2023

[HOLD for payment 2023-05-22] [$1000] Company name is out of screen on Additional information page #18000

Closed

6 tasks

mananjadhav mentioned this pull request Jun 1, 2023

[HOLD for payment 2023-06-01] [HOLD PR #18865][$1000] iOS / Safari - On long pressing "terms of service" link it gets completely hidden #17103

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add VBA flow Part 1 #3459

Add VBA flow Part 1 #3459

marcaaron commented Jun 9, 2021 •

edited

Loading

marcaaron Jun 9, 2021

marcaaron Jun 9, 2021

marcaaron Jun 9, 2021

marcaaron commented Jun 9, 2021 •

edited

Loading

roryabraham left a comment

MonilBhavsar Jun 10, 2021

marcaaron Jun 10, 2021

marcaaron commented Jun 10, 2021

roryabraham left a comment

roryabraham Jun 10, 2021

roryabraham Jun 10, 2021

marcaaron Jun 10, 2021

marcaaron commented Jun 10, 2021

jasperhuangg Jun 11, 2021

marcaaron Jun 11, 2021

jasperhuangg Jun 11, 2021

marcaaron Jun 11, 2021 •

edited

Loading

jasperhuangg Jun 11, 2021

marcaaron Jun 11, 2021

marcaaron commented Jun 11, 2021

OSBotify commented Jun 11, 2021

MonilBhavsar commented Jun 11, 2021 •

edited

Loading

OSBotify commented Jun 11, 2021

OSBotify commented Jun 15, 2021

		@@ -78,48 +71,42 @@ class TermsStep extends React.Component {
		Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

Add VBA flow Part 1 #3459

Add VBA flow Part 1 #3459

Conversation

marcaaron commented Jun 9, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron commented Jun 9, 2021 • edited Loading

roryabraham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron commented Jun 10, 2021

roryabraham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron commented Jun 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron Jun 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcaaron commented Jun 11, 2021

OSBotify commented Jun 11, 2021

MonilBhavsar commented Jun 11, 2021 • edited Loading

OSBotify commented Jun 11, 2021

OSBotify commented Jun 15, 2021

marcaaron commented Jun 9, 2021 •

edited

Loading

marcaaron commented Jun 9, 2021 •

edited

Loading

marcaaron Jun 11, 2021 •

edited

Loading

MonilBhavsar commented Jun 11, 2021 •

edited

Loading