Fix opening an expense report from Reports page shows the same report when open Inbox#79436
Conversation
|
@aimane-chnaif 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] |
|
I haven't fixed the regression yet because I need some advice on which approach is the best to fix it. Expense - Not here page opens in Inbox after deleting report on Reports We can easily fix both issues by checking if the report still exists, but it means we need to subscribe to the whole report collection in NavigationTabBar/index.ts, which means it will re-render every time any report is updated. The other approach is:
|
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I think that we should definitely not do the first approach. I am not sure about the other two, I am curious if @WojtekBoman faced similar issue before and maybe he would know what to recommend |
|
@bernhardoj, If I understand correctly how onyx selectors work, you could modify the first proposed solution and create a selector that will extract only the information you need to make it work (if the report still exists) |
@adamgrzybowski I thought about creating a selector that maps the collection into "another" collection that holds boolean as the value, but I realize it creates a new object each time the selector is called, so it would still re-render. So, I tried this: const lastReportRoute = useRootNavigationState((rootState) => {
if (!rootState) {
return undefined;
}
return getLastRoute(rootState, NAVIGATORS.REPORTS_SPLIT_NAVIGATOR, SCREENS.REPORT);
});
const lastReportRouteReportID = (lastReportRoute?.params as ReportsSplitNavigatorParamList[typeof SCREENS.REPORT])?.reportID;
const doesLastReportExistSelector = useCallback((report: OnyxEntry<Report>) => !!report?.reportID, []);
const [doesLastReportExist] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${lastReportRouteReportID}`, {canBeMissing: true, selector: doesLastReportExistSelector}, [lastReportRouteReportID]);
const [lastReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${lastReportRouteReportID}`, {canBeMissing: true});
useEffect(() => {
console.log('tab bar', lastReportRouteReportID, doesLastReportExist, lastReport)
}, [lastReportRouteReportID, doesLastReportExist, lastReport])but somehow the useOnyx is not being recalculated after the report collection is cleared (when switching copilot). Could there be an onyx bug? (if I only delete the individual report like the steps outlined in #79249, then the useOnyx is recalculated) |
|
@bernhardoj Do you need the whole collection as output from the selector? Can't it be a single boolean? |
@adamgrzybowski it's a single booelan |
|
Oh, sorry, I didn't notice 😄. In that case, I would contact someone who is an expert in onyx, because this looks like the best solution that should work if we manage to fix this bug. BTW, is there an imperative way to get information from Onyx? In that case, the Boolean selector could be the one for |
I don't think there is one.
Posted in Slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1768666683491279 |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Woo! So glad we're fixing this, it's really annoying. ;) Product approved.
|
Pushed the fix, but we need to wait for the onyx fix. |
| Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(reportID, reportActionID, referrer, backTo)); | ||
| return; | ||
| } | ||
| if (isRoutePreloaded(NAVIGATORS.REPORTS_SPLIT_NAVIGATOR)) { |
There was a problem hiding this comment.
@WojtekBoman @adamgrzybowski is it correct to prioritize the existing report?
There was a problem hiding this comment.
I think yes, but I will review this with @WojtekBoman tomorrow once again and confirm
|
@adamgrzybowski @WojtekBoman can you review please? @bernhardoj conflicts |
|
Fixed |
|
Lint |
|
@mountiny fixed |
|
Hey, I'd like to confirm one thing :) Is this bug reproducible on staging? Screen.Recording.2026-02-04.at.10.59.43.mov |
|
I can repro it on web.mp4 |
|
@aimane-chnaif can you please test and review? |
|
@bernhardoj please merge main |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-02-10.at.12.21.34.pm.mov |
|
Regression tests pass Screen.Recording.2026-02-10.at.12.12.15.pm.movScreen.Recording.2026-02-10.at.12.16.19.pm.movScreen.Recording.2026-02-10.at.12.12.57.pm.mov |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@aimane-chnaif done |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @mountiny 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/mountiny in version: 9.3.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.17-9 🚀
|
Explanation of Change
Fixed Issues
$ #67122
PROPOSAL: #67122 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Prerequisite: have an expense
Web:
Android/iOS/mWeb:
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Android: Native
android.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
Uploading ios mweb.mp4…
MacOS: Chrome / Safari
web.mp4