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

m-natarajan · 2024-08-27T01:08:42Z

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?

Screenshots/Videos

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: @strepanier03

melvin-bot · 2024-08-27T01:08:51Z

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 · 2024-08-27T01:20:52Z

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

App/src/pages/settings/Wallet/PaymentMethodList.tsx

Line 324 in d4d1c0c

disabled={!isUserValidated}

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 · 2024-08-27T01:51:07Z

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

App/src/pages/settings/Wallet/PaymentMethodList.tsx

Line 324 in d4d1c0c

disabled={!isUserValidated}

App/src/pages/EnablePayments/AddBankAccount/SetupMethod.tsx

Line 55 in d4d1c0c

isDisabled={!!isPlaidDisabled || !user?.validated}

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 #48041 (comment). 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:

Screen98.mov

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 · 2024-08-27T01:57:15Z

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.

App/src/pages/settings/Wallet/PaymentMethodList.tsx

Line 324 in e244e51

disabled={!isUserValidated}
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)

enable_bank_account_btn.mp4

Krishna2323 · 2024-08-27T02:00:23Z

Proposal Updated

Added result video and a note.

shawnborton · 2024-08-27T12:29:39Z

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

shawnborton · 2024-08-27T12:30:05Z

Actually looks like @cretadn22 already proposed that.

Krishna2323 · 2024-08-27T14:24:48Z

@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 · 2024-08-27T14:39:56Z

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 · 2024-08-27T15:34:38Z

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 · 2024-08-27T16:56:06Z

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:

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

daledah · 2024-08-27T17:12:47Z

Updated proposal

base on this comment

daledah · 2024-08-27T17:14:19Z

Actually looks like @cretadn22 already proposed that.

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

cretadn22 · 2024-08-27T17:15:59Z

Updated proposal

dannymcclain · 2024-08-27T17:18:08Z

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

trjExpensify · 2024-08-27T18:34:54Z

Samesies! ^^

dubielzyk-expensify · 2024-08-28T02:01:19Z

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

daledah · 2024-08-28T03:34:50Z

updated proposal

shawnborton · 2024-08-28T11:22:50Z

@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 · 2024-08-30T18:19:28Z

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

melvin-bot · 2024-09-03T18:24:24Z

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

melvin-bot · 2024-09-04T09:18:30Z

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

melvin-bot · 2024-09-20T13:29:56Z

