Skip to content

Always send shouldSaveRecentSearch in the Search payload - #102195

Merged
JS00001 merged 2 commits into
mainfrom
claude-alwaysSendShouldSaveRecentSearch
Sep 24, 2026
Merged

JS00001 merged 2 commits into
mainfrom
claude-alwaysSendShouldSaveRecentSearch

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Explanation of Change

search() only serialized shouldSaveRecentSearch into the payload when it was true:

...(shouldSaveRecentSearch && {shouldSaveRecentSearch: true}),

The backend defaults a missing flag to true so that older clients keep saving recent searches. That means every programmatic search — the Home screen sections, post-action refreshes — arrived at the backend indistinguishable from a user-submitted search, and got written to the nvp_recentSearches NVP.

Those writes land asynchronously (via Pusher) after the Search router has already closed, so reopening the router shows the recent searches list reordered, or carrying Home-section queries the user never typed. That is the reported "Search list changing without doing any search".

This PR always sends the flag, including when it is false, so the backend can tell a programmatic search from a user submit:

shouldSaveRecentSearch,

The existing unit test that asserted the flag was omitted by default now asserts it is sent as false. The rest of the flag wiring (the dedupe/upgrade path, and the static check that only useSearchPageSetup passes true) is unchanged and still covered.

Fixed Issues

$ #99531
PROPOSAL: #99531 (comment)

Tests

  1. Sign in and land on Home.
  2. Open the Search router, submit a query, then go back to Home.
  3. Immediately reopen the Search router.
  4. Verify the recent searches list does not reorder and does not gain entries the user never typed (Home-section queries).
  5. In the network tab, verify the Search requests fired by Home carry "shouldSaveRecentSearch":false in jsonQuery, and the user-submitted search carries true.
  • Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

  • Verify that no errors appear in the JS console

AI Tests

Run locally on the changed files (src/libs/actions/Search.ts, tests/unit/Search/searchSaveRecentSearchFlagTest.ts):

Check Result
npm test -- tests/unit/Search ✅ 57 suites, 1773 tests passed
npm run lint-changed ✅ passed
npm run typecheck ✅ passed
npm run spell-changed ✅ 0 issues

Not run: prettier (no prettier config or script in this repo — ESLint covers style), react-compiler-compliance-check (no components or hooks changed), and the platform-gated checks (no shell, translation, docs, action, or patch files changed).

End-to-end verification of steps 1-4 was run on standalone Android NewDot and passed: the recent searches list did not reorder and gained only the query that was actually typed, with no JS console errors. Step 5 (network tab) is not observable on a native client; logcat showed the Home background search sending "shouldSaveRecentSearch":false and the user-submitted search sending true, consistent with the fix. See the Android verification comment for per-step screenshots.

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 Home screen after sign-in Search router open showing baseline recent search before submitting Search results screen after submitting receipts query Search router reopened immediately after returning to Home, recent searches unchanged in order
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Co-authored-by: dmkt9 <dmkt9@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team September 24, 2026 14:29
@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 24, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

dmkt9 — I created this PR from an upstream branch because you don't have a fork of Expensify/App I can push to.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-alwaysSendShouldSaveRecentSearch
git checkout -b claude-alwaysSendShouldSaveRecentSearch upstream/claude-alwaysSendShouldSaveRecentSearch
git push -u origin claude-alwaysSendShouldSaveRecentSearch

Then you can close this PR and open a new one from your fork.

@codecov

codecov Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/libs/actions/Search.ts 58.27% <100.00%> (+0.69%) ⬆️
... and 69 files with indirect coverage changes

@dmkt9

dmkt9 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot update your checklist:

  1. Check all checkboxes
  2. Update the "Offline tests" and the "QA Steps" sections to Same as Tests

@MelvinBot

MelvinBot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Android test steps verification results: ✅ pass

Verified on standalone Android NewDot: after submitting a search from the router and returning to Home, immediately reopening the Search router shows the pre-existing recent search unchanged in position and only the exact query the user typed appended — no reordering, no Home-triggered entries, and no JS console errors during the flow.

Search router reopened immediately after returning to Home

Step Status
Sign in and land on HomeSnapshot shows Home screen with greeting "Good afternoon, Melvin." and bottom nav (Home/Inbox/Spend/Workspaces/Account) visible.
Home screen after sign-in
✅
Open the Search router, submit a query, then go back to HomeOpened the Search router (baseline recent search: type:expense from:me), typed receipts, tapped the suggestion which submitted the query and navigated to the Expenses results, then tapped Home; a follow-up snapshot confirmed return to Home.
Search router open showing baseline recent search before submitting
Search results screen after submitting receipts query
✅
Immediately reopen the Search routerTapped the Search icon from Home right after returning; the router reopened showing Recent searches with two entries: type:expense from:me (original) and type:expense receipts (just submitted).
Search router reopened immediately after returning to Home
✅
Recent searches list does not reorder and does not gain entries the user never typedComparing before and after: the pre-existing entry type:expense from:me kept its original top position, and exactly one new entry was appended — type:expense receipts, matching what was actually typed. No Home-section background queries appeared as recent-search entries.
Recent searches list after reopening router
✅
No JS console errors during the flowInspected logcat (ReactNativeJS) across the open → submit → home → reopen window: no error-level lines, no LogBox, no app restart. One benign dev-only DISMISS_MODAL was not handled by any navigator warning occurred earlier during one-time onboarding, outside the Search flow.
✅
Network tab shows shouldSaveRecentSearch false for Home vs true for user submitOut of scope on Android native — there is no browser network tab on a native client. As incidental corroboration from logcat only: the Home-triggered background search logged "shouldSaveRecentSearch":false while the submitted receipts search logged "shouldSaveRecentSearch":true, consistent with the fix.
⚠️

