Skip to content

Trim search term before deriving the phone/email search value in useSearchSelector - #100031

Merged
mountiny merged 3 commits into
mainfrom
claude-trimSearchTermForInviteOption
Sep 9, 2026
Merged

mountiny merged 3 commits into
mainfrom
claude-trimSearchTermForInviteOption

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Fixed Issues

$ #99419
PROPOSAL: #99419 (comment)

Tests

Same as QA steps

  • Verify that no errors appear in the JS console

Offline tests

Same as QA steps

QA Steps

  1. Navigate to staging.new.expensify.com.
  2. Navigate to Start chat.
  3. Enter a valid email address for a user who does not have an account, including a space before or after the email.
  4. Verify the user doesn't disappear and remain visible in the suggestion list.
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

…earchSelector

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team September 1, 2026 12:55
@melvin-bot melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Sep 1, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Android test steps verification results: ✅ pass

PR #100031's search-term trim fix verified end-to-end on Android: leading/trailing-space email and phone searches still surface the invite row in both Start Chat and the Submit Expense participant picker, and selecting the padded row always yields a trimmed login.

Phone invite row in Start chat

Step Status
Open Start chat from the FABFAB > Start chat opened NewChatPage with a focused 'Name, email, or phone number' field.
✅
Type email with no Expensify account (notanaccount99419@example.com); invite row appearsInvite row rendered with contact avatar, email text, and 'Add to group' action.
Invite row for plain email in Start chat
✅
Leading space before email; invite row still shownField byte-verified to hold ' notanaccount99419@example.com' (leading space); invite row remained present/unchanged.
Invite row still shown with leading space
✅
Trailing space after email; invite row still shownField byte-verified to hold 'notanaccount99419@example.com ' (trailing space); invite row remained present/unchanged.
Invite row still shown with trailing space
✅
Select invite row from space-padded search; chat opens with trimmed emailChat opened with header and body text 'notanaccount99419@example.com' (no space present).
Chat opened with trimmed email login
✅
Repeat steps 2-5 in the Submit expense participant picker (same hook)Choose recipient picker showed the contact/invite row for plain, leading-space, and trailing-space email searches (byte-verified); selecting the padded row advanced to Confirm details with participant shown as 'notanaccount99419@example.com', no space.
Participant picker plain email
Participant picker leading space
Participant picker trailing space
Confirm details showing trimmed participant email
✅
Space-padded phone number ( +1 (234) 567-8901 ) still surfaces the invite rowField byte-verified to hold the leading/trailing spaces; invite row rendered as '+12345678901 / +1 234-567-8901' in the Submit expense picker and 'Select, +1 234-567-8901' in Start chat.
Phone invite row in Submit expense participant picker
Phone invite row in Start chat
✅

No console errors were observed.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Web test steps verification results: ✅ pass

Verified on dev NewDot web: leading/trailing spaces no longer hide the email/phone invite row in Start chat and in the Submit-expense participant picker, and selecting a space-padded invite correctly opens/confirms with the trimmed value. Confirms the useSearchSelector fix in this PR works end-to-end.

Invite row surfaces for space-padded phone number in Choose recipient

