[HOLD for payment 2024-02-14] [$500] Workspace room - No red dot in LHN for same WS room once internet connection is enabled

[lanitochka17]

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.4.30-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4226256&group_by=cases:section_id&group_order=asc&group_id=229067
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Navigate to staging.new.expensify.com & log in
2. On main device, create a workspace room from the FAB button
3. Disable the internet connection in the device
4. Click the header of the workspace room to open the room details page
5. Click "Settings"
6. Rename the room (#examplerename) and hit save (Note: inputs are grayed out (50% opacity)
7. Sign into the same account on another device or incognito window (Note: in this case I signed in on mobile web - please check the video attachment for secondary device on mobile web)
8. Create another workspace room on the same workspace with the same name (#examplerename)
9. In the main testing device - enable the internet connection

Expected Result:

Red dot should appear in LHN once internet connection is enabled in main device for a WS room that is created on secondary device with same name

Actual Result:

No red dot in LHN for same WS room once internet connection is enabled

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

Bug6353198_1706102039453.Recording__15.mp4

Bug6353198_1706102039450.Secondarydevice-Create_same_WS.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0132f5dcb3eeda3dc1
• Upwork Job ID: 1750167560080097280
• Last Price Increase: 2024-01-24
• Automatic offers:
• bernhardoj | Contributor | 28129966

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

Proposal

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

The LHN doesn't show a red dot error when there is an error.

What is the root cause of that problem?

This is a regression from #31448

The red dot will show if there is an error.

App/src/libs/SidebarUtils.ts

Lines 322 to 323 in 6cfdd6f

	result.allReportErrors = OptionsListUtils.getAllReportErrors(report, reportActions);
	result.brickRoadIndicator = hasErrors || hasViolations ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';

And hasErrors is defined before result.allReportErrors.

App/src/libs/SidebarUtils.ts

Line 309 in 6cfdd6f

const hasErrors = Object.keys(result.allReportErrors ?? {}).length !== 0;

So, it's always false

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

Put hasErrors after result.allReportErrors

or we can just have a new local var called allReportErrors and use it for both hasErrors and result.allReportErrors.

[Santhosh-Sellavel]

Seems like a regression from #31448. Will it be taken care of there, or should it be fixed here?
PR is still in the regression period
cc: @cead22 @cdanwards

[cead22]

In the interest of moving this forward quickly, I'm good with solving this here. @Santhosh-Sellavel does the proposal look good to you?

[Santhosh-Sellavel]

@bernhardoj's proposal LGTM!

we can just have a new local var called allReportErrors and use it for both hasErrors and result.allReportErrors

This sounds good to me!

C+ Reviewed
🎀 👀 🎀

cc: @cead22

[melvin-bot]

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

[Santhosh-Sellavel]

Waiting for CME Review.

I'm unavailable next week, Please assign a new C+ Issue here if required while I am away, thanks!

cc: @NikkiWines

[kadiealexander]

@NikkiWines @cead22 any thoughts on Santhosh's assessment?

[NikkiWines]

That proposal looks good to me - we can assign another C+ if the PR is up before @Santhosh-Sellavel is back!

[melvin-bot]

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

[bernhardoj]

PR is ready

[aimane-chnaif]

PR is up too early 👍
I can review PR if we cannot wait until next week.

[melvin-bot]

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

[melvin-bot]

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

• Fix red dot error doesn't show on LHN #35356

If no regressions arise, payment will be issued on 2024-02-14. 🎊

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

• @bernhardoj requires payment automatic offer (Contributor)
• @Santhosh-Sellavel requires payment through NewDot Manual Requests

[melvin-bot]

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:

• [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR: Adds new ReportUtils for Violations #31448
• [@Santhosh-Sellavel] 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: https://github.com/Expensify/App/pull/31448/files#r1499771503
• [@Santhosh-Sellavel] 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: Can raise in slack if necessary but this should've been caught by including offline testing for the changes in the linked PR. It was a fairly large change without specified offline steps, which is why this slipped through.
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug. ❌ see above
• [@Santhosh-Sellavel] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. ❌ see above
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[kadiealexander]

Payouts due:

• Contributor: $500 @bernhardoj
• Contributor+: $500 @Santhosh-Sellavel

Upwork job is here.

[kadiealexander]

@Santhosh-Sellavel don't forget about the checklist please!

[kadiealexander]

Santhosh is OOO til March - @NikkiWines could you please help with the BugZero checklist in his absence?

[NikkiWines]

@kadiealexander done!

[kadiealexander]

Closing, Santhosh can request payment on NewDot!

[Santhosh-Sellavel]

Requested on ND

[JmillsExpensify]

$500 approved for @Santhosh-Sellavel