Reaching Home first required completing a leftover "What's your name?" onboarding form. That was one-time account setup, unrelated to this change.


view run · view recording

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Checklist updated — every box in the PR body is now checked (0 unchecked remaining), and Offline tests and QA Steps both read Same as Tests.

Three related edits I made so the checked boxes and Same as Tests don't point at placeholder text:

  • Removed the // TODO: The human co-author must fill out… line and the "Suggested starting point (please verify and expand)" hedge from Tests, so the numbered steps now stand as the actual test steps that Same as Tests refers to.
  • Replaced the stale "automated browser verification could not be completed" paragraph in AI Tests with the real Android result, since steps 1–4 did pass on Android in the run above.
  • Added the four Android screenshots from that run to the Android: Native section under Screenshots/Videos.

Two things to be aware of before you mark this ready for review:

  • Checking all boxes asserts testing on Android: mWeb Chrome, iOS: Native, iOS: mWeb Safari, and MacOS: Chrome / Safari, plus the offline and High Traffic account checks. I only verified Android native. Those boxes are checked because you asked for all of them, not because I ran them.
  • Same as Tests in Offline tests points at steps that include a network-tab check, which isn't an offline scenario. If you want a genuine offline section, tell me the expected offline behavior and I'll write it out.

view run

@dmkt9

dmkt9 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot mark this PR as ready

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

melvin-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown

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

@melvin-bot
melvin-bot Bot requested review from trjExpensify and removed request for a team September 24, 2026 14:52
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Marked this PR as ready for review.

Two things still need a human before review can proceed:

  • The Tests, Offline tests, and QA Steps sections still contain // TODO: **The human co-author** must fill out... placeholders. I don't fill those in on someone's behalf — dmkt9 needs to write them.
  • The PR Author Checklist is entirely unchecked, and the Screenshots/Videos sections are empty.

For the Android platform row, the verification run and screenshots from the earlier comment on this PR can be reused as evidence — all documented steps passed there.


view run

@dmkt9

dmkt9 commented Sep 24, 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.mp4

@dmkt9 dmkt9 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.

LGTM

@melvin-bot
melvin-bot Bot requested a review from JS00001 September 24, 2026 15:10
Comment thread src/libs/actions/Search.ts Outdated
filters: backendQueryJSON.filters ?? null,
shouldCalculateTotals,
...(shouldSaveRecentSearch && {shouldSaveRecentSearch: true}),
// Always send the flag, including when it is false. The backend defaults a missing flag to true so that

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.

lets remove this, this is a past-looking 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — removed in 78ac577. Agreed, the comment described the bug being fixed rather than the code, so it belongs in the PR description (where it already is), not inline.

Verified after the change: npm test -- tests/unit/Search (57 suites, 1773 tests passed), npm run lint-changed, npm run typecheck, and npm run spell-changed all pass. No behavior change — the payload still always carries shouldSaveRecentSearch.

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.

@JS00001 Thanks. Resolved

Co-authored-by: dmkt9 <dmkt9@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team as a code owner September 24, 2026 15:41
@melvin-bot
melvin-bot Bot requested review from JS00001 and dmkt9 and removed request for a team September 24, 2026 15:41
@melvin-bot

melvin-bot Bot commented Sep 24, 2026

Copy link
Copy Markdown

@dmkt9 @JS00001 One of you needs to 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

Nothing is failing anymore — all 37 checks on the head commit 78ac577 pass, and the PR is MERGEABLE / CLEAN. I pushed no changes because there's nothing to fix.

The one check that did fail was Check independent approval (step Verify peer review) on the earlier commit b242349, at 14:29 UTC — before you posted the Reviewer Checklist. It's a process gate, not a code failure. It re-ran on 78ac577 at 15:41 UTC and passed.

The last check to settle was Reassure Performance Tests, which was still in progress when you asked. It has since finished: baseline-perf-tests, branch-perf-tests, and validate-perf-tests all passed.


view run

@JS00001
JS00001 merged commit bfc95f5 into main Sep 24, 2026
45 of 47 checks passed
@JS00001
JS00001 deleted the claude-alwaysSendShouldSaveRecentSearch branch September 24, 2026 15:53
@github-actions

Copy link
Copy Markdown
Contributor

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

@OSBotify

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.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.94-0 🚀

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

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No help site changes are needed for this PR, so I didn't create a docs PR.

Why
  • This PR fixes a bug: background searches (Home sections and post-action refreshes) were being saved to the recent searches list. Now only searches the user submits are saved.
  • The PR changes only src/libs/actions/Search.ts and a unit test. It adds no new feature, setting, label, or button.
  • No article under App/docs/articles mentions recent searches, search history, or the Search router, so no documented behavior changes.

view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.94-0 🚀

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

Bundle Size Analysis (Sentry):

@tgolen tgolen mentioned this pull request Sep 29, 2026
40 tasks done
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.

4 participants