[HOLD for payment 2023-11-18] [$500] Deleting all workspaces does not remove red dot #29235

m-natarajan · 2023-10-10T21:21:33Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

Version Number: 1.3.80-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696933257327709

Action Performed:

Open the app
Open settings->workspaces->any workspace->bank account->connect manually
Fill in step 1 and 2 details
In step 3, fill in details and add any character in first name eg: 'test%%' (note that we are just doing this to trigger server error, it is not compulsory to trigger error only by this method)
Click on Save & Continue and notice that it will trigger server side error
Exit back to home page
Click on settings->workspaces and delete all workspaces one by one
Observe that even after deleting all workspaces, red dot is displayed on profile and besides workspaces in settings

Expected Result:

App should remove red dot of workspace error if we delete all the workspaces

Actual Result:

App displays red dot of workspace error even after we delete all the workspaces, it removes the red dot on relogin

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android: mWeb Chrome

android.chrome.red.dot.error.not.removed.after.workspace.delete.mp4

iOS: Native

iOS: mWeb Safari

ios.safari.red.dot.not.removed.after.workspace.delete.mov

MacOS: Chrome / Safari

mac.chrome.red.dot.not.dismissed.even.on.all.workspace.delete.mov

Red.dot.mp4

MacOS: Desktop

mac.desktop.red.dot.not.dismissed.even.on.all.workspace.delete.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~013278e8038908746f
Upwork Job ID: 1711854514958786560
Last Price Increase: 2023-10-10
Automatic offers:

hoangzinh | Reviewer | 27222500
dukenv0307 | Contributor | 27222503
dhanashree-sawant | Reporter | 27222505

melvin-bot · 2023-10-10T21:21:38Z

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

melvin-bot · 2023-10-10T21:21:38Z

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

melvin-bot · 2023-10-10T21:21:41Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-10-10T21:21:45Z

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

abzokhattab · 2023-10-10T22:29:25Z

Proposal

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

Deleting all workpsaces doesnt remove the red dot from the avatar and the settings workpsace tap

What is the root cause of that problem?

in this line we dont check if the current admin policies are empty or beside the reimbursementAccount.errors check

App/src/pages/settings/InitialSettingsPage.js

Lines 180 to 181 in 7f7b424

    
           const policyBrickRoadIndicator = 
        
               !_.isEmpty(props.reimbursementAccount.errors) ||

same for the avatar indicator

App/src/components/Indicator.js

Line 82 in 0cbee22

() => !_.isEmpty(props.reimbursementAccount.errors),

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

