Load full report when admins open a room from the Workspace Rooms list#93795
Conversation
Admins open the report details RHP directly from the Workspace Rooms list instead of the room report, which skipped the openReport call ReportScreen makes. The RHP then operated on the stub report loaded for the list, breaking Join (member count dropped to 0), Invite and renaming. Fetch the full report when navigating so these actions run against complete data. Fixes Expensify#93248
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@gijoe0295 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-06-18.at.17.33.47.movAndroid: mWeb ChromeScreen.Recording.2026-06-18.at.17.31.25.moviOS: HybridAppScreen.Recording.2026-06-18.at.17.26.49.moviOS: mWeb SafariScreen.Recording.2026-06-18.at.17.28.02.movMacOS: Chrome / SafariScreen.Recording.2026-06-18.at.17.21.47.mov |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4a48da470
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isAdmin) { | ||
| // Admins open the details RHP directly instead of the room report, so the report is never fetched via ReportScreen. | ||
| // Fetch it here so the RHP has full data (participants, metadata) for Join, Invite and renaming. | ||
| openReport({reportID: report.reportID, introSelected, betas}); |
There was a problem hiding this comment.
When an admin who is already a member of a room opens it from Workspace settings → Rooms just to view details, this new openReport call uses the chat-opening path, whose implementation says it “updates the last read message” and sends useLastUnreadReportAction: true/clientLastReadTime. That means merely opening the details RHP can clear unread state for the room even though the user has not opened or read the conversation; use a report-data fetch that does not advance read state, or gate this to cases where marking read is intended.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@sumo-slonik This looks like a valid concern. Why have you marked this as 👎🏽 here?
There was a problem hiding this comment.
bump on this thread @sumo-slonik. Can we take off hold?
|
@shubham1206agra Hi, I’m wondering whether you should have been automatically assigned to the review here instead of @gijoe0295, since it’s related to rooms. |
|
Sorry but I'm already testing and filling the checklist 😿 |
|
Checklist is here |
|
Sorry @gijoe0295, but you won't be paid for this review as @sumo-slonik pointed out the wrong reviewer assignment before you filled any point in checklist. I will leave the decision to @grgia about this. |
|
@gijoe0295 sorry about that. It's true that this is @shubham1206agra's issue |
|
@grgia @sumo-slonik Isn't this issue solvable from BE side? Am I missing something? This is same bug as incomplete report/participant object is being sent from BE. |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Sure |
@shubcham, It doesn't seem like a backend issue to me. I fixed it purely on the frontend side, without any backend changes, and everything is working correctly now. Could you please take a look and review the changes when you have a moment? |
|
@sumo-slonik Let's hold this PR on https://github.com/Expensify/Auth/pull/22368 |
sumo-slonik
changed the title
@shubham1206agra Unfortunately, I can't check the status of that PR. Do you have any updates on it? |
|
@sumo-slonik PR still in review |
thanks! |
|
@sumo-slonik @shubham1206agra do we actually need to hold this PR on the auth fix or can they be separated? |
I don’t think we need to block this PR on the auth fix. They look independent and shouldn’t conflict. @shubham1206agra suggested we hold off, and then I assumed it is low priority, which is why I agreed to wait. |
|
https://github.com/Expensify/Auth/pull/22368 has been merged. Waiting for deploy. After this, retest the original issue. |
| if (isAdmin) { | ||
| // Admins open the details RHP directly instead of the room report, so the report is never fetched via ReportScreen. | ||
| // Fetch it here so the RHP has full data (participants, metadata) for Join, Invite and renaming. | ||
| openReport({reportID: report.reportID, introSelected, betas}); |
There was a problem hiding this comment.
An admin who is a member of the room loses its unread indicator just by opening the details RHP from the rooms list, without ever reading the chat.
Add a flag to OpenReportActionParams (e.g. shouldMarkAsRead, default true) that skips useLastUnreadReportAction and clientLastReadTime when the caller only needs report data (note: this is a new openReport param).
Alternatively, can we retest the original issue given auth is deployed. And close this if it's been fixed?
There was a problem hiding this comment.
@grgia I retested #93248 on main with the auth fix deployed — Join is fixed now, but Invite and renaming the room from the RHP are still broken, so we still need this PR.
I also confirmed the unread concern (opening the RHP was clearing the unread indicator), so I added the shouldMarkAsRead param as you suggested — when false, OpenReport is sent without useLastUnreadReportAction and clientLastReadTime.
There was a problem hiding this comment.
@sumo-slonik Another auth PR in place to fix invite https://github.com/Expensify/Auth/pull/22816
There was a problem hiding this comment.
If we're fixing this on the backend side as well, is it possible that editing the room from the RHP could be resolved the same way? In that case, this PR might not be necessary either.
What do you think, @shubham1206agra @grgia?
sumo-slonik
changed the title
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
I’ll be away for the rest of the week, so I’m leaving this PR in @WojtekBoman good hands. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.4.36-0 🚀
|
|
🤖 No help site changes required. This PR is a data-loading bug fix — when an admin opens a room from the Workspace Rooms list, the full report is now fetched before navigating so Join, Invite member, and room renaming operate on complete data. It doesn't add, remove, or change any user-facing feature, workflow, copy, or button label. The relevant help article — Find Rooms in a Workspace — already documents the correct, intended behavior this fix restores:
Since the documented behavior is unchanged and the PR simply makes the product match it, no draft PR was created. @sumo-slonik, let me know if you'd still like a docs update anyway and I can draft one. |
6 participants
Explanation of Change
Admins open the report details RHP directly from the Workspace Rooms list, skipping the
openReportcall thatReportScreennormally makes — so the RHP operated on the stub report loaded for the list and Join (member count dropped to 0), Invite and renaming broke. We now fetch the full report before navigating, so those actions run against complete data.Fixed Issues
$ #93248
PROPOSAL:
Tests
workspaceRoomsPagebeta enabled.Offline tests
N/A — the change only affects how an admin opens an existing room (an extra report fetch before navigation); there is no offline-specific behavior to verify.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
MacOS: Chrome / Safari
final_admin_test.mp4