Fix/92672 anon public room lhn#93048
Conversation
An anonymous session can only access public rooms, whose notification preference defaults to `hidden`. Previously a public room only survived the LHN filter while it was the focused report, so opening a thread inside the room (which steals focus) dropped the room from the Inbox, leaving the anon user unable to return to it. Add an override in `shouldDisplayReportInLHN` so a public room stays in the LHN for anonymous users regardless of focus, mirroring how ReportUtils already gates anon-only public-room behaviour (e.g. canLeaveChat). Fixes Expensify#92672 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
melvin-bot
Bot
requested review from
brunovjk and
flaviadefaria
and removed request for
a team
melvin-bot
Bot
requested review from
a team and
trjExpensify
and removed request for
a team
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| function BeneficialOwnerDetailsFormPages({stepNames, policyID, onFinished, backTo}: BeneficialOwnerDetailsFormPagesProps) { | ||
| const {translate} = useLocalize(); | ||
| const [reimbursementAccountDraft] = useOnyx(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM_DRAFT); | ||
| const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT) subscribes to the entire reimbursement account object, but getCurrencyForNonUSDBankAccount only reads reimbursementAccount?.achData?.country (a single nested field). This causes the component to re-render whenever any field on the reimbursement account changes.
Use a selector to extract only the needed field:
const [reimbursementAccountCountry] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT, {
selector: (account) => account?.achData?.[INPUT_IDS.ADDITIONAL_DATA.COUNTRY],
});Then pass this value directly instead of the full object, or adjust getCurrencyForNonUSDBankAccount to accept the country string.
Reviewed at: e420185 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| const {translate} = useLocalize(); | ||
| const [reimbursementAccountDraft] = useOnyx(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM_DRAFT); | ||
| const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); | ||
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
useOnyx(ONYXKEYS.COLLECTION.POLICY${policyID}) subscribes to the entire policy object, but getCurrencyForNonUSDBankAccount only reads policy?.outputCurrency (a single string field). This causes the component to re-render whenever any policy field changes.
Use a selector to extract only the needed field:
const [outputCurrency] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {
selector: (policy) => policy?.outputCurrency,
});Then pass this value directly instead of the full policy object, or adjust getCurrencyForNonUSDBankAccount to accept the currency string.
Reviewed at: e420185 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
1 participant
Explanation of Change
An anonymous session can only ever access public rooms, and such a room's notification preference defaults to
hidden. In the LHN filter (shouldDisplayReportInLHNinSidebarUtils), ahiddenreport only survives when one of theshouldOverrideHiddenconditions is true — and for an anonymous user the only one that ever applied wasisFocused. So the public room stayed in the Inbox only while it was the focused report. The moment the anon user opened a thread inside the room, the thread became the focused report, the public room was no longer focused,isHidden && !shouldOverrideHiddenbecame true, and the room was dropped from the Inbox — collapsing it to the "Woohoo! All caught up" empty state and leaving the anon user unable to return to the room.This PR adds a new
shouldOverrideHiddencondition so a public room is never hidden from an anonymous user's LHN just because it isn't the focused report:This pairs
isPublicRoomwithisAnonymousUser()exactly the wayReportUtilsalready gates anon-only public-room behaviour elsewhere (e.g.canLeaveChat). It is a display-only predicate — it does not mutate any report/Onyx data or notification preference — so there are no side effects on unread/GBR or leave-room logic. After sign-inisAnonymousUser()becomesfalse, the override no longer applies, and the existing flow (#91931) re-fetches the room with a real notification preference, so there is no regression and no double-handling.Fixed Issues
$ #92672
PROPOSAL: #92672 (comment)
Tests
…/r/<publicRoomID>).Offline tests
This change is a purely client-side display predicate, so it behaves identically offline.
QA Steps
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
Android: Native
Before fix:
Before-fix-android.mp4
After fix:
After-fix-android.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Before fix:
Before-fix.Chrome-mac.mp4
After fix:
After-fix.Chrome-mac.mp4