Get the admin typed workspaces and check if they are empty or not beside reimbursementAccount.errors check

        const adminPolicies = _.chain(props.policies)
            .filter((policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN)
            .value();

change the check to:

            (!_.isEmpty(props.reimbursementAccount.errors) && !_.isEmpty(adminPolicies)) ||

also in the indicator component we do the same ..

    const adminPolicies = _.chain(cleanPolicies)
        .filter((policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN)
        .value();

        () => !_.isEmpty(adminPolicies) && !_.isEmpty(props.reimbursementAccount.errors),

POC

Screen.Recording.2023-10-11.at.1.30.13.AM.mov

dukenv0307 · 2023-10-11T03:13:04Z

Proposal

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

App displays red dot of workspace error even after we delete all the workspaces, it removes the red dot on relogin

What is the root cause of that problem?

We're not clearing the errors from the reimbursementAccount after all workspaces are deleted.

So the red dot will still show in the "Workspaces" in settings in this condition and still show in user avatar in this condition

The reimbursementAccount 's sole purpose is for the workspaces so we shouldn't retain red dot error for reimbursement account if we don't have any workspace.

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

In deleteWorkspace, if that's the only workspace left, we should clear the errors of the reimbursementAccount in Onyx in optimisticData (or we should clear all data in reimbursementAccount if that's also OK), and restore it in failureData. Since reimbursement account is related to workspace and it doesn't make sense to keep its errors (and data) after all workspaces are deleted.

What alternative solutions did you explore? (Optional)

NA

abzokhattab · 2023-10-11T08:16:22Z

forgot to mention that by making those changes, whenever we create a new workspace again after we delete all of them, the red dot will be back again, which i think is the expected behavior.

dukenv0307 · 2023-10-11T12:23:54Z

Just IMO I'd be quite surprised if an error comes out of nowhere once I create a new workspace 😄

hoangzinh · 2023-10-11T14:44:09Z

@abzokhattab @dukenv0307 I'm asking on slack here https://expensify.slack.com/archives/C01GTK53T8Q/p1697023569138709 Would love to hear your voice there. Thanks

Charan-hs · 2023-10-12T16:59:48Z

@hoangzinh, hey from here it's mentioned that there is proper separation between workspace's bank account hence we should do the same thing in error also right? i.e. workspace's based error.

hoangzinh · 2023-10-12T23:38:53Z

@hoangzinh, hey from here it's mentioned that there is proper separation between workspace's bank account hence we should do the same thing in error also right? i.e. workspace's based error.

@Charan-hs Sorry, I don't get it. Could you clarify it again? Thanks

Charan-hs · 2023-10-13T09:24:40Z

@hoangzinh
Before this PR #24114, we stored a single bank detail for all workspaces but now we are storing separate bank details based on workspaces
For this Issue, we have to do the same here like each workspace will have its own error, which means an error object will store the workspace ID.

hoangzinh · 2023-10-13T10:45:48Z

@Charan-hs The PR you mentioned is closed. It looks like we still keep the previous behavior, the bank account will shared for all workspaces.

abzokhattab · 2023-10-13T10:47:05Z

What is the final verdict on here? which approach should we take? @hoangzinh

hoangzinh · 2023-10-13T10:55:37Z

@abzokhattab According to discussion here https://expensify.slack.com/archives/C01GTK53T8Q/p1697045876921169?thread_ts=1697023569.138709&cid=C01GTK53T8Q

We're expecting the bank account's error will be cleared out completely once we delete all workspaces

hoangzinh · 2023-10-13T10:59:32Z

@abzokhattab Thanks for your proposal #29235 (comment). But according to our expectation above #29235 (comment), your RCA is not totally correct, thus the solution won't solve the root cause.

hoangzinh · 2023-10-13T11:01:17Z

@dukenv0307 Thanks for your proposal #29235 (comment). Your solution looks great. But can you add more details to your RCA? It's hard to understand why we not clearing the errors from the reimbursementAccount after all workspaces are deleted will cause this bug. Thanks

dukenv0307 · 2023-10-13T18:09:11Z

But can you add more details to your RCA?

Proposal updated to add more detailed RCA, thank you for the feedback!

hoangzinh · 2023-10-14T06:22:33Z

@dukenv0307 thanks for updating. Your proposal has a correct RCA and also provided a solution that fits with our expectation here https://expensify.slack.com/archives/C01GTK53T8Q/p1697045876921169?thread_ts=1697023569.138709&cid=C01GTK53T8Q

Thus, I think we go with @dukenv0307's proposal #29235 (comment)

🎀👀🎀 C+ reviewed

melvin-bot · 2023-10-14T06:22:39Z

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

melvin-bot · 2023-10-16T15:38:50Z

📣 @hoangzinh 🎉 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

melvin-bot · 2023-10-16T15:38:56Z

📣 @dukenv0307 🎉 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 📖

melvin-bot · 2023-10-16T15:39:00Z

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

luacmartins · 2023-10-16T15:39:18Z

Assigning @dukenv0307 to the issue!

dukenv0307 · 2023-10-17T14:53:27Z

@hoangzinh The PR is ready for review.

hoangzinh · 2023-10-25T16:56:15Z

Hey team @dukenv0307 @luacmartins @NicMendonca , just wanna raise a recent change of Workspace bank account error here #27901 (comment). After that issue, our issue can not be reproduced anymore (because the Workspace bank account's error will be cleared when the user closes RHN). But It can be reproduced by another way as @dukenv0307 described in his PR #29781 (comment)

Close the tab and open app again

Just wanna discuss whether we should continue to fix this issue with those new steps

luacmartins · 2023-10-25T19:58:01Z

@hoangzinh do we have a video of what that looks like? Might help with deciding if we should still fix this

hoangzinh · 2023-10-26T16:36:55Z

sure. Those are recordings

Before PR fix blinking loader in connect bank account page #29260 (close LHN, workspace bank account error is still shown as RBR in user avatar)

Screen.Recording.2023-10-26.at.23.32.50.-.before.mov

After PR fix blinking loader in connect bank account page #29260 (close LHN, workspace bank account error is disappear in user avatar)

Screen.Recording.2023-10-26.at.23.31.20.-.after.mov

New way to reproduce (as suggested by @dukenv0307 in his PR Remove error when all workspace is deleted #29781 (comment))

Screen.Recording.2023-10-26.at.23.31.58.-.new.mov

hoangzinh · 2023-11-01T07:23:41Z

cc @luacmartins @NicMendonca waiting for your thoughts on the comment above ^.

luacmartins · 2023-11-01T16:50:39Z

Thanks for the videos @hoangzinh! Yea, I think we should still fix this.

hoangzinh · 2023-11-18T12:43:33Z

It looks like Melvin didn't process the next step for this issue. The PR #29781 has been deployed to PROD 10/11/2023

hoangzinh · 2023-11-18T12:43:51Z

cc @NicMendonca @luacmartins

hoangzinh · 2023-11-20T14:34:40Z

BugZero Checklist:

The PR that introduced the bug has been identified. Link to the PR: N/A. I think there is no PR directly causing this bug. It would be hard when we were implementing deleting a workspace, we also need to clear the workspace's bank account error. This issue is a kind of supplement to the logic we implemented previously.
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
Determine if we should create a regression test for this bug: ⛔ It's an edge case after PR fix blinking loader in connect bank account page #29260. This bug only occurs when the user closes/kills the App after saving Workspace's bank account.

NicMendonca · 2023-11-28T22:41:49Z

omg, sorry, this didn't pop open as a daily for me to pay

NicMendonca · 2023-11-28T22:43:34Z

Everyone has been paid via Upwork 🎉

m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 10, 2023

melvin-bot bot changed the title ~~Deleting all workspaces does not remove red dot~~ [$500] Deleting all workspaces does not remove red dot Oct 10, 2023

melvin-bot bot assigned NicMendonca Oct 10, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 10, 2023

melvin-bot bot assigned hoangzinh Oct 10, 2023

melvin-bot bot assigned luacmartins Oct 14, 2023

melvin-bot bot added the Overdue label Oct 16, 2023

luacmartins assigned dukenv0307 Oct 16, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 16, 2023

melvin-bot bot removed the Overdue label Oct 16, 2023

dukenv0307 mentioned this issue Oct 17, 2023

Remove error when all workspace is deleted #29781

Merged

59 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 17, 2023

kavimuru mentioned this issue Nov 7, 2023

Deploy Checklist: New Expensify 2023-11-06 #30929

Closed

luacmartins changed the title ~~[$500] Deleting all workspaces does not remove red dot~~ [HOLD for payment 2023-11-18] [$500] Deleting all workspaces does not remove red dot Nov 19, 2023

NicMendonca closed this as completed Nov 28, 2023

[HOLD for payment 2023-11-18] [$500] Deleting all workspaces does not remove red dot #29235

[HOLD for payment 2023-11-18] [$500] Deleting all workspaces does not remove red dot #29235

Comments

m-natarajan commented Oct 10, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 10, 2023

melvin-bot bot commented Oct 10, 2023

melvin-bot bot commented Oct 10, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Oct 10, 2023

abzokhattab commented Oct 10, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

POC

dukenv0307 commented Oct 11, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

abzokhattab commented Oct 11, 2023

dukenv0307 commented Oct 11, 2023

hoangzinh commented Oct 11, 2023

Charan-hs commented Oct 12, 2023

hoangzinh commented Oct 12, 2023

Charan-hs commented Oct 13, 2023

hoangzinh commented Oct 13, 2023

abzokhattab commented Oct 13, 2023

hoangzinh commented Oct 13, 2023

hoangzinh commented Oct 13, 2023

hoangzinh commented Oct 13, 2023

dukenv0307 commented Oct 13, 2023

hoangzinh commented Oct 14, 2023

melvin-bot bot commented Oct 14, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 16, 2023

luacmartins commented Oct 16, 2023

dukenv0307 commented Oct 17, 2023

hoangzinh commented Oct 25, 2023

luacmartins commented Oct 25, 2023

hoangzinh commented Oct 26, 2023

hoangzinh commented Nov 1, 2023

luacmartins commented Nov 1, 2023

hoangzinh commented Nov 18, 2023 • edited Loading

hoangzinh commented Nov 18, 2023

hoangzinh commented Nov 20, 2023

BugZero Checklist:

NicMendonca commented Nov 28, 2023

NicMendonca commented Nov 28, 2023

m-natarajan commented Oct 10, 2023 •

edited by melvin-bot bot

Loading

abzokhattab commented Oct 10, 2023 •

edited

Loading

dukenv0307 commented Oct 11, 2023 •

edited

Loading

hoangzinh commented Nov 18, 2023 •

edited

Loading