Triggered auto assignment to @strepanier03 (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.

miljakljajic · 2024-09-20T13:30:21Z

Heading out on my parental leave - reassigning to another BZ team member: thank you @strepanier03!

melvin-bot · 2024-10-03T16:14:35Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot · 2024-10-03T16:14:38Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot · 2024-10-04T09:59:55Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot · 2024-10-04T20:58:43Z

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

melvin-bot · 2024-10-04T20:58:49Z

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

fix: requires validate when opening bank account #49230

If no regressions arise, payment will be issued on 2024-10-11. 🎊

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

@jjcoffee requires payment automatic offer (Reviewer)
@daledah requires payment (Needs manual offer from BZ)

melvin-bot · 2024-10-04T20:58:50Z

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:

[@jjcoffee] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
[@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

ikevin127 · 2024-10-04T22:02:12Z

❓ Question regarding the new verification screen in RHP (ROUTES.SETTINGS_WALLET_VERIFY_ACCOUNT):
Is it expected behaviour to be logged out once you introduced the magic code ?

ℹ️ For context, I'm asking because we held issue #49523 until the PR for this one was merged, I observed this behaviour while testing our PR and wanted to confirm that being logged out is the correct behaviour.

cc @arosiclair @jjcoffee @daledah

daledah · 2024-10-05T07:28:45Z

Is it expected behaviour to be logged out once you introduced the magic code ?

@ikevin127 I'm not quite understand what you're saying, does it mean the user is logged out after entering the magic code, instead of navigating back to the supposed route?

ikevin127 · 2024-10-05T07:51:56Z

Yes, the user is logged out instead of opening the Add bank account page, here's a video:

Screen.Recording.2024-10-05.at.00.41.53.mov

I'm trying to figure out if this is a regression of this issue's implementation or we didn't use your implementation correctly and the regression is on our issue's side.

From what I can see, your PR is using the route with redirect to add BA:

Navigation.navigate(ROUTES.SETTINGS_WALLET_VERIFY_ACCOUNT.getRoute(ROUTES.SETTINGS_ADD_BANK_ACCOUNT));

whereas in our PR, the author did not pass anything:

Navigation.navigate(ROUTES.SETTINGS_WALLET_VERIFY_ACCOUNT.getRoute());

which might be the cause of the logout issue 🤔

I tested using:

Navigation.navigate(ROUTES.SETTINGS_WALLET_VERIFY_ACCOUNT.getRoute(ROUTES.SETTINGS_ADD_BANK_ACCOUNT));

like you used, but it still logs me out:

Screen.Recording.2024-10-05.at.00.56.31.mov

Any ideas why the route you implemented would behave like this in our case ? 🤔

daledah · 2024-10-05T08:26:09Z

@ikevin127 Looks like when validating account, BE send Onyx data that changes session's authToken:

And then FE calls getMissingOnyxMessages, but still using old authToken:

I'm not sure if this bug is either from this issue or #49523 though.

ikevin127 · 2024-10-05T08:51:54Z

@daledah Thanks for looking into the root cause! 🚀

Are you experiencing the logout upon validation as well in this issue's flow testing ?

This is definitely weird, I mean it makes sense that when you validate an unvalidated account the authToken would change, but because a call is made right after with the outdated key -> the user is logged out (expected if you ask me).

Given this behaviour I'm leaning towards the scenario where none of the issues (this or ours) is the direct cause of the logout, but rather because the logic we implemented, we revealed that the FE logic wasn't designed to account for the edge case of transitioning between unvalidated to validated account and changing authToken.

In this case I'm not sure who should handle a fix for this edge case in a follow-up 🤔

Tagging the assigned CME's of both issues for visibility ⏬

cc @arosiclair from this issue
cc @nkuoch from issue #49523

arosiclair · 2024-10-07T15:12:09Z

Thanks for looking into the issue @daledah @ikevin127. Is this reproducible in staging or main with any other flow at the moment? If so, we can report the bug and start investigating in another issue.

arosiclair · 2024-10-07T15:32:25Z

Nevermind, looks like it is here. It sounded like we did not merge the PR for #49523 yet, but it looks like we did? I think we probably need to revert it and find a fix for the root problem.

strepanier03 · 2024-10-07T17:41:10Z

Read through the above and it sounds like I can still pay this as normal on 10-11 correct?

melvin-bot · 2024-10-07T17:48:17Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

arosiclair · 2024-10-07T19:54:25Z

Read through the above and it sounds like I can still pay this as normal on 10-11 correct?

Yeah that's correct.

strepanier03 · 2024-10-11T17:40:30Z

Payment Summary

$250 - @jjcoffee requires payment paid via Upwork
$250 - @daledah via New Expensify request, feel free to put in that request now.

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 27, 2024

melvin-bot bot assigned miljakljajic Aug 27, 2024

melvin-bot bot added the Overdue label Aug 29, 2024

miljakljajic added the External Added to denote the issue can be worked on by a contributor label Sep 4, 2024

melvin-bot bot changed the title ~~Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts~~ [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts Sep 4, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 4, 2024

melvin-bot bot assigned jjcoffee Sep 4, 2024

miljakljajic added the NewFeature Something to build that is a new item. label Sep 20, 2024

melvin-bot bot assigned strepanier03 Sep 20, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 4, 2024

melvin-bot bot changed the title ~~[$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts~~ [HOLD for payment 2024-10-11] [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts Oct 4, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Oct 4, 2024

strepanier03 changed the title ~~[HOLD for payment 2024-10-11] [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts~~ [Payment 2024-10-11] [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts Oct 7, 2024

ikevin127 mentioned this issue Oct 7, 2024

[$250] Pay someone - User is signed out after verifying account during pay someone flow #50284

Open

6 tasks

strepanier03 closed this as completed Oct 11, 2024

strepanier03 changed the title ~~[Payment 2024-10-11] [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts~~ [PAID] [$250] Wallet empty state has disabled "Add bank account" button for new, unvalidated accounts Oct 11, 2024

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

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

Comments

m-natarajan commented Aug 27, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Aug 27, 2024

daledah commented Aug 27, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

cretadn22 commented Aug 27, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Krishna2323 commented Aug 27, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Krishna2323 commented Aug 27, 2024

Proposal Updated

shawnborton commented Aug 27, 2024

shawnborton commented Aug 27, 2024

Krishna2323 commented Aug 27, 2024

shawnborton commented Aug 27, 2024

cretadn22 commented Aug 27, 2024

shawnborton commented Aug 27, 2024

daledah commented Aug 27, 2024

daledah commented Aug 27, 2024

cretadn22 commented Aug 27, 2024

dannymcclain commented Aug 27, 2024

trjExpensify commented Aug 27, 2024

dubielzyk-expensify commented Aug 28, 2024 • edited Loading

daledah commented Aug 28, 2024

shawnborton commented Aug 28, 2024

melvin-bot bot commented Aug 30, 2024

melvin-bot bot commented Sep 3, 2024

melvin-bot bot commented Sep 4, 2024

melvin-bot bot commented Sep 20, 2024

miljakljajic commented Sep 20, 2024

melvin-bot bot commented Oct 3, 2024

melvin-bot bot commented Oct 3, 2024

melvin-bot bot commented Oct 4, 2024

melvin-bot bot commented Oct 4, 2024

melvin-bot bot commented Oct 4, 2024

melvin-bot bot commented Oct 4, 2024

ikevin127 commented Oct 4, 2024

daledah commented Oct 5, 2024

ikevin127 commented Oct 5, 2024 • edited Loading

daledah commented Oct 5, 2024

ikevin127 commented Oct 5, 2024

arosiclair commented Oct 7, 2024

arosiclair commented Oct 7, 2024

strepanier03 commented Oct 7, 2024

melvin-bot bot commented Oct 7, 2024

arosiclair commented Oct 7, 2024

strepanier03 commented Oct 11, 2024

Payment Summary

m-natarajan commented Aug 27, 2024 •

edited by melvin-bot bot

Loading

daledah commented Aug 27, 2024 •

edited

Loading

cretadn22 commented Aug 27, 2024 •

edited

Loading

Krishna2323 commented Aug 27, 2024 •

edited

Loading

dubielzyk-expensify commented Aug 28, 2024 •

edited

Loading

ikevin127 commented Oct 5, 2024 •

edited

Loading