Report details - Not found page opened when clicking admin report header - #96888
Report details - Not found page opened when clicking admin report header#96888lorretheboy wants to merge 7 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@lorretheboy any estimation? |
|
As I said earlier in the issue #94977 (comment), I will try to open it for review today @marufsharifi |
|
@lorretheboy, do you need any help here? |
|
@marufsharifi No need helps here, I will open for review in 1 hour |
|
@marufsharifi 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] |
|
Hey @marufsharifi, for android app recording, I will upload later as now I am having android build issue |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen_Recording_20260725_104451_Expensify.Dev.mp4Android: mWeb ChromeScreen_Recording_20260725_104658_Chrome.mp4iOS: HybridAppScreen.Recording.2026-07-25.at.11.26.05.AM.moviOS: mWeb SafariScreen.Recording.2026-07-25.at.11.29.36.AM.movMacOS: Chrome / SafariScreen.Recording.2026-07-25.at.10.31.02.AM.mov |
|
@lorretheboy, could you please merge the main, and upload android app record. thanks. |
|
@lorretheboy, Gentle bump. thanks. |
|
Will do it today |
|
@marufsharifi I merged latest main & added android app recording |
| * rooms, …) rather than jumping to the report — see https://github.com/Expensify/App/pull/90113. | ||
| */ | ||
| function navigateToDetailsPage(report: OnyxEntry<Report>) { | ||
| function navigateToDetailsPage(report: OnyxEntry<Report>, isInSidePanel = false) { |
There was a problem hiding this comment.
The new isInSidePanel flag is only applied to the REPORT_DETAILS branch, but the early PROFILE branch a few lines below still falls back to getActiveRoute():
if (isSelfDMReport || isOneOnOneChatReport) {
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.PROFILE.getRoute(participantAccountID.at(0))));
return;
}The Side Panel shows either the admins room (a chat room → hits the fixed branch ✅) or the concierge report (SidePanelContextProvider.tsx:94). Concierge is a chat report with no chat type and a single other participant, so isOneOnOneChat can return true for it — meaning clicking the concierge header from the Side Panel takes this PROFILE branch and can reproduce the same Not-found symptom this PR fixes (e.g. building /workspaces/<policyID>/overview/<profile-suffix>).
Since concierge is the default Side Panel content for non-admins / forceConcierge, this isn't an exotic path. Suggest threading the same flag here:
if (isSelfDMReport || isOneOnOneChatReport) {
const basePath = isInSidePanel ? ROUTES.REPORT_WITH_ID.getRoute(report?.reportID) : undefined;
Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.PROFILE.getRoute(participantAccountID.at(0)), basePath));
return;
}Could you verify how concierge details opens from the Side Panel? If it Not-founds, the above is the fix (plus a matching test case). If it opens cleanly today, a short test documenting that is enough.
|
@MelvinBot review |
Review — looks correct and well-scoped ✅The fix correctly addresses the root cause. When a report is rendered in the Side Panel, Both the code and the test read well, and it's a nicely minimal change. A few things I checked and one thing worth a quick sanity-check: Verification details
One thing worth a quick check (not a blocker): the Side Panel report can also be the Concierge report ( Everything else looks good and all CI checks are green. |
|
@lorretheboy, could you please check the above comments |
|
@marufsharifi Please check again |
|
the changes works, i am reviewing the code changes. |
|
@lorretheboy Could you cleanup the comments in the added test. They are too verbose and make references to issues and PRs that will be out of place for someone reading this codebase in the future. |
|
@inimaga I removed those comments. Kindly check again |
| * @param isInSidePanel - Whether the report is rendered inside the Side Panel. A Side Panel report is not part of the | ||
| * navigation state (`SidePanelReport` renders `ReportScreen` with a synthetic route), so the active route belongs to | ||
| * whichever screen sits behind the panel. In that case the destination (details or profile, since the Side Panel shows | ||
| * either the admins room or the 1:1 Concierge chat) has to be anchored to the report itself. | ||
| * Everywhere else the active route is used on purpose, so details opens within the current context (Search, workspace | ||
| * rooms, …) rather than jumping to the report — see https://github.com/Expensify/App/pull/90113. |
There was a problem hiding this comment.
@lorretheboy One more suggested improvement to this doc section as it still references an external PR, which you can apply and we're good to merge.
| * @param isInSidePanel - Whether the report is rendered inside the Side Panel. A Side Panel report is not part of the | |
| * navigation state (`SidePanelReport` renders `ReportScreen` with a synthetic route), so the active route belongs to | |
| * whichever screen sits behind the panel. In that case the destination (details or profile, since the Side Panel shows | |
| * either the admins room or the 1:1 Concierge chat) has to be anchored to the report itself. | |
| * Everywhere else the active route is used on purpose, so details opens within the current context (Search, workspace | |
| * rooms, …) rather than jumping to the report — see https://github.com/Expensify/App/pull/90113. | |
| * Side Panel reports use a synthetic route that is not part of the navigation state, so navigation must be anchored to the report. | |
| * Otherwise, the active route is preserved to keep the details page within its originating context. | |
| * | |
| * @param isInSidePanel - Whether the report is rendered inside the Side Panel |
4 participants
Explanation of Change
Fixed Issues
$ #94977
PROPOSAL: #94977 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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
Screen.Recording.2026-07-27.at.22.05.50.mov
Android: mWeb Chrome
Screen.Recording.2026-07-24.at.17.57.52.mov
iOS: Native
Screen.Recording.2026-07-24.at.17.54.39.mov
iOS: mWeb Safari
Screen.Recording.2026-07-24.at.17.56.17.mov
MacOS: Chrome / Safari
96888.mov