Skip to content
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.

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

[HOLD for Payment 2024-10-24] [$250] Bank account - Not here page shows up when clicking add bank account #48506

Open
2 of 6 tasks
lanitochka17 opened this issue Sep 3, 2024 · 44 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 3, 2024

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.28-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: N/A
Email or phone of affected tester (no customers): testpayment935@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Create a workspace
  3. Add a bank account
  4. On another tab sign in with a new account (This is going to be invited as an employee)
  5. As the admin invite the user signed in on step 4
  6. As the employee submit an expense
  7. Admin pays the expense with expensify
  8. As the employee navigate to the expense report page and click on Add bank account

Expected Result:

User gets navigated to validate their account first

Actual Result:

Hmm not here page shows up

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6592163_1725393801912.Screen_Recording_20240903_224959_Chrome.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021834204861349531229
  • Upwork Job ID: 1834204861349531229
  • Last Price Increase: 2024-09-12
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 3, 2024
Copy link

melvin-bot bot commented Sep 3, 2024

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

@lanitochka17
Copy link
Author

@kevinksullivan FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@nyomanjyotisa
Copy link
Contributor

Proposal

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

Not here page shows up when clicking add bank account

What is the root cause of that problem?

Add bank account page is not accessible for non verified user and we don't disable the "Add bank account" button if the user is not validated here, like we did in the wallet page

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

Disable the "Add bank account" button if the user is not validated here

const [isUserValidated] = useOnyx(ONYXKEYS.USER, {selector: (user) => !!user?.validated});
...
<Button
    success
    style={[styles.w100, styles.requestPreviewBox]}
    text={translate('bankAccount.addBankAccount')}
    onPress={() => BankAccounts.openPersonalBankAccountSetupView(Navigation.getTopmostReportId() ?? linkedReport?.reportID)}
    pressOnEnter
    large
    isDisabled={!isUserValidated}
/>

What alternative solutions did you explore? (Optional)

When non validated account click "Add bank account" button, display a modal that inform the user is not validated yet so they cant add bank account, with 2 button: "Validate account" & "Cancel"

Once "Validate account" clicked navigate to default contact method page, to verify

Once verification success navigate back to that expense report page

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 4, 2024

Edited by proposal-police: This proposal was edited at 2024-09-11 08:56:21 UTC.

Proposal

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

Hmm not here page shows up. We should open validate code page first before executing the add bank account flow.

What is the root cause of that problem?

We show not found page when user is not validated:

<FullPageNotFoundView shouldShow={!isUserValidated}>

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

Old solution

We should not disable Add bank account button in the message because that is a CTA button, without it, user wouldn't know what to do next to get reimbursed.

We should follow the behavior from policy's add bank account page, when user is not validated, we would show a warning message requesting them to validate their accounts first:

Screenshot 2024-09-04 at 16 24 01

To do this, in AddPersonalBankAccountPage:

  1. Remove the FullPageNotFoundView logic
  2. Show ValidateAccountMessage component when isUserValidated is not satisfied:
{isUserValidated ? (
    {shouldShowSuccess ? <ConfirmationPage /> : <FormProvider />}
) : (
    <ValidateAccountMessage backTo={ROUTES.SETTINGS_ADD_BANK_ACCOUNT}/>
)}

We also did this in 2FA page:

Screenshot 2024-09-11 at 15 55 37
  1. We need to reuse the ValidateCodeActionModal just like we did in NewContactMethodPage:

const handleSubmitForm = useCallback((validateCode: string) => {
  User.validateSecondaryLogin(loginList, account?.primaryLogin ?? '', validateCode);
}, [loginList, account]);


<ValidateCodeActionModal
    handleSubmitForm={handleSubmitForm}
    isVisible={isValidateCodeActionModalVisible}
/>
  1. When the page mounts, check if user is validated or not to display the modal:
useEffect(() => {
        if (!isUserValidated && !isValidateCodeActionModalVisible) {
            setIsValidateCodeActionModalVisible(true);
        }
}, []);
  1. If isUserValidated, we show the page main content:

{shouldShowSuccess ? (

  1. There are some polishes like validate error, pending fields and navigation when closing/dismissing the modal but we can handle those in the PR

@melvin-bot melvin-bot bot added the Overdue label Sep 6, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

@kevinksullivan Eep! 4 days overdue now. Issues have feelings too...

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 11, 2024

@kevinksullivan Please proceed this one.

Copy link

melvin-bot bot commented Sep 11, 2024

@kevinksullivan 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 12, 2024

@kevinksullivan Can you reassign BZ here since you're heading OOO?

@kevinksullivan
Copy link
Contributor

I'd put this in collect since it's the mainline reimbursement flow. Also looping in another BZ as I am going OOO

@melvin-bot melvin-bot bot removed the Overdue label Sep 12, 2024
@kevinksullivan kevinksullivan removed the Bug Something is broken. Auto assigns a BugZero manager. label Sep 12, 2024
@kevinksullivan kevinksullivan removed their assignment Sep 12, 2024
@kevinksullivan kevinksullivan added Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor labels Sep 12, 2024
@melvin-bot melvin-bot bot changed the title Bank account - Not here page shows up when clicking add bank account [$250] Bank account - Not here page shows up when clicking add bank account Sep 12, 2024
Copy link

melvin-bot bot commented Sep 12, 2024

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

Copy link

melvin-bot bot commented Sep 12, 2024

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

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

melvin-bot bot commented Sep 12, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 12, 2024
@allroundexperts
Copy link
Contributor

Thanks for the proposals everyone. I think @mkzie2's proposal is better, since disabling the button straight away would just cause confusion to the user. Let's also confirm with the design team if this is something we need to do here.

@Expensify/design can you please weigh in?

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Sep 24, 2024
@mkzie2
Copy link
Contributor

mkzie2 commented Sep 24, 2024

@allroundexperts The PR is ready.

@mkzie2
Copy link
Contributor

mkzie2 commented Sep 26, 2024

@greg-schroeder @allroundexperts Let's hold this for #49230.

@greg-schroeder greg-schroeder changed the title [$250] Bank account - Not here page shows up when clicking add bank account [HOLD for #49230] [$250] Bank account - Not here page shows up when clicking add bank account Sep 26, 2024
@greg-schroeder
Copy link
Contributor

Held

@greg-schroeder
Copy link
Contributor

#49230 was merged. Awaiting deploy to staging -> prod

@greg-schroeder
Copy link
Contributor

greg-schroeder commented Oct 8, 2024

Taking this off hold now @mkzie2 @allroundexperts

I think Sibtain is up next with a review requested on the PR, yes?

@greg-schroeder greg-schroeder changed the title [HOLD for #49230] [$250] Bank account - Not here page shows up when clicking add bank account [$250] Bank account - Not here page shows up when clicking add bank account Oct 8, 2024
@greg-schroeder greg-schroeder added Daily KSv2 and removed Weekly KSv2 labels Oct 8, 2024
@mkzie2
Copy link
Contributor

mkzie2 commented Oct 9, 2024

Post an update in PR, we can continue reviewing.

@greg-schroeder
Copy link
Contributor

I believe @allroundexperts is next up on the review

Copy link

melvin-bot bot commented Oct 16, 2024

@MariaHCD, @greg-schroeder, @allroundexperts, @mkzie2 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@greg-schroeder
Copy link
Contributor

Same as above

Copy link

melvin-bot bot commented Oct 18, 2024

@MariaHCD, @greg-schroeder, @allroundexperts, @mkzie2 Eep! 4 days overdue now. Issues have feelings too...

@allroundexperts
Copy link
Contributor

Seems like PR has no diff at all. Waiting for @mkzie2's comment.

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 21, 2024

@greg-schroeder @allroundexperts Validate code step was added to adding bank account flow in #50228. We can proceed payments and close this.

@greg-schroeder
Copy link
Contributor

Okay so just to be clear - the linked PR above is irrelevant and will be closed? I'm confused

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 21, 2024

In short, this issue was already fixed in #50228 so we can close the current PR #49366.

@greg-schroeder
Copy link
Contributor

Okay, reviewing again

@greg-schroeder
Copy link
Contributor

Payment summary:

Contributor: @mkzie2 - $250 - Upwork
C+: @allroundexperts - $250 - Manual request via ND

@greg-schroeder
Copy link
Contributor

@mkzie2 Do you mind applying to the new job I created so I can pay you, thanks! https://www.upwork.com/jobs/~021849539020945682815

@greg-schroeder greg-schroeder added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Oct 24, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 24, 2024
@greg-schroeder greg-schroeder changed the title [$250] Bank account - Not here page shows up when clicking add bank account [HOLD for Payment 2024-10-24] [$250] Bank account - Not here page shows up when clicking add bank account Oct 24, 2024
@melvin-bot melvin-bot bot removed the Overdue label Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Polish
Development

No branches or pull requests

10 participants