Part 1: Remove Onyx.connect() for the key: ONYXKEYS.PERSONAL_DETAILS_LIST in src/libs/actions/Report.ts#77636
Part 1: Remove Onyx.connect() for the key: ONYXKEYS.PERSONAL_DETAILS_LIST in src/libs/actions/Report.ts#77636lorretheboy wants to merge 24 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@parasharrajat 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] |
src/pages/NewChatPage.tsx
Outdated
| const [isSearchingForReports] = useOnyx(ONYXKEYS.IS_SEARCHING_FOR_REPORTS, {initWithStoredValues: false, canBeMissing: true}); | ||
| const [reportAttributesDerived] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {canBeMissing: true, selector: reportsSelector}); | ||
| const selectionListRef = useRef<SelectionListHandle | null>(null); | ||
| const [allPersonalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {canBeMissing: true}); |
There was a problem hiding this comment.
use personalDetails hook instead.
There was a problem hiding this comment.
@lorretheboy would you ping when tests are passing
| const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? [], isOffline, reportTransactionIDs); | ||
| const oneTransactionID = reportTransactions.at(0)?.transactionID; | ||
|
|
||
| const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {canBeMissing: true}); |
There was a problem hiding this comment.
should we use the hook here too
|
🚧 @grgia 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, Desktop, and Web. Happy testing! 🧪🧪
|
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
grgia
left a comment
There was a problem hiding this comment.
@lorretheboy would you please merge main so it's up to date with latest, since it's been a few weeks during the holidays 🙇
|
Merged main |
|
Bump @lorretheboy |
|
what's the latest here @lorretheboy |
|
@parasharrajat @grgia It is ready for re-review now. Thxx for waiting |
src/pages/Share/ShareDetailsPage.tsx
Outdated
| })) ?? []; | ||
|
|
||
| const ownerAccountID = report.ownerAccountID; | ||
| if (ownerAccountID && personalDetails?.[ownerAccountID]) { |
There was a problem hiding this comment.
you should pass reportOwnerPersonalDetails in a different param not in participants.
| undefined, | ||
| undefined, | ||
| ); | ||
| const participants: ParticipantInfo[] = |
There was a problem hiding this comment.
When I asked to pass personaldetails of participants and owner as param. I also meant that instead of subscribing to whole ONYXKEYS.PERSONAL_DETAILS_LIST collection, we should only subscribe to participants and owner user's personal details.
So you should update the logic to use a selector in following subscription which filters the personal details for necessary users only.
If we can't create selector dynamically at some places then we should use the personalDetails hook instead.
|
bump @lorretheboy |
|
@lorretheboy Let us know when this is ready again. |
|
It is ready for review last week. Sorry I forgot to ping you. It's now conflict again, lemme resolve it |
|
@lorretheboy More Conflicts. |
|
@lorretheboy Conflicts. |
parasharrajat
left a comment
There was a problem hiding this comment.
Can we use const personalDetails = usePersonalDetails(); hook instead where possible?
| displayReport.participantsList | ||
| ?.filter((u) => u.accountID !== personalDetail.accountID) | ||
| .map((u) => ({ | ||
| login: u.login ?? '', | ||
| accountID: u.accountID, | ||
| personalDetails: personalDetails?.[u.accountID] ?? undefined, | ||
| })) ?? []; | ||
|
|
||
| const ownerPersonalDetails = report?.ownerAccountID && personalDetails?.[report?.ownerAccountID] ? personalDetails[report?.ownerAccountID] : undefined; |
There was a problem hiding this comment.
I guess there is no use of this now as we are already subscribing to whole personaldetails collection....
As per my previous comment, we can't use the selector pattern now. Do you still see any benefit of passing only participants and owner personal details.
|
Bump @lorretheboy. I am ready to close this fast, but if you handlethe requested changes in a timely manner. |


Explanation of Change
Fixed Issues
$ #73656
PROPOSAL: #73656 (comment)
Tests
Test 1:
Test 2 (Only for native device IOS, Android):
Offline tests
QA Steps
Same as Tests
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.mov
Screen.Recording.2025-12-14.at.21.52.37.mov
Android: mWeb Chrome
WEBSITE.ANDROID.mov
iOS: Native
ios_0AQlQiwu.mp4
Screen.Recording.2025-12-14.at.21.50.05.mov
iOS: mWeb Safari
WEBSITE.IOS.mov
MacOS: Chrome / Safari
WEBSITE.mov