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] Workspace - Long loading and errors after a new non USD WS offline and adding a bank account #41431

Closed
1 of 6 tasks
lanitochka17 opened this issue May 1, 2024 · 55 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented May 1, 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.69.0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4529975&group_by=cases:section_id&group_id=283225&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

Precondition: Your default workspace currency shouldn't be USD

  1. Log in with a new expensifail account
  2. Go offline
  3. Create a workspace
  4. Enable Workflows in the WS settings
  5. Go online
  6. Start adding a bank account
  7. Click on "Update to USD"

Expected Result:

The loading shouldn't take long and there shouldn't be any errors while adding a bank account

Actual Result:

After clicking on "Connect bank account", the loading takes up to 15 seconds or sometimes it's infinite. "Hmm it's not here" error appears briefly when it loads and the user clicks somewhere to close the "Connect bank account" modal.

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

Bug6468171_1714582139396.bandicam_2024-05-01_18-29-09-073.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0177c40c7be0407379
  • Upwork Job ID: 1788976368546947072
  • Last Price Increase: 2024-07-05
Issue OwnerCurrent Issue Owner: @hoangzinh
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label May 1, 2024
Copy link

melvin-bot bot commented May 1, 2024

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

Copy link
Contributor

github-actions bot commented May 1, 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.

@lanitochka17
Copy link
Author

@AndrewGable 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

@lanitochka17
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@francoisl
Copy link
Contributor

I'm also seeing a delay on production, I think it's just because there are a bunch of sequential API requests to finish before the view can be shown (also there's some crazy flickering, looks like it's due to rerenders; but that's unrelated)

Screen.Recording.2024-05-01.at.2.57.20.PM.mov

@AndrewGable AndrewGable added Bug Something is broken. Auto assigns a BugZero manager. and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels May 1, 2024
Copy link

melvin-bot bot commented May 1, 2024

Triggered auto assignment to @JmillsExpensify (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 Daily KSv2 label May 1, 2024
@AndrewGable
Copy link
Contributor

Agree with @francoisl if it's on production let's look at it like a normal bug.

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

melvin-bot bot commented May 7, 2024

@JmillsExpensify, @AndrewGable Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented May 7, 2024

@JmillsExpensify, @AndrewGable Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented May 9, 2024

@JmillsExpensify, @AndrewGable Still overdue 6 days?! Let's take care of this!

@AndrewGable AndrewGable added the External Added to denote the issue can be worked on by a contributor label May 10, 2024
@melvin-bot melvin-bot bot changed the title Workspace - Long loading and errors after a new non USD WS offline and adding a bank account [$250] Workspace - Long loading and errors after a new non USD WS offline and adding a bank account May 10, 2024
Copy link

melvin-bot bot commented May 10, 2024

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

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

melvin-bot bot commented May 10, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label May 10, 2024
@nkdengineer
Copy link
Contributor

nkdengineer commented May 10, 2024

Proposal

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

After clicking on "Connect bank account", the loading takes up to 15 seconds or sometimes it's infinite. "Hmm it's not here" error appears briefly when it loads and the user clicks somewhere to close the "Connect bank account" modal.

What is the root cause of that problem?

When CreateWorkspace API is called, there are some gaps onyx updates between the client and the given server updates and then the queue is pause before we call EnablePolicyWorkflows API.

flushOnyxUpdatesQueue();

After we handle the gap, the queue is unpause and we call flushOnyxUpdatesQueue here without checking the persisted requests is empty or not and then areWorkflowsEnabled is override as false

SequentialQueue.unpause();

flushOnyxUpdatesQueue();

Another problem that makes areWorkflowsEnabled is override is when the queue is pause, the process function returns a resolve promise and then resolveIsReadyPromise is called here which makes openPolicyMoreFeaturesPage is called before EnablePolicyWorkflows API is called

resolveIsReadyPromise?.();

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

We should check the persisted requests list is empty or not before we resolve IsReady promise here

if (PersistedRequests.getAll().length === 0) {
    resolveIsReadyPromise?.();
}

resolveIsReadyPromise?.();

and flushOnyxUpdatesQueue here

if (numberOfPersistedRequests === 0) {
    flushOnyxUpdatesQueue();
}

flushOnyxUpdatesQueue();

function isPolicyFeatureEnabled(policy: OnyxEntry<Policy> | EmptyObject, featureName: PolicyFeatureName): boolean {

What alternative solutions did you explore? (Optional)

NA

Copy link

melvin-bot bot commented May 13, 2024

@JmillsExpensify, @AndrewGable, @hoangzinh Whoops! This issue is 2 days overdue. Let's get this updated quick!

@wildan-m
Copy link
Contributor

wildan-m commented Jun 10, 2024

Proposal

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

Workspace - Long loading and errors after a new non USD WS offline and adding a bank account

What is the root cause of that problem?

Policy is not ready to use when we connect a bank account. There are too many fields missing in the optimistic data.

Compare Policy Object. Left (Offline), Right (Online and policy creation completed)

AwesomeScreenshot-text-compare--2024-06-10_4_58_part1
AwesomeScreenshot-text-compare--2024-06-10_4_58_part2

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

Better to wait until workspace created to show bank account page. Change this check to:

    const isLoading = (!!isLoadingApp || !!account?.isLoading || isReimbursementAccountLoading || policy?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT);

Or

    const isLoading = (!!isLoadingApp || !!account?.isLoading || isReimbursementAccountLoading || policy?.pendingAction) && (!plaidCurrentEvent || plaidCurrentEvent === CONST.BANK_ACCOUNT.PLAID.EVENTS_NAME.EXIT);

What alternative solutions did you explore? (Optional)

Add missing required values to optimistic data, but IMO this approach will require a lot of trial errors to find the required values

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Overdue labels Jun 10, 2024
@hoangzinh
Copy link
Contributor

Hi @wildan-m, does your proposal work when network is offline?

@wildan-m
Copy link
Contributor

@hoangzinh I can't reproduce it anymore, seems we can close it

@hoangzinh
Copy link
Contributor

I couldn't reproduce this issue either. @JmillsExpensify can you ask QA to test this issue again before we close it? Thanks

Copy link

melvin-bot bot commented Jun 14, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Jun 14, 2024
@hoangzinh
Copy link
Contributor

Waiting to re-test again.

@melvin-bot melvin-bot bot removed the Overdue label Jun 15, 2024
Copy link

melvin-bot bot commented Jun 19, 2024

@JmillsExpensify, @AndrewGable, @hoangzinh Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Jun 19, 2024
@JmillsExpensify
Copy link

Asked QA to test again.

@kavimuru
Copy link

Tester is still able to reproduce.

bandicam.2024-06-20.07-08-47-013.mp4

Copy link

melvin-bot bot commented Jun 21, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Jun 21, 2024

@JmillsExpensify, @AndrewGable, @hoangzinh Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Jun 25, 2024

@JmillsExpensify, @AndrewGable, @hoangzinh 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@JmillsExpensify
Copy link

QA is able to reproduce so keeping this open.

@AndrewGable AndrewGable added Weekly KSv2 and removed Daily KSv2 labels Jun 26, 2024
@melvin-bot melvin-bot bot removed the Overdue label Jun 26, 2024
@wildan-m
Copy link
Contributor

@hoangzinh Just letting you know that my solution is no longer working.

Copy link

melvin-bot bot commented Jun 28, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

Copy link

melvin-bot bot commented Jul 5, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Jul 5, 2024
@JmillsExpensify
Copy link

Given that QA can no longer reproduce, and others have seen the same, I'm closing this issue.

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. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

9 participants