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-04-30] [$500] BA - Disconnected bank account is still displayed on bank account preview on Workflows page #39439

Closed
2 of 6 tasks
kbecciv opened this issue Apr 2, 2024 · 33 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 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Apr 2, 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: 1.4.59-0
Reproducible in staging?: y
Reproducible in production?: unable to check
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4469599
Issue reported by: Applause - Internal Team

Action Performed:

Pre-requisite: user must be logged in, have created a collect workspace and have connected a bank account.

  1. Go to Workspace > Workflows > Bank account.
  2. Tap on "Disconnect bank account" and confirm.
  3. Tap on the back arrow.
  4. Verify the bank account is still displayed on the bank account preview.
  5. Go to Settings > Wallet.
  6. Delete the bank account.
  7. Go to Workspace > Workflows.
  8. Verify the bank account is still displayed on the bank account preview.

Expected Result:

The bank account should not be displayed on the preview after the account has been disconnected.

Actual Result:

The bank account is still displayed on the preview after the account has been disconnected.

Workaround:

n/a

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

Bug6435967_1712085957572.Aokm2842_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~018935ae4d1df3ea3c
  • Upwork Job ID: 1775552022142980096
  • Last Price Increase: 2024-04-10
  • Automatic offers:
    • rayane-djouah | Reviewer | 0
Issue OwnerCurrent Issue Owner: @rayane-djouah
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Apr 2, 2024
Copy link

melvin-bot bot commented Apr 2, 2024

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

Copy link
Contributor

github-actions bot commented Apr 2, 2024

👋 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.

@kbecciv
Copy link
Author

kbecciv commented Apr 2, 2024

Linking closed and fixed KI #38200

@hayata-suenaga
Copy link
Contributor

@kbecciv, am I correct to understand that the fix for this issue has been deployed to production in the PR you linked and the issue is no longer reproducible both on staging or production?

The linked PR appears to have been deployed to production about 11 hours ago. I don't know why this issue was detected on staging about 2 hours ago.

@hayata-suenaga
Copy link
Contributor

@hayata-suenaga hayata-suenaga removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Apr 2, 2024
@kavimuru
Copy link

kavimuru commented Apr 3, 2024

@hayata-suenaga Tester reproduced this bug.

bandicam.2024-04-02.20-09-26-330.mp4

@hayata-suenaga
Copy link
Contributor

I'll reopen this issue as the QA team was able to reproduce this.

However, we cannot confirm if this issue exists in production or not as we don't have a test bank account that we can use on production. So, I'll treat this as a normal bug issue.

@hayata-suenaga hayata-suenaga reopened this Apr 3, 2024
@hayata-suenaga hayata-suenaga added Daily KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors Bug Something is broken. Auto assigns a BugZero manager. labels Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@hayata-suenaga hayata-suenaga added the External Added to denote the issue can be worked on by a contributor label Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

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

@melvin-bot melvin-bot bot changed the title BA - Disconnected bank account is still displayed on bank account preview on Workflows page [$500] BA - Disconnected bank account is still displayed on bank account preview on Workflows page Apr 3, 2024
Copy link

melvin-bot bot commented Apr 3, 2024

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

@allgandalf
Copy link
Contributor

Proposal

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

Disconnected bank account is still displayed on bank account preview on Workflows page

What is the root cause of that problem?

When we disconnect bank account and get on the workflow page, we do not fetch the latest data as we only call it once when we load the page at the start:

useEffect(() => {
fetchData();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

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

Add a dependency of policy so that it will fetch the latest details everytime there is a change in policy settings

What alternative solutions did you explore? (Optional)

N/A

@nkdengineer
Copy link
Contributor

nkdengineer commented Apr 4, 2024

Proposal

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

The bank account is still displayed on the preview after the account has been disconnected.

What is the root cause of that problem?

The workspace workflows page relies on the policy?.achAccount here do decide whether to show the bank name in the bank account preview part.

However, when we disconnect the bank account, we're not setting achAccount to null in optimistic data here.

So the bank account preview will still show.

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

Set achAccount of the policy to null in optimistic data here, and potentially restore it in failureData.

Sample code:

{
    onyxMethod: Onyx.METHOD.MERGE,
    key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
    value: {
        achAccount: null,
    },
}

What alternative solutions did you explore? (Optional)

If we only want the bank account preview to disappear after the API request to disconnect is successful, we can set achAccount of the policy to null in successData.

We might need to set achAccount to null when deleting the bank account (like in Wallet) too.

We can consider optimistically update the achAccount when we're at the final step of connecting the bank account too.

Copy link

melvin-bot bot commented Apr 10, 2024

Current assignee @hayata-suenaga is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

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

melvin-bot bot commented Apr 10, 2024

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Apr 10, 2024

📣 @nkdengineer You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added the Overdue label Apr 12, 2024
@rayane-djouah
Copy link
Contributor

Bump @nkdengineer

@melvin-bot melvin-bot bot removed the Overdue label Apr 12, 2024
@nkdengineer
Copy link
Contributor

Will open PR today.

@nkdengineer
Copy link
Contributor

@rayane-djouah The PR is here.

@rayane-djouah
Copy link
Contributor

Deployed to production #40226 (comment)

@rayane-djouah
Copy link
Contributor

@sakluger, please remove the Reviewing label and bump to Daily as payment is due

@hayata-suenaga hayata-suenaga added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Weekly KSv2 labels Apr 30, 2024
@sakluger
Copy link
Contributor

@sakluger sakluger changed the title [$500] BA - Disconnected bank account is still displayed on bank account preview on Workflows page [HOLD for Payment 2024-04-30] [$500] BA - Disconnected bank account is still displayed on bank account preview on Workflows page Apr 30, 2024
@sakluger
Copy link
Contributor

Summarizing payment on this issue:

Contributor: @nkdengineer $500, sent offer via Upwork (https://www.upwork.com/nx/wm/offer/102102495/)
Contributor+: @rayane-djouah $500, paid via Upwork

@rayane-djouah please complete the BZ checklist.

@sakluger
Copy link
Contributor

sakluger commented May 2, 2024

@nkdengineer please let me know once you've accepted the offer. @rayane-djouah bump on the BZ checklist.

@sakluger
Copy link
Contributor

sakluger commented May 2, 2024

I completed payment to @nkdengineer. Now just waiting on the BZ checklist before we can close the issue.

@rayane-djouah
Copy link
Contributor

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:

Regression test proposal
Pre-requisite: The user should be logged in, have created a collect workspace and have connected a bank account.

  • Go to Workspace > Workflows > Bank account.

  • Tap on "Disconnect bank account" and confirm.

  • Tap on the back arrow.

  • Verify that the bank account is not displayed on the preview after the account has been disconnected.

  • Do we agree 👍 or 👎

@sakluger
Copy link
Contributor

sakluger commented May 3, 2024

Thanks!

@sakluger sakluger closed this as completed May 3, 2024
@github-project-automation github-project-automation bot moved this from Polish to Done in [#whatsnext] #wave-collect May 3, 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 Engineering External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Archived in project
Development

No branches or pull requests

8 participants