Step Status
1. Open Start chat from the FABClicked the global "Open actions menu" FAB then "Start chat"; snapshot showed the "Start chat" heading with the Name/email/phone search field.
✅
2. Type notanaccount99419@example.com (no-account user) in Start chat search - invite row appearsAfter filling the search field, a Contacts row "notanaccount99419@example.com" with "Add to group" appeared (button label "Select, notanaccount99419@example.com").
Invite row appears for notanaccount99419@example.com in Start chat
✅
3. Add a leading space before the email in Start chat search - invite row still shownFilled field with " notanaccount99419@example.com"; settle diff showed 0 changes - the invite option/row remained present, did not disappear.
Invite row persists with a leading space in Start chat search
✅
4. Replace with a trailing space after the email in Start chat search - invite row still shownFilled field with "notanaccount99419@example.com "; settle diff showed 0 changes - invite row remained present.
Invite row persists with a trailing space in Start chat search
✅
5. Select the invite row from the space-padded search in Start chat - chat opens with trimmed email (no space in login)Clicked the invite option; app navigated to a 1:1 chat whose header link/label read exactly "notanaccount99419@example.com" (accessibility attrs confirmed no leading/trailing whitespace in the label).
Chat opened with trimmed login
✅
6. Repeat steps 2-5 in the Submit expense participant picker (Create expense -> Choose recipient), which uses the same hookIn Choose recipient: plain email produced an invite option/radio for notanaccount99419@example.com (step 2); leading space kept the option/radio present with 0-diff settle (step 3); trailing space likewise kept it present with 0-diff settle (step 4); selecting the invite option advanced to Confirm details, which showed the recipient as "notanaccount99419@example.com" with no stray space (step 5).
Invite row appears in Choose recipient
Invite row persists with leading space in Choose recipient
Invite row persists with trailing space in Choose recipient
Confirm details shows trimmed recipient
✅
7. A space-padded phone number (" +1 (234) 567-8901 ") still surfaces the invite rowIn Choose recipient, filling " +1 (234) 567-8901 " produced an invite option/radio "+12345678901" (displayed as "+1 234-567-8901") - the padded phone number resolved to e164 form and surfaced the invite row, matching the PR's described side effect.
Invite row surfaces for space-padded phone number in Choose recipient
✅

No console errors were observed.


view run

@eh2077

eh2077 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot workspace invite new member search input has same issue, please fix it as well

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

No fix needed here — main is broken, not this PR. I did not push anything.

The only real failure is typecheck, and it points at src/hooks/useTransactionInlineEdit.ts:98 — a file this PR doesn't touch:

src/hooks/useTransactionInlineEdit.ts(98,32): error TS2304: Cannot find name 'originalUseOnyx'.
src/hooks/useTransactionInlineEdit.ts(100,32): error TS2304: Cannot find name 'originalUseOnyx'.
src/hooks/useTransactionInlineEdit.ts(132,43): error TS2304: Cannot find name 'originalUseOnyx'.

A fix is already open: #100314 ("[No QA] Fix live Onyx hook references" by puneetlath). Once it merges, re-run this PR's typecheck and it should go green — no change is needed on this branch.

Why this is a broken main, not a PR regression

Semantic merge conflict between two PRs that merged cleanly at the text level.

  1. Commit 4becd4f4 (2026-09-02 16:13 UTC) renamed the import app-wide and updated every call site that existed at the time:

    -import {useOnyx as originalUseOnyx} from 'react-native-onyx';
    +import {useOnyx as useOnyxWithoutSnapshots} from 'react-native-onyx';
  2. Refactor TransactionInlineEdit actions into pure functions (remove connectWithoutView) #99279 merged today at 14:45 UTC as c408634c and added three new call sites still using the old alias:

    +    const [liveParentReport] = originalUseOnyx(...)
    +    const [liveSelfDMReport] = originalUseOnyx(...)
    +    const [liveTransactionThreadReport] = originalUseOnyx(...)
    

Git had no textual overlap to conflict on, so both landed. main at ea45c1c now imports the hook as useOnyxWithoutSnapshots on line 33 while three lines still reference the undefined originalUseOnyx. The typecheck workflow only runs on PRs, so nothing caught this on main itself.

Scope check: this PR changes only src/hooks/useSearchSelector/base.ts, src/hooks/usePersonalDetailSearchSelector/base.ts, and their two test files. No overlap with the failing file.

Other checks: all 8 test jobs, Bun tests, Storybook, spellcheck, Oxfmt, React Compiler Compliance, knip and the builds pass. ESLint check and branch-perf-tests were still running when I looked. Check independent approval fails because the PR has no independent approval yet — expected, not a code problem.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Fixed and pushed — b7d31cf.

