Default rooms are not grayed out when creating a new workspace while offline

[neil-marcellini]

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

---

Action Performed:

1. Sign in with any account
2. Go offline
3. Click the green plus at the bottom, New workspace. Or if you already have a workspace click the avatar in the header to open the settings, click on a workspace, click the 3 vertical dots at the top, New workspace.

Expected Result:

The default rooms, #admins, #announce, and the workspace expense chat should be grayed out in the LHN and on the search page. When navigating to any of these rooms the whole room should be grayed out (50% opacity).

Actual Result:

The default rooms are not grayed out (50% opacity).

Workaround:

Ignore it

Platform:

All platforms

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: v1.2.18-10 Reproducible in staging?: Yes Reproducible in production?: Yes Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos:

https://user-images.githubusercontent.com/26260477/198112025-cf40f3df-9dbb-4ceb-b93b-bd8a97156a69.mov

Expensify/Expensify Issue URL: Issue reported by: @neil-marcellini Slack conversation:

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @CortneyOfstad (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[CortneyOfstad]

Was able to reproduce as well!

[melvin-bot[bot]]

Triggered auto assignment to @grgia (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot[bot]]

@grgia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@grgia Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

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

[grgia]

Not overdue, engineering triage!

[strepanier03]

@grgia - Are you recommending the External label on this?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[strepanier03]

Internal job posting: https://www.upwork.com/ab/applicants/1587582985521102848/suggested

---

External job posting: https://www.upwork.com/jobs/~01cf8f22223f210999

[michaelhaxhiu]

Hey @strepanier03, just dropping a note as a reminder to keep the pressure on to find a contributor and get this one closed out. This was just posted a few days ago so not much more you can do at this moment, but let's get clear on your question for @grgia (above):

@grgia - Are you recommending the External label on this?

[michaelhaxhiu]

Adding the daily label back btw! There's more recent context in slack here for reference.

[melvin-bot[bot]]

@eVoloshchak, @strepanier03, @MonilBhavsar Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[tienifr]

Proposal

Problem

In https://github.com/Expensify/App/blob/0724337138a4d80f6603074cc9af5c186cb524ec/src/libs/ReportUtils.js#L817-L852

when build the optimistic data for default chat report (admin, announce) we don't set pendingFields for them so In https://github.com/Expensify/App/blob/0724337138a4d80f6603074cc9af5c186cb524ec/src/components/LHNOptionsList/OptionRowLHN.js#L102

and https://github.com/Expensify/App/blob/0724337138a4d80f6603074cc9af5c186cb524ec/src/components/OptionRow.js#L136 the pendingAction is null

=> the logic to enable opacity is not applied

Solution

In https://github.com/Expensify/App/blob/0724337138a4d80f6603074cc9af5c186cb524ec/src/libs/ReportUtils.js#L817 I'll add pendingFields with createChat ='add'

        statusNum: 0,
        visibility,
+        pendingFields: {
+            createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+        }    
    };

https://user-images.githubusercontent.com/113963320/200283688-86463677-4c6c-4c08-b906-a16b5a3e0bdd.mp4

[strepanier03]

@eVoloshchak - Looks like there is a proposal to review for this.

When you get a chance please review and if you are okay with the solution ping the CME Monil so they can give their sign off as well.

[eVoloshchak]

when build the optimistic data for default chat report (admin, announce) we don't set pendingFields for them

@tienifr, thanks for the proposal, makes sense

I'll add pendingFields with createChat ='add'

This is indeed resolving the issue. Only thing I'm afraid of is that it will introduce issues with chat creation. @tienifr, we're calling buildOptimisticChatReport in a couple of places, is adding pendingFields to all of them safe?

[tienifr]

@eVoloshchak we're using buildOptimisticChatReport in 4 places: announceChatData, adminsChatData, expenseChatData, policyReport. As we can see the result above when pendingFields of announceChatData, adminsChatData, expenseChatData are updated to null when we go online. Here is the evidence of creating new policyReport:

https://user-images.githubusercontent.com/113963320/200779111-921de183-d7b4-4a51-adec-1a7ef7f80742.mp4

[eVoloshchak]

@tienifr thank you for checking this

we're using buildOptimisticChatReport in 4 places: announceChatData, adminsChatData, expenseChatData, policyReport.

Correct me if I'm wrong, but I can see it being used in 9 places (in requestMoney, createSplitsAndOnyxData, getOrCreateChatReport, etc)

[strepanier03]

@tienifr - Just a friendly bump to response to the comment from @eVoloshchak here. Thanks

I am out of the office tomorrow. If there is an urgent need on this GH from a BugZero team member, please post in #expensify-open-source for assistance.

[tienifr]

Hi @eVoloshchak We are using buildOptimisticChatReport in 8 places

In /Expensify/App/src/libs/ReportUtils.js

1. line 901: announceChatData (for creating workspace )
2. line 917: adminsChatData (for creating workspace )
3. line 929: expenseChatData (for creating workspace ) (But after using buildOptimisticChatReport in 3 above places, we update directly to Oynx and don't update pendingField and it causes this issue)

In /Expensify/App/src/libs/actions/IOU.js 4. line 63: chatReport (for creating request money ) 5. line 245: groupChatReport (for creating split bill) 6. line 331: oneOnOneChatReport (for creating split bill)

In /Expensify/App/src/libs/actions/Report.js 7. line 569: newChat = ReportUtils.buildOptimisticChatReport(formattedUserLogins); (for opening and entering chat) 8. line 1057: const policyReport = ReportUtils.buildOptimisticChatReport(...) (for creating room)

--> In 5 above places (4->8) after using buildOptimisticChatReport in 3 places, we update pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD} before syncing to Oynx

So I suggest 2 solutions to this problem

Solution 1

We only update pendingField in the first 3 places (1-->3)

the sample PR: https://github.com/Expensify/App/pull/12664

I tested and it worked correctly

https://user-images.githubusercontent.com/113963320/201303589-cd0defcf-3aba-4be3-bd48-acaa35874e5d.mp4

Solution 2

In https://github.com/Expensify/App/blob/0724337138a4d80f6603074cc9af5c186cb524ec/src/libs/ReportUtils.js#L817 I'll add pendingFields with createChat ='add'

        statusNum: 0,
        visibility,
+        pendingFields: {
+            createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+        }    
    };

and remove updating pendingFields manually in other places (4-->8) The sample PR: https://github.com/Expensify/App/pull/12663

I tested it in all places and it worked correctly

https://user-images.githubusercontent.com/113963320/201309120-bcde4ab6-b26a-439e-b96c-e791d7c4dbe9.mp4

[melvin-bot[bot]]

@eVoloshchak, @strepanier03, @MonilBhavsar Whoops! This issue is 2 days overdue. Let's get this updated quick!

[eVoloshchak]

Solution 1

We only update pendingField in the first 3 places (1-->3) the sample PR: #12664

I like @tienifr proposal I think we should go with the first solution since it adds pending actions only for the rooms we need to be greyed out

@MonilBhavsar, could you help me out, are there any default rooms besides #admins and #announce? The issue description states The default rooms, #admins, #announce, and the workspace expense, which implies there are other default rooms

[strepanier03]

@MonilBhavsar - Just a friendly bump on @eVoloshchak's comment.

Based on the recreation video in the OP post though, I think the "workspace expense" room is the one named "Workspace", right above the admin and announce rooms.

If you agree with the proposal and hiring @tienifr, just let me know and I'll take care of the next steps.

[strepanier03]

Will check tomorrow for an update.

[MonilBhavsar]

Sorry missed this. Default rooms include - Workspace room, #admins, #announce and domain default group. In this specific issue, I think we'll be taking into account Workspace room, #admins and #announce as they are related to a Workspace

[eVoloshchak]

@MonilBhavsar, thank you!

In that case Solution 1 of @tienifr's proposal looks good to me 🎀👀🎀 C+ reviewed!

[melvin-bot[bot]]

📣 @tienifr You have been assigned to this job by @strepanier03! Please apply to this job in Upwork 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 📖

[strepanier03]

The job post is here, please apply and I'll monitor for applications.

@tienifr - As a reminder, I wanted to link to the new process change regarding PR merge and bonuses here. You can read more about it in the CONTRIBUTING.md as well.