Expense-To field shows correct avatar but wrong name after creating new report on confirm page - #92902
Conversation
…ew report on confirm page
|
@ahmedGaber93 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] |
|
@DylanDylann please review |
| * @returns Record mapping policyID to Policy | ||
| */ | ||
| function useParticipantsPolicies(participants: ParticipantWithPolicyID[]): Record<string, Policy> { | ||
| const [participantsPolicies = getEmptyObject<Record<string, Policy>>()] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: getPoliciesSelector(participants)}, [participants]); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
The selector on ONYXKEYS.COLLECTION.POLICY returns Record<string, Policy> containing full Policy objects. Since useOnyx runs deepEqual on selector output, it must deep-compare entire Policy objects on every Onyx update to the policy collection. Policy objects can be large, making this comparison expensive.
Consider one of these approaches:
- Narrow the selector output to only the fields actually needed downstream (e.g.,
name,type,id), sodeepEqualcompares a small object per entry. - Remove the selector entirely and subscribe without one (using cheaper
shallowEqual), then filter/transform inline:
function useParticipantsPolicies(participants: ParticipantWithPolicyID[]): Record<string, Policy> {
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
// Filter inline — shallowEqual on raw references is cheaper than deepEqual on full Policy objects
const participantsPolicies = participants.reduce<Record<string, Policy>>((acc, participant) => {
const key = `${ONYXKEYS.COLLECTION.POLICY}${participant.policyID}`;
if (allPolicies?.[key] && participant.policyID) {
acc[participant.policyID] = allPolicies[key];
}
return acc;
}, {});
return participantsPolicies;
}Reviewed at: 12e1ee8 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-06-09.at.01.05.36.moviOS: mWeb SafariScreen.Recording.2026-06-09.at.01.06.41.movMacOS: Chrome / SafariScreen.Recording.2026-06-09.at.01.03.43.mov |
abzokhattab
left a comment
There was a problem hiding this comment.
The changes look good to me.. the issue is fixed .. the perf improvement comment makes sense, but its not a blocker
|
🚧 @tylerkaraszewski has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Expense-To field shows correct avatar but wrong name after creating new report on confirm page (cherry picked from commit 66125d0) (cherry-picked to staging by Julesssss)
|
🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.4.0-6 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review — no changes required I reviewed the changes in this PR against the help site articles under Conclusion: No help site updates are needed. Why:
No draft PR was created since there is nothing to document. |
|
Verified fixed: #92847 (comment) |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.0-7 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.4.1-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.1-6 🚀
|
Explanation of Change
Fixed Issues
$ #92847
PROPOSAL:
Tests
Precondition:
Create two workspaces - A and B.
Workspace A is the default workspace.
→ Workspace A is selected.
Offline tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-06-08.at.16.05.37.mov