Root cause: workspace Invite member goes through a different hook, usePersonalDetailSearchSelector, which passed the raw debounced input straight through as searchString at src/hooks/usePersonalDetailSearchSelector/base.ts:229. From there it reaches the same anchored-regex guard as the original bug, so a boundary space killed the invite row.

Fix: trim once in the hook and pass the trimmed value as searchString — base.ts:223, base.ts:233. Same shape as the useSearchSelector fix, and it covers every consumer of the hook, not just workspace invite.

I also updated the PR description with the second root cause and added test steps 8-14 for the Invite member flow.

⚠️ Not browser-verified. The CI browser session was stuck on a blank page (0 DOM nodes) after sign-in again, same harness failure as the earlier run, so I could not drive the Invite member flow. The dev server was healthy (HTTP 200, all bundles served). Please run steps 8-13 manually before marking this ready for review.

Still blank white page after ~5 minutes of waiting

Evidence chain and tests

The chain

searchString is forwarded verbatim as searchValue into filterUserToInvite → getUserToInviteOption. There Str.isValidEmail uses an anchored ^…$ regex, so ' a@b.com' fails; with isValidEmail === false, isValidPhoneNumber === false and shouldAcceptName === false the guard returns null and no invite option is built. Existing contacts kept matching because that path goes through processSearchString, which splits on spaces and drops empty tokens — the same asymmetry as the original report.

Space-padded phone numbers were broken here too, for a second reason: this hook never called getSearchValueForPhoneOrEmail, and appendCountryCode doesn't see a leading + on ' +1 (234) 567-8901 ', so it built '+1 +1 (234) 567-8901', failed to parse, and produced no invite row. The trim fixes that as well.

Also covered by this one change (all consumers of the hook): DynamicWorkspaceInvitePage, DynamicRoomInvitePage, DynamicReportParticipantsInvitePage, AddDelegatePage, DomainAddAdminPage, MoneyRequestAttendeeSelector, DynamicTaskAssigneeSelectorModal, both AssigneeSteps, WorkspaceConfirmationOwnerSelectorPage, DynamicWorkspaceWorkflowsApprovalsExpensesFromPage, BaseVacationDelegateSelectionComponent, and Search's UserSelector.

Tests

4 new unit tests in tests/unit/usePersonalDetailSearchSelectorTest.tsx (leading space, trailing space, space-padded phone, whitespace-only). The first three were confirmed to fail against the unfixed code; the fourth asserts a whitespace-only search still yields no invite row, which held before and after.

Check Result
npm test -- tests/unit/usePersonalDetailSearchSelectorTest.tsx tests/unit/useSearchSelectorTest.tsx tests/ui/BaseVacationDelegateSelectionComponentTest.tsx tests/ui/AssignCardAssigneeStepTest.tsx tests/ui/UserSelectorTest.tsx tests/ui/WorkspaceInviteMessageApproverPageTest.tsx tests/unit/components/BaseVacationDelegateSelectionComponentTest.tsx tests/unit/pages/settings/ConfirmDelegatePageTest.tsx tests/actions/PolicyMemberTest.ts ✅ 118 passed
npx eslint on the two changed files ✅ pass
npm run typecheck ✅ pass
npm run react-compiler-compliance-check check src/hooks/usePersonalDetailSearchSelector/base.ts ✅ pass
npm run fmt (oxfmt) ✅ no changes
npm run spell-changed ✅ 0 issues
Browser verification ❌ blocked by the harness failure above

npm run lint-changed couldn't run here — it needs git merge-base origin/main HEAD and this checkout has no shared history with origin/main. I ran npx eslint on the changed files directly instead.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Update — still nothing to fix on this branch. I re-checked; main is still broken and I did not push anything.

Since my earlier comment, ESLint check has also failed — same file, same root cause as typecheck:

src/hooks/useTransactionInlineEdit.ts
   98:11  error  Unsafe assignment of an error typed value         @typescript-eslint/no-unsafe-assignment
   98:32  error  Unsafe call of a type that could not be resolved  @typescript-eslint/no-unsafe-call
  100:11  100:32  101:11  132:11  132:43  199:13  201:13
✖ 9 problems

Those are all downstream of the same undefined originalUseOnyx — the type-aware lint rules can't resolve the call, so every value derived from it becomes error-typed. One fix clears both checks.

Current state:

To unblock this PR: get #100314 reviewed and merged, then re-run typecheck and ESLint check here. No change is needed on claude-trimSearchTermForInviteOption, which touches only the two search-selector hooks and their tests.

Root cause recap

A semantic merge conflict between two PRs that merged cleanly at the text level:

  1. Commit 4becd4f4 (2026-09-02 16:13 UTC) renamed the import app-wide, useOnyx as originalUseOnyx → useOnyx as useOnyxWithoutSnapshots, updating every call site that existed then.
  2. Refactor TransactionInlineEdit actions into pure functions (remove connectWithoutView) #99279, merged 2026-09-03 14:45 UTC as c408634c, added three new call sites still using the old alias.

Git saw no textual overlap, so both landed. typecheck and the type-aware ESLint rules only run on PRs, so nothing caught it on main itself.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Still nothing to fix on this branch — main is broken. I did not push anything.

ESLint check has now failed too (it was still running when I last commented). Both failures come from the same cause, and all of them land in a file this PR does not touch:

Check Errors Location
typecheck 3 × TS2304: Cannot find name 'originalUseOnyx' useTransactionInlineEdit.ts:98, :100, :132
ESLint check 9 × no-unsafe-assignment / no-unsafe-call same file, lines 98, 100, 101, 132, 199, 201

The ESLint errors are downstream of the typecheck ones — when originalUseOnyx can't be resolved, its return value becomes error-typed, so the assignments and every consumer of those values get flagged.

Fix is still open, not merged: #100314 ("[No QA] Fix live Onyx hook references" by puneetlath). main at e2918fc still contains all three broken references. Once #100314 merges, re-run typecheck and ESLint check here and both should clear — no change is needed on this branch.

Why this is a broken main, not a PR regression

Semantic merge conflict between two PRs that had no textual overlap.

  1. [Payment due @ahmedGaber93] [No QA] Rename originalUseOnyx to useOnyxWithoutSnapshots #100200 ("[No QA] Rename originalUseOnyx to useOnyxWithoutSnapshots") merged 2026-09-02 19:59 UTC. It renamed the import and updated every call site that existed at the time:

    -import {useOnyx as originalUseOnyx} from 'react-native-onyx';
    +import {useOnyx as useOnyxWithoutSnapshots} from 'react-native-onyx';
  2. Refactor TransactionInlineEdit actions into pure functions (remove connectWithoutView) #99279 ("Refactor TransactionInlineEdit actions into pure functions") merged 2026-09-03 14:45 UTC as c408634c. It added three new call sites still using the old alias:

    +    const [liveParentReport] = originalUseOnyx(...)
    +    const [liveSelfDMReport] = originalUseOnyx(...)
    +    const [liveTransactionThreadReport] = originalUseOnyx(...)
    

Git saw no conflicting lines, so both landed. main now imports the hook as useOnyxWithoutSnapshots on line 33 while three lines still call the undefined originalUseOnyx. typecheck and ESLint check only run on PRs, so nothing caught this on main itself.

Scope check: this PR changes only src/hooks/useSearchSelector/base.ts, src/hooks/usePersonalDetailSearchSelector/base.ts, and their two test files. Zero overlap with the failing file.

Everything else is green: all 8 test jobs, Bun tests, Storybook, spellcheck, Oxfmt, React Compiler Compliance, knip, unused-styles and all builds pass. Check independent approval fails only because the PR has no independent approval yet — expected, not a code problem.


view run

@eh2077

