[$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts

m-natarajan commented 3 weeks ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.25-0 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @shawnborton Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724687808309139

Action Performed:

Sign up a new public domain account
Click on avatar > Wallet
Click on enable wallet
Expected Result:

Step 2 and 3"Add bank account" should be enabled

Actual Result:

"Add bank account" is disabled and grayed out For the new unvalidated account, we'd mimic something like this where we just ask you to verify your account:

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

CleanShot 2024-08-26 at 11 57 03@2x CleanShot 2024-08-26 at 11 55 25@2x Snip - (2) New Expensify - Google Chrome (3)

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~019c85c6a0fbfa60c9
Upwork Job ID: 1831260528939127027
Last Price Increase: 2024-09-04
Automatic offers:

jjcoffee | Reviewer | 103880851

Issue Owner

Current Issue Owner: @jjcoffee

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to @miljakljajic (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

daledah commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-27 17:11:58 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts

What is the root cause of that problem?

New Feature

What changes do you think we should make in order to solve the problem?

Create new translation key for 'This feature requires you to validate your account' and 'Validate your account'
We should build new component VerifyAccountPage using ValidateCodeForm component like ContactMethodDetailsPage with header and message we define in step 1
Add new route for this page like ROUTES.VERIFY_ACCOUTNT_PAGE
We should remove this condition

https://github.com/Expensify/App/blob/d4d1c0cf1ff50ebe4b7cc7934f932ed723eeea06/src/pages/settings/Wallet/PaymentMethodList.tsx#L324

When the user is not validated, we will navigate to the verify account page. Update onPress function here

  onPress={()=>{
      if(!isUserValidated){
          Navigation.navigate(ROUTES.VERIFY_ACCOUTNT_PAGE.getRoute(currentUserPersonalDetails.login ?? ''));
          return;
      }
      onPress();
  }}

What alternative solutions did you explore? (Optional)

NA

cretadn22 commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-27 17:13:03 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Wallet empty state has disabled "Add bank account" button for new unvalidated accounts

What is the root cause of that problem?

New refactor

What changes do you think we should make in order to solve the problem?

Remove the isUserValidated condition from two places

https://github.com/Expensify/App/blob/d4d1c0cf1ff50ebe4b7cc7934f932ed723eeea06/src/pages/settings/Wallet/PaymentMethodList.tsx#L324

https://github.com/Expensify/App/blob/d4d1c0cf1ff50ebe4b7cc7934f932ed723eeea06/src/pages/EnablePayments/AddBankAccount/SetupMethod.tsx#L55

If the user clicks the button and is not validated, they will be redirected to the SETTINGS_CONTACT_METHOD_DETAILS page. Additionally, the backTo parameter needs to be updated and adopt into the ContactMethodDetailsPage.

In SETTINGS_CONTACT_METHOD_DETAILS, we add a note to that explains why we need to validate for that particular feature like this: "This feature requires you to validate your account" as suggested https://github.com/Expensify/App/issues/48041#issuecomment-2313076530. We need a condition to determine when to display this note, so it won't be shown in every case. We can choose to display the note based on the backTo parameter

Output:

https://github.com/user-attachments/assets/20b6281e-1dd1-431a-84f2-d09612a41a4a

What alternative solutions did you explore? (Optional)

Optionally, we can also show an announcement to the user with two choices: either navigating to SETTINGS_CONTACT_METHOD_DETAILS, and sign in again

Krishna2323 commented 3 weeks ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts

What is the root cause of that problem?

Improvement

What changes do you think we should make in order to solve the problem?

Firstly should remove the disabled prop. https://github.com/Expensify/App/blob/e244e51b7ece393d0f8d725e0ea46fd328036409/src/pages/settings/Wallet/PaymentMethodList.tsx#L324

Then we should add ConfirmModal in the component, the modal will let the user know that their account isn't validated. We will show a button Validate in the modal and if the user selects it, we will navigate to SETTINGS_CONTACT_METHOD_DETAILS.

       const [isValidateModalOpen, setIsValidateModalOpen] = useState(false);

        <ConfirmModal
            title="Account not validated"
            isVisible={isValidateModalOpen}
            onConfirm={() => {
                const backTo = Navigation.getActiveRouteWithoutParams();
                Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.getRoute(user.login ?? '', backTo));
                setIsValidateModalOpen(false);
            }}
            onCancel={() => setIsValidateModalOpen(false)}
            prompt="Please validate you account before connecting a bank account."
            confirmText="Validate"
            cancelText={translate('common.cancel')}
        />

When we click on the "Add bank account" button, we should check if the user is validated or not and if not, we will open the modal.

        <MenuItem
            onPress={() => {
                if (!isUserValidated) {
                    setIsValidateModalOpen(true);
                    return;
                }
                onPress();
            }}

Lastly we need to make changes in ContactMethodDetailsPage, the component should use the backTo param to navigate back if it is present .
Note, the translation, button colors and other minor details can be discussed in the PR.

What alternative solutions did you explore? (Optional)

https://github.com/user-attachments/assets/96b61cbf-6e08-4f2c-bfa0-ed85b7650737

Krishna2323 commented 3 weeks ago

Proposal Updated

Added result video and a note.

shawnborton commented 3 weeks ago

@Krishna2323 rather than throwing an error modal, I was thinking we could show the validate UI in the RHP.

shawnborton commented 3 weeks ago

Actually looks like @cretadn22 already proposed that.

Krishna2323 commented 3 weeks ago

@shawnborton, I thought it would be odd to directly push users to the validate page without providing any information. That's why I suggested showing a modal.

shawnborton commented 3 weeks ago

I think it would be simpler if we just added some text in that RHP view that explains why we need to validate for that particular feature.

cretadn22 commented 3 weeks ago

I think it would be simpler if we just added some text in that RHP view that explains why we need to validate for that particular feature.

We can present the user with an announcement offering two options: either go to SETTINGS_CONTACT_METHOD_DETAILS and sign in again, or provide additional explanation. This approach could also be used in other places

shawnborton commented 3 weeks ago

I think we should do something simple like this, where we basically have a little line that says "This feature requires you to validate your account" and then we show the validate form, something like this: CleanShot 2024-08-27 at 12 55 19@2x

cc @Expensify/design in case you have any additional thoughts.

daledah commented 3 weeks ago

Updated proposal

base on this comment

daledah commented 3 weeks ago

Actually looks like @cretadn22 already proposed that.

@shawnborton my proposal is first and @cretadn22's proposal is same as mine

cretadn22 commented 3 weeks ago

Updated proposal

dannymcclain commented 3 weeks ago

I like what you're proposing Shawn. Keeps you in the flow and doesn't derail you, but still gets you to validate.

trjExpensify commented 3 weeks ago

Samesies! ^^

dubielzyk-expensify commented 3 weeks ago

Dig it. Would it be a bit clearer if we have the header say "Validate your account" instead of the email address?

daledah commented 3 weeks ago

updated proposal

shawnborton commented 3 weeks ago

@dubielzyk-expensify yeah totally agree with that, I just grabbed a quick example from the contact methods flow since that is the only spot (I think?) that has this kind of in-product account validation thus far.

melvin-bot[bot] commented 2 weeks ago

@miljakljajic Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 2 weeks ago

@miljakljajic Still overdue 6 days?! Let's take care of this!

melvin-bot[bot] commented 2 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~019c85c6a0fbfa60c9

melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @jjcoffee (External)

miljakljajic commented 2 weeks ago

@shawnborton whose proposal are we going with here? I'll get them assigned

shawnborton commented 2 weeks ago

I think that would be @jjcoffee's decision right?

jjcoffee commented 2 weeks ago

Hmm I'm not sure if I agree with navigating to the contact details page here. We wouldn't normally reuse a page for a different purpose, but rather the components in the page (ValidateCodeForm in this case). I think navigating to a completely different route also will cause issues; if you refresh after tapping Add bank account, for example, you'd lose your place (the central pane would switch in the background to the profile screen).

I think I'd be more in favour of adding a ValidateCodeForm either in a new page or maybe as the first step of the add bank account flow (only if you aren't validated of course!).

daledah commented 2 weeks ago

@jjcoffee I don't think it's a big issue. In our project, we also have instances where we use shared modals. For example, on the I'm a teacher page, we reuse the Contact methods from the profile.

https://github.com/Expensify/App/blob/7aae84275eb02635a573acc6433c688651131079/src/pages/TeachersUnite/ImTeacherUpdateEmailPage.tsx#L41

jjcoffee commented 1 week ago

@daledah In that case it makes sense since the purpose is the same (adding an email address) and so we're directing the user to the page appropriate to that purpose.

Once we start talking about customising the contact details page to add text and a header that's specific to this flow, I think it doesn't really make sense to reuse the page anymore.

daledah commented 1 week ago

@jjcoffee agree with you, updated proposal

jjcoffee commented 1 week ago

Thanks @daledah, happy to go with your updated proposal!

:ribbon::eyes::ribbon: C+ reviewed