Repository navigation
[$250] Android – Task – Workspace #Admin is missing in Share somewhere when create task via FAB #46210
Description
Activity
Triggered auto assignment to @AndrewGable (
DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.👋 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:
- Identify the pull request that introduced this issue and revert it.
- Find someone who can quickly fix the issue.
- Fix the issue yourself.
We think that this bug might be related to #vip-vsp
Doesn't really look like any changes to
SearchForReportsin 2+ months on the backend, so thinking this might be a front end changeCan't repro on Android native 9.0.12-0
Screen.Recording.2024-07-25.at.12.09.19.PM.mov
If it's not reproducible every time and it's a smaller feature (tasks), let's not block on it. We will treat it as a normal bug.
9 remaining items
Proposal
Please re-state the problem that we are trying to solve in this issue.
The WS #admin room doesn't show in LHN or in the task share somewhere.
What is the root cause of that problem?
Don't show the #admins room in the LHN till there is (1) another admin (2) there is a policy audit log to review. (3) The user clicks on chat with your guide link in onboarding
This happens after #45048 where we hide #admins room if it's "empty".
Lines 5580 to 5583 in 0559307
This works well for LHN, however,
shouldReportBeInOptionListis being used in 3 places. 1 for the LHN (SidebarUtils),
1 for the unread indicator updater, to count how many unread
App/src/libs/UnreadIndicatorUpdater/index.ts
Lines 11 to 23 in 0559307
and the last one is in
getOptions, to get the list of options of search, share somewhere, etc.
App/src/libs/OptionsListUtils.ts
Lines 1897 to 1908 in 0559307
What changes do you think we should make in order to solve the problem?
We can add a new param called
excludeEmptyAdminswhich defaults to true and set it to false forgetOptions.if (excludeEmptyAdmins && isAdminRoom(report) && !shouldAdminsRoomBeVisible(report)) { return false; }We can set it to true for the unread indicator updater too, but I think we don't want to count unread from reports that don't show in LHN.
@bernhardoj thanks for your proposal. Your RCA looks great to me. Reg. your solution, I think it would cause a regression because we also want to show reports that have > 1 admins even though it's empty
Lines 5428 to 5432 in 3008ee0
Moreover, what do you think if we reuse the existing option
excludeEmptyChats?Moreover, what do you think if we reuse the existing option excludeEmptyChats?
I thought about that too, but the definition of empty between the usages is different. The current
excludeEmptyChatsusage see a report without a message is empty, but it's different with the admin room, that's why I add a new one.Reg. your solution, I think it would cause a regression because we also want to show reports that have > 1 admins even though it's empty
We still use the same condition
shouldAdminsRoomBeVisible, so I don't think it would cause a regression.We still use the same condition shouldAdminsRoomBeVisible, so I don't think it would cause a regression.
You're right @bernhardoj.
I thought about that too, but the definition of empty between the usages is different. The current excludeEmptyChats usage see a report without a message is empty, but it's different with the admin room, that's why I add a new one.
I agree they use different methods. But eventually, don't they define an empty chat report (one for normal chats, another one for admin rooms)? I am happy with your proposal, but I just want to try to understand what is different and why you'd like to add a new one.
but I just want to try to understand what is different and why you'd like to add a new one.
I think the only other difference is that in UnreadIndicatorUpdater, we include empty chat when calculating unread reports. But this doesn't affect the current empty chat because if it's empty, then it won't affect the unread count.
I think we can use excludeEmptyChats but also update UnreadIndicatorUpdater to exclude empty chat.
Cool, I think we can discuss and select a better one when we implement it in PR. Overall, @bernhardoj's proposal looks good to me.
Link to proposal #46210 (comment)
🎀👀🎀 C+ reviewed
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
Wait, hmm. Is this actually a bug? Can they post directly in the #admins room in this scenario? If not, I think it'd make sense that they also can't share tasks to it.
We currently can't access the #admins room from the search page, but we can still open it from the workspace page Go to #admins.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Awaiting @puneetlath on 2nd review.
I'm sorry y'all, after thinking about it more, I don't actually think we should do this. If we aren't showing the #admins room in the chat switcher, then it makes sense to me that you also can't share tasks to it. I'm gonna close this out, but feel free to comment/reopen if you disagree!
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: 9.0.12-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/4768436
Email or phone of affected tester (no customers): applausetester+jp_e_category_2@applause.expensifail.com
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Workspace #Admin is present in Share somewhere list
Actual Result:
Workspace #Admin is missing in Share somewhere list. Admin room is missing in LHN when creating a new WS
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6552587_1721908173913.Admin.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @hoangzinh