eh2077 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot pull main

Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Pulled main into claude-trimSearchTermForInviteOption.

Merge commit: 7138df1 (merged f68d487 — Update Mobile-Expensify submodule version to 9.4.73-1). No conflicts, and the PR's four changed files are untouched by the merge.

@eh2077

eh2077 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixed Issues

$ #99419
PROPOSAL: #99419 (comment)

Tests

Same as QA steps

  • Verify that no errors appear in the JS console

Offline tests

Same as QA steps

QA Steps

  1. Navigate to staging.new.expensify.com.
  2. Navigate to Start chat.
  3. Enter a valid email address for a user who does not have an account, including a space before or after the email.
  4. Verify the user doesn't disappear and remain visible in the suggestion list.
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

@eh2077

eh2077 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot apply #100031 (comment) to update PR description, and then mark PR as ready for review

@MelvinBot
MelvinBot marked this pull request as ready for review September 7, 2026 14:23
@MelvinBot
MelvinBot requested a review from a team as a code owner September 7, 2026 14:23
@melvin-bot
melvin-bot Bot requested review from eh2077 and removed request for a team September 7, 2026 14:23
@melvin-bot

melvin-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

@eh2077 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]

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done.

  • PR description replaced with the content from your comment, byte-for-byte (9268 chars).
  • PR is out of draft and marked ready for review.

@melvin-bot
melvin-bot Bot removed the request for review from a team September 7, 2026 14:23
@eh2077

eh2077 commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Screen.Recording.2026-09-07.at.10.15.14.PM.mov
Android: mWeb Chrome
Screen.Recording.2026-09-07.at.10.14.00.PM.mov
iOS: HybridApp
Screen.Recording.2026-09-07.at.10.12.21.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-09-07.at.10.10.30.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-09-07.at.9.47.30.PM.mov

@eh2077 eh2077 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well

@melvin-bot
melvin-bot Bot requested a review from mountiny September 7, 2026 14:31

@joekaufmanexpensify joekaufmanexpensify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for product.

@eh2077

eh2077 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

@mountiny it's your call, thx!

@mountiny
mountiny merged commit 56f257b into main Sep 9, 2026
48 of 50 checks passed
@mountiny
mountiny deleted the claude-trimSearchTermForInviteOption branch September 9, 2026 13:07
@OSBotify

OSBotify commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify OSBotify mentioned this pull request Sep 9, 2026
83 tasks done
@OSBotify

OSBotify commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.75-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 No help site changes are required for this PR.

This is an internal bug fix, not a behavior or copy change. It trims leading/trailing whitespace from the search term before getSearchValueForPhoneOrEmail / getUserToInviteOption run, so a pasted email with a stray space no longer fails Str.isValidEmail and drops the invite option.

The help site already documents the intended behavior this PR restores:

  1. Enter the name, email, or phone number of the person you want to chat with.

— docs/articles/new-expensify/chat/Create-a-New-Chat.md:21

Why no article needs editing

What changed (source only, plus tests):

Docs criteria checked:

  • No new or renamed feature, tab, setting, or button — nothing to add or rename under HELPSITE_NAMING_CONVENTIONS.md.
  • No user-facing copy or label changed — no article text goes stale.
  • No workflow step added or removed — the Start chat steps in Create-a-New-Chat.md are unchanged.
  • The old buggy behavior was never documented, so there is no incorrect statement or workaround to remove.

I also checked the other articles that reference Start chat — Create-an-Expense.md and Concierge-Basics.md — and neither describes email entry in a way this fix affects.

Because no changes are required, I did not create a draft help site PR, so there is nothing to label HelpDot or assign.

@eh2077, please confirm you agree that no help site update is needed here. If you'd rather I document something specific (for example, an FAQ entry about pasting an email with extra spaces), reply with @MelvinBot and what you want covered, and I'll open the draft PR.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 failure ❌
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 failure ❌
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants