Fix #88730: show invite-as-user row for unknown emails in SearchRouter#89405
Fix #88730: show invite-as-user row for unknown emails in SearchRouter#8940515antonian wants to merge 8 commits into
Conversation
…earchRouter
canCreateOptimisticPersonalDetailOption used a zero-results gate that
suppressed userToInvite whenever any partial-match contact existed.
Replace with an exact-login-match check so the invite row appears even
when other contacts are present. Label it with translate('common.invite')
in SearchAutocompleteList so it renders as "Invite" rather than the raw
email, matching every other userToInvite surface in the app.
…nsify#88730 Verify that the invite row appears when partial-match contacts exist but none has an exact login match, and that it is suppressed when an exact match is found. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@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] |
|
Web (MacOS Chrome) — after fix: Typing an unknown email now shows the "Invite" row: mWeb (375px viewport) — after fix: Screenshots taken at Both show: typing |
|
All contributors have signed the CLA ✍️ ✅ |
|
@15antonian |
|
But sorry |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57361654b6
ℹ️ 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".
| const hasExactLoginMatch = | ||
| recentReportOptions.some((o) => o.login?.toLowerCase() === normalizedSearchValue) || personalDetailsOptions.some((o) => o.login?.toLowerCase() === normalizedSearchValue); |
There was a problem hiding this comment.
Compare raw phone logins before adding invite row
The new exact-match gate only compares option logins against addSMSDomainIfPhoneNumber(searchValue), which misses existing contacts whose login is stored as raw E.164 (e.g. +15005550006 without @expensify.sms). In that case, typing that exact phone number still passes canCreateOptimisticPersonalDetailOption, so userToInvite is created even though an exact contact already exists, yielding a duplicate/inaccurate “Invite” result. This regression is reachable in flows where phone logins are kept without the SMS suffix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, addressed. Updated canCreateOptimisticPersonalDetailOption to also match logins stored as raw E.164 (no @expensify.sms suffix), and added two regression tests covering both storage formats. Existing tests still pass.
|
I have read the CLA Document and I hereby sign the CLA |
|
And please |
|
Will send the others shortly. |
|
Videos added for all platforms — Android Native, iOS Native, iOS mWeb Safari, and MacOS Chrome. Android mWeb Chrome shares the same code path as iOS mWeb Safari (covered by that recording). |
|
Hi @ZhenjaHorbach, friendly bump for re-review when you have a chance. Happy to record any additional platforms if helpful, just let me know. |
…ersonalDetailOption Expensify#88730
|
@CLAassistant ok recheck |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-05-17.16.15.10.movAndroid: mWeb Chrome2026-05-17.16.24.18.moviOS: HybridApp2026-05-17.16.15.10.moviOS: mWeb Safari2026-05-17.16.24.18.movMacOS: Chrome / Safari2026-05-17.16.07.31.mov |
|
Hi @ZhenjaHorbach, friendly bump to close this one out. I've added the additional check and updated the unit test. Thanks! |
|
Will check today |
|
@CLAassistant ok recheck |
|
Hi @ZhenjaHorbach — I signed the CLA on May 1st by posting the required comment, but the CLA bot failed with: |
|
recheck |
|
I'm working on it |
|
Looks like the CLA workflow passed. |
|
Sorry for delay |
|
Can you look into the failing tests? |
|
I suppose it will be enough to update the branch to the latest version of main to fix perf tests |
|
Yes that was it. Thank you. @ZhenjaHorbach Failiing tests are now passing @AndrewGable |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…n SearchAutocompleteList Covers the branch added in Expensify#88730 where userToInvite is spread with alternateText: translate('common.invite') before being pushed to recentReportsOptions. Closes the Codecov coverage gap on line 377 of SearchAutocompleteList.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Tests are passing locally and I've added a UI test to cover the new code path in SearchAutocompleteList. Ready for re-review. |
|
Tests are failing (eslint) |
|
I think on your next prs you should have the tests run automatically instead of needing to be approved every time so hopefully will prevent the failing test loop. |
|
Last commit was not signed, can you resign it? https://superuser.com/a/428474 |
The ui/ test suite is what Codecov instruments. Adds a case that passes userToInvite via the mocked getSearchOptions and asserts the "Invite" alternateText is rendered, covering line 377 of SearchAutocompleteList.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2bc1455 to
f0db258
Compare
| const styles = useThemeStyles(); | ||
|
|
||
| if (isSearchQueryListItem(props)) { | ||
| return <SearchQueryListItem {...props} />; |
There was a problem hiding this comment.
Why do we need these changes?
There was a problem hiding this comment.
The react/jsx-props-no-spreading ESLint rule forbids JSX prop spreading the {...props} here caused a CI failure.
There was a problem hiding this comment.
Don't see any issues with the pipeline in this place in your previous commits
Do you have this issue locally?
There was a problem hiding this comment.
Ran it locally. npx eslint src/components/Search/SearchAutocompleteList.tsx throws error Prop spreading is forbidden react/jsx-props-no-spreading at line 111. CI also failed on that check in the run right before this fix.
There was a problem hiding this comment.
You're right, the original had an eslint-disable-next-line comment suppressing it. I removed that and replaced the spread with explicit props instead, which is the proper fix rather than suppressing the rule. Happy to revert to the disable comment if preferred.
There was a problem hiding this comment.
Reverted. My bad that eslint-disable comment was introduced by me in this PR, not from main. Reverted to the plain spread


Explanation of Change
Two compounding bugs prevented the "Invite" affordance from appearing when an unknown email is typed in the SearchRouter:
Bug 1 —
canCreateOptimisticPersonalDetailOptionused a zero-results gate (recentReportOptions.length + personalDetailsOptions.length > 0 → return false). This suppresseduserToInviteentirely whenever any partial-match contact existed in the results, even when none of those matches was the typed email address. Replaced with an exact-login-match check:userToInviteis now suppressed only when a contact with that exact login already appears in the results.Bug 2 — Missing "Invite" label. When
searchOptions.userToInvitewas non-null, it was pushed into the flatreportOptionsarray with bothtextandalternateTextset to the raw email (the default fromgetUserToInviteOption). The row was visually indistinguishable from a regular contact. Fixed by spreadingalternateText: translate('common.invite')onto the invite item when appending it, which renders "Invite" in the secondary text position — matching every other surface in the app that surfacesuserToInvite(NewChatPage, RoomInvitePage, etc). No new translation strings needed;common.invitealready exists.Fixed Issues
$ #88730
PROPOSAL: #88730 (comment)
Tests
nobody-test-88730@unknown-domain-xyz.com)Offline tests
The
userToInviterow is constructed from the locally typed query with an optimistic accountID — no network call is needed to render it. Go offline after loading the app, open the SearchRouter, type an unknown email, and verify the invite row still appears.QA Steps
qa-test-88730@expensify-qa-unknown.com)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.mp4
Android: mWeb Chrome
Not tested — see MacOS: Chrome / Safari and iOS: mWeb Safari for mWeb coverage.
iOS: Native
ios.mp4
iOS: mWeb Safari
Not tested — see MacOS: Chrome / Safari and iOS: mWeb Safari for mWeb coverage.
MacOS: Chrome / Safari
web.mp4
mweb.mp4