fix: migrate NewChatPage to use useSearchSelector - #95778
Conversation
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.
|
|
@ZhenjaHorbach 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 can't log in to native app: Screen.Recording.2026-07-13.at.22.26.01.movScreen.Recording.2026-07-13.at.22.26.17.movI'll try fix the issue and add missing screenshots later. |
![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: f5a1a9cb1b
ℹ️ 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".
| hasMore: hasMoreFilteredPersonalDetails, | ||
| } = usePaginatedData(options.personalDetails, PAGINATION_SIZE, {resetKey: cleanSearchTerm, skipPagination: !isSearching}); | ||
| maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, | ||
| enablePhoneContacts: true, |
There was a problem hiding this comment.
When native contact import is enabled here, the old NewChatPage path first ran mergeAndSortPersonalDetailsWithContacts, which de-duped imported contacts against Onyx personal details and preferred the real Onyx account on login collisions. The selector path now relies on useSearchSelector, whose native wrapper simply concatenates contact options before getValidOptions, so a device contact whose email/phone already exists in personal details can render as a second row with a generated accountID; selecting that duplicate writes the generated ID into NEW_GROUP_CHAT_DRAFT, and the confirm/create flow treats the existing member as an optimistic invite rather than the real account.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| personalDetails.length + recentReports.length !== 0, | ||
| !!userToInvite, | ||
| debouncedSearchTerm.trim(), | ||
| countryCode, |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The new updateGroupDraft builds the group-chat draft participants array (mapping the selected options to {login, accountID} with the CONST.DEFAULT_NUMBER_ID fallback and appending the current user as creator) using logic that is nearly identical to the pre-existing createGroup function in this same file. Duplicating this participant-construction logic in two places means a future change (e.g. adding a field to SelectedParticipant) has to be made twice and can silently drift.
Extract the shared participant-array construction into a single helper and call it from both places, e.g.:
const buildDraftParticipants = (options: OptionData[]): SelectedParticipant[] => {
if (!personalData?.login || !personalData?.accountID) {
return [];
}
return [
...options.map((option) => ({
login: option.login,
accountID: option.accountID ?? CONST.DEFAULT_NUMBER_ID,
})),
{login: personalData.login, accountID: personalData.accountID},
];
};
const updateGroupDraft = (newSelectedOptions: OptionData[]) => {
const participants = buildDraftParticipants(newSelectedOptions);
if (!participants.length) {
return;
}
setGroupDraft({participants});
};Then reuse buildDraftParticipants(selectedOptions) inside createGroup as well.
Reviewed at: f5a1a9c | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
@ZhenjaHorbach Friendly bump for review. |
|
@ZhenjaHorbach Friendly bump for review! |
|
Will check today or tomorrow! |
|
@codex |
|
@MelvinBot |
|
🤖 PR Review — overall this is a clean, well-scoped refactor. It removes the per-page option/pagination plumbing in 1. Native testing gap on the PR's headline behavior. The device-contact de-dupe is the substantive behavioral change here, and it is native-only — 2. This changes shared-hook behavior for all consumers, not just Minor points (non-blocking)
Nothing here is a hard blocker on the code itself — the main ask is native verification of the contact de-dupe before merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e51455381c
ℹ️ 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".
|
@ZhenjaHorbach I updated. |
3 participants
Explanation of Change
Fixed Issues
$ #71061
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Screen.Recording.2026-07-13.at.22.25.00.mp4
iOS: Native
iOS: mWeb Safari
Screen.Recording.2026-07-13.at.22.22.34.mp4
MacOS: Chrome / Safari
Screen.Recording.2026-07-13.at.22.11.05.mp4