fix: Expense report still shows Concierge message on LHN when it is hidden in the expense report#79888
fix: Expense report still shows Concierge message on LHN when it is hidden in the expense report#79888etCoderDysto wants to merge 9 commits intoExpensify:mainfrom
Conversation
…ory is disabled in policy
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.
|
…r message when category is disabled
…category is disabled
melvin-bot
bot
requested review from
abzokhattab and
trjExpensify
and removed request for
a team
|
@abzokhattab 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const reportPreviewAction = useMemo(() => getReportPreviewAction(report.chatReportID, report.reportID), [report.chatReportID, report.reportID]); | ||
| const didLayout = useRef(false); | ||
| const {isOffline} = useNetwork(); | ||
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, {canBeMissing: true}); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
This useOnyx hook fetches the entire policy object but only uses the areCategoriesEnabled property. This causes unnecessary re-renders when unrelated policy fields change.
Use a selector to fetch only the needed property:
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, {
canBeMissing: true,
selector: (policy) => ({areCategoriesEnabled: policy?.areCategoriesEnabled}),
});Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
I have made the changes for this comment
![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: d8f1f47330
ℹ️ 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".
src/libs/OptionsListUtils/index.ts
Outdated
| const reportActionsForDisplay = sortedReportActions.filter( | ||
| (reportAction, actionKey) => | ||
| (!(isWhisperAction(reportAction) && !isReportPreviewAction(reportAction) && !isMoneyRequestAction(reportAction)) || isActionableMentionWhisper(reportAction)) && | ||
| shouldReportActionBeVisible(reportAction, actionKey, isWriteActionAllowed) && | ||
| shouldReportActionBeVisible(reportAction, actionKey, isWriteActionAllowed, policy) && |
There was a problem hiding this comment.
reportActionsForDisplay now depends on policy.areCategoriesEnabled, but this logic still runs only inside the Onyx.connect handler for REPORT_ACTIONS; when categories are toggled, the policy collection updates without necessarily changing report actions, so lastVisibleReportActions can remain stale and continue showing the concierge category whisper in LHN until another report-action update happens. This makes the fix nondeterministic in sessions where policy data arrives or changes after actions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes, lastVisibleReportActions can remain stale in offline state. But it will be updated when the user is online since OpenReport API call will return report actions for the expense report. Then the callback will be called and the whisper message will be hidden. We also depend on BE to return the latest reply count when we disable categories feature.
|
can you review the ai generated comments also lets add test cases in order to increase test coverage @etCoderDysto |
Sure! I will review the ai generated comments and I will add tests. |
…s category feature
Hi @abzokhattab, I have added a unit test. And I have addressed AI reviewer's comment. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 482cc390ab
ℹ️ 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".
| (reportAction, actionKey) => | ||
| (!(isWhisperAction(reportAction) && !isReportPreviewAction(reportAction) && !isMoneyRequestAction(reportAction)) || isActionableMentionWhisper(reportAction)) && | ||
| shouldReportActionBeVisible(reportAction, actionKey, isWriteActionAllowed) && | ||
| shouldReportActionBeVisible(reportAction, actionKey, isWriteActionAllowed, policy?.areCategoriesEnabled) && |
There was a problem hiding this comment.
This filter now makes lastVisibleReportActions depend on policy?.areCategoriesEnabled, but the cache is only rebuilt inside the REPORT_ACTIONS Onyx callback; when Categories is toggled without any new report-action updates, the cached last action stays stale and getLastMessageTextForReport() can continue deriving LHN text from an old concierge category action. In practice this means the sidebar preview can remain incorrect until another report action arrives, so the cache should also be invalidated/recomputed from policy changes.
Useful? React with 👍 / 👎.
|
reviewing .. |
|
could you please remerge to main @etCoderDysto |
Hi @abzokhattab, this PR has completely changed how we calculate the last visible actions throughout the app. I will review the new changes and update my PR accordingly. |
|
Lots of conflicts here 🙏 |
|
Please let me know when its ready @etCoderDysto |
3 participants
Explanation of Change
Fixed Issues
$ #75276
$ #79817
PROPOSAL: #75276 (comment)
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Note: There is no offline test.
Prerequisite: Account has at least one workspace.
Prerequisite 2: Categories enabled.
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))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
Android: Native
web.Screen.Recording.2026-02-11.at.11.02.31.at.night.mp4
Android: mWeb Chrome
android.Screen.Recording.2026-02-11.at.11.40.10.at.night.mp4
iOS: Native
ios.Screen.Recording.2026-02-11.at.11.09.13.at.night.mp4
iOS: mWeb Safari
ios.safari.Screen.Recording.2026-02-11.at.11.23.19.at.night.mp4
MacOS: Chrome / Safari
web.Screen.Recording.2026-02-11.at.11.02.31.at.night.mp4