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

[$250] [CVP] Subpar account validation flow in the bottom up flow when paying with a business bank account #48699

Closed
1 of 6 tasks
trjExpensify opened this issue Sep 6, 2024 · 27 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@trjExpensify
Copy link
Contributor

trjExpensify commented Sep 6, 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:
Reproducible in staging?:
Reproducible in production?:
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: @trjExpensify
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1725290110940169

Action Performed:

Prerequisite: The submitter and payer have a personal currency of USD, in order to see the option to Pay with Expensify

  1. [Submitter] Submit a $50 expense to someone who doesn’t have an Expensify account
  2. [Payer] Click the Pay button in the email received to sign-in unvalidated
  3. [Payer] Click Pay with Expensify > Business Bank Account
  4. [Payer] Click Connect with Plaid

Expected Result:

When the user lands on the Connect bank account page, it should look like this:

image

Note: To compare, simply create workspace > enable workflows > payments > connect bank account which shows the above screen to require validation.

Actual Result:

Connect options are not greyed out, user can continue and then hit a subpar auth error on step one of the VBBA setup flow:

image

image (17)

Workaround:

Validate the account.

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

See above.

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021832008208936334242
  • Upwork Job ID: 1832008208936334242
  • Last Price Increase: 2024-09-06
  • Automatic offers:
    • CyberAndrii | Contributor | 103923785
@trjExpensify trjExpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

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

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Sep 6, 2024
@melvin-bot melvin-bot bot changed the title [CVP] Subpar account validation flow in the bottom up flow when paying with a business bank account [$250] [CVP] Subpar account validation flow in the bottom up flow when paying with a business bank account Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

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

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

melvin-bot bot commented Sep 6, 2024

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

@trjExpensify
Copy link
Contributor Author

CC: @NikkiWines @allroundexperts @dukenv0307 with experience on the validation prompt in the connect bank account flow.

@dukenv0307
Copy link
Contributor

Let me check.

@CyberAndrii
Copy link
Contributor

The SigninUserWithLink request sets user.validated to false. Later OpenApp overrides it to true.

The user.validated property is used to determine whether those buttons should be disabled

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

I think this is a BE issue with the OpenApp request.


On the other hand, there's the account.validated property that works similarly but doesn't have this bug. I'm confused what's the difference between them.

/** Whether the account is validated */
validated?: boolean;

Anyway, unless there's a way to point the email link to dev.new.expensify.com:8082, I don't think external contributors can fix this.

@s77rt
Copy link
Contributor

s77rt commented Sep 7, 2024

@CyberAndrii Thanks for looking into this. From the Expected results:

Note: To compare, simply create workspace > enable workflows > payments > connect bank account which shows the above screen to require validation.

Did you get the restricted view or were you able to proceed normally?

@trjExpensify
Copy link
Contributor Author

@CyberAndrii any movement on that Q from @s77rt? We're keen to get this one resolved ASAP. Thanks!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 9, 2024
@CyberAndrii
Copy link
Contributor

The button is not grayed out and I don't get the error. Are you sure opening the email link doesn't verify the account? It looks like it either does or BE is returning an incorrect value for user.validated as mentioned in my previous comment.

Screen.Recording.2024-09-11.at.13.41.06.mov

@trjExpensify
Copy link
Contributor Author

@CyberAndrii interesting. Can you continue that flow to adding the account by clicking Continue > Choose "Wells Fargo" > U: user_good P: pass_good > follow the prompts to have the modal close and return to Expensify.

@CyberAndrii
Copy link
Contributor

Yup, I see the error now. After re-logging into the account it disappeared and I can continue with the steps.

Screen.Recording.2024-09-11.at.15.05.55.mov

Do you know the difference between these two? Maybe we should use the other one here

/** Whether the account is validated */
validated?: boolean;

/** Is the user account validated? */
validated: boolean;

@trjExpensify
Copy link
Contributor Author

Yup, I see the error now.

Wahoo, I'm not crazy! 😉

Do you know the difference between these two? Maybe we should use the other one here

I don't I'm afraid. @mountiny might, but also @blazejkustra perhaps just following the blame on when these were touched last.

@trjExpensify
Copy link
Contributor Author

Maybe we should use the other one here

Does that fix it when making the change from !user?.validated to !account?.validated in dev?

@trjExpensify
Copy link
Contributor Author

@blazejkustra
Copy link
Contributor

I don't I'm afraid. @mountiny might, but also @blazejkustra perhaps just following the blame on when these were touched last.

Unfortunately I don't have much context, I created these types when migrating the codebase to TS 😢

@CyberAndrii
Copy link
Contributor

Does that fix it when making the change from !user?.validated to !account?.validated in dev?

Yes it does

@s77rt
Copy link
Contributor

s77rt commented Sep 11, 2024

I think the value in the account modal is the source of truth and we should remove user.validated from FE and BE.

🎀 👀 🎀 To check the BE and where it's sending user.validated and change it to account.validated (if necessary)

Copy link

melvin-bot bot commented Sep 11, 2024

Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@s77rt
Copy link
Contributor

s77rt commented Sep 11, 2024

The plan:

  1. BE PR to send account.validated in any place that we are sending user.validated
  2. FE PR to use account.validated
  3. BE PR to remove and stop sending user.validated (Important: This has to be done after all clients got upgraded to the latest App version)

@s77rt
Copy link
Contributor

s77rt commented Sep 11, 2024

@francoisl Please assign @CyberAndrii for the proposed solution

@francoisl
Copy link
Contributor

For now let's do step 2 and use account.validated instead of user.validated in that flow, but I don't think we should completely remove user.validated from the backend - there are a few flows I need to double-check on first.

@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
Copy link

melvin-bot bot commented Sep 12, 2024

📣 @CyberAndrii 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

Copy link

melvin-bot bot commented Sep 14, 2024

⚠️ 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.

@CyberAndrii
Copy link
Contributor

The PR was deployed to production on September 16 so this should be ready for payment

@johncschuster
Copy link
Contributor

Payment Summary:

Contributor: @CyberAndrii paid $250 via Upwork - PAID 🎉
Contributor+: @s77rt due $250 via NewDot

Upwork job here!

@s77rt do we need a regression test step list here? If so, can you provide one?

@s77rt
Copy link
Contributor

s77rt commented Sep 27, 2024

@johncschuster I'm not due payment here. The C+ was @ishpaul777 and he is paid already (#49215). I don't think this requires a regression test based on the fact that we had this similar test 1971031 and it's now deleted. This can be closed.

@johncschuster
Copy link
Contributor

Ah, thank you for the correction, @s77rt! Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Development

No branches or pull requests

7 participants