Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.53k stars 2.88k forks source link

[$250] [HOLD for payment 2024-09-23] Bank account - Bank account modal prompts user to verify account after being done validating #49215

Closed IuliiaHerets closed 1 month ago

IuliiaHerets commented 1 month 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.35-0 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): nathanmulugetatesting+1484@gmail.com Issue reported by: Applause Internal Team

Action Performed:

  1. Navigate to staging.new.expensify.com and sign up with a new unvalidated Gmail account
  2. Create a workspace
  3. Enable workflows
  4. Click on connect to bank account
  5. Update currency to usd if already not
  6. Click on "Verify your account here" link and validate your account

Expected Result:

After validating account bank account setup continues

Actual Result:

After validating account bank account setup continues

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/2d15efbe-babb-4e66-adcd-88730c0a23bc

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021839023851099596990
  • Upwork Job ID: 1839023851099596990
  • Last Price Increase: 2024-09-25
melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

:wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.
melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @tylerkaraszewski (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

ishpaul777 commented 1 month ago

offending PR https://github.com/Expensify/App/pull/49130, cc @CyberAndrii @s77rt @francoisl @

Nodebrute commented 1 month ago

Proposal

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

Bank account modal prompts user to verify account after being done validating

What is the root cause of that problem?

Here we set user validated to true but then in this page we check for account?.Validated https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/libs/actions/User.ts#L623-L627

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

We can change User to ACCOUNT https://github.com/Expensify/App/blob/0c618acc8b5a0b77eb27dfa049a18e3135c51c52/src/libs/actions/User.ts#L623-L627

What alternative solutions did you explore? (Optional)

Or we can keep the user and also add a new entry here validated: true,

CyberAndrii commented 1 month ago

Damn, I accidentally reverted that line while I was reverting other changes :facepalm:

But I agree we should include both of them to be safe

{
    onyxMethod: Onyx.METHOD.MERGE, 
    key: ONYXKEYS.ACCOUNT, 
    value: {
        isLoading: false,
        validated: true, 
    },
},
{
    onyxMethod: Onyx.METHOD.MERGE, 
    key: ONYXKEYS.USER, 
    value: {
        validated: true, 
    },
},
s77rt commented 1 month ago

@CyberAndrii This happens only optimistically right? IOW if we refresh the page the error would disappear? If so, I agree with the suggested solution, please raise a PR asap

CyberAndrii commented 1 month ago

PR is ready.

@s77rt after reloading (without the changes from the PR) the error don't disappear. However, I don’t think we need a BE fix here

s77rt commented 1 month ago

@CyberAndrii If you logout and login, do you still see the error?

This bug could be related to the BE if that flow is still using user.validated internally. If that's the case we should revert the PR (to unblock the deploy), make the necessary changed in BE and then re-submit the PR (basically same as the plan)

CyberAndrii commented 1 month ago

No error, BE sets the value correctly

image

s77rt commented 1 month ago

Unfortunately I'm unable to reproduce the bug (asked in Slack). @ishpaul777 If you can reproduce both the initial bug and this new bug, do you want to take that issue over and review/test the follow up PR?

ishpaul777 commented 1 month ago

sure i can try reproducing... 👀

ishpaul777 commented 1 month ago

let discusss on Slack https://expensify.slack.com/archives/C01GTK53T8Q/p1726358698700859

grgia commented 1 month ago

We discussed and decided to move forward with the fix proposed in https://github.com/Expensify/App/pull/49221

ishpaul777 commented 1 month ago

please assign me so this is on my K2 🙏

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

If no regressions arise, payment will be issued on 2024-09-23. :confetti_ball:

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

melvin-bot[bot] commented 1 month ago

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

garrettmknight commented 1 month ago

Payment Summary:

melvin-bot[bot] commented 1 month ago

Payment Summary

[Upwork Job]()

BugZero Checklist (@garrettmknight)

ishpaul777 commented 1 month ago

No regression test and checklist requires, since this was caught in normal deploy QA process and we already know offending PR https://github.com/Expensify/App/pull/49221

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

Current assignee @ishpaul777 is eligible for the External assigner, not assigning anyone new.

garrettmknight commented 1 month ago

@ishpaul777 offer out to you

ishpaul777 commented 1 month ago

Thank You. Offer Accepted !

garrettmknight commented 1 month ago

Paid, closing!