Fix Track workspace admins room visibility in LHN#95324
Conversation
|
@linhvovan29546 I tested the following cases, and they all passed:
One case needs confirmation: after signing out and signing back in, the active Track workspace And I think this is outside the expected scope of this issue. |
|
@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38be94bf7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isPinned: shouldPinAdminRoomByDefault(), | ||
| currentUserAccountID, | ||
| }), | ||
| ...(shouldShowTrackAdminRoomInLHN && {isTrackOnboardingAdminRoom: true}), |
There was a problem hiding this comment.
Do not pin Track onboarding admins rooms
When shouldShowTrackAdminRoomInLHN is true for Track-business onboarding, the report still inherits isPinned: shouldPinAdminRoomByDefault() from the optimistic chat data, which is true for normal non-Expensify/Guides accounts. That means the new onboarding #admins room is pinned instead of merely kept visible, so the documented QA path with a regular new account will show a pinned room; override isPinned to false for this marked Track onboarding case.
Useful? React with 👍 / 👎.
| !isConciergeChatReport(report, conciergeReportID) && | ||
| !isSystemChatReport && | ||
| !isSelfDMWithVisiblePreference && | ||
| !(getReportMetadata(report.reportID)?.isTrackOnboardingAdminRoom && isAdminRoom(report) && !isReportArchived) && |
There was a problem hiding this comment.
Hmm, if the user clears the cache, will the admin room be gone?
Explanation of Change
Marks the
#adminsroom created during Track-business onboarding so it remains visible in the LHN while active, without pinning it or changing regular workspace rooms. Archived rooms are excluded from the LHN while remaining accessible through search.Fixed Issues
$ #92431
PROPOSAL: #92431 (comment)
Tests
#adminsroom is visible in the LHN.#adminsremains visible.Offline tests
Not applicable. Completing Track-business onboarding and creating the workspace requires an active network connection.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android-native-admin.mp4
Android: mWeb Chrome
android-web-admin.mp4
iOS: Native
ios-native-admin.mp4
iOS: mWeb Safari
ios-web-admin.mp4
MacOS: Chrome / Safari
macos-admin.mp4