Skip to content

[CP Staging] Revert "fix: stop Search re-firing an identical query on unrelated Onyx updates" - #98548

Merged
mountiny merged 1 commit into
mainfrom
revert-98305-fix/search-refire-highlight-hook
Aug 13, 2026
Merged

[CP Staging] Revert "fix: stop Search re-firing an identical query on unrelated Onyx updates"#98548
mountiny merged 1 commit into
mainfrom
revert-98305-fix/search-refire-highlight-hook

Conversation

@mountiny

Copy link
Copy Markdown
Contributor

Reverts #98305

Fixed issues

$ #98494
$ #98502
$ #98543
$ #98544

@mountiny mountiny self-assigned this Aug 13, 2026
@mountiny
mountiny requested review from a team as code owners August 13, 2026 07:21
@melvin-bot
melvin-bot Bot requested review from QichenZhu, joekaufmanexpensify and mkhutornyi and removed request for a team and joekaufmanexpensify August 13, 2026 07:21
@melvin-bot

melvin-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

@mkhutornyi @QichenZhu 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]

@melvin-bot
melvin-bot Bot removed the request for review from a team August 13, 2026 07:21

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79be2d8d3e

ℹ️ 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".


// Check if there is a change in the transactions or report actions list
if ((isChat ? hasReportActionsIDsChange : hasTransactionsIDsChange) || hasPendingSearchRef.current) {
if ((!isChat && hasTransactionsIDsChange) || hasReportActionsIDsChange || hasPendingSearchRef.current) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Gate report-action changes to chat searches

For a non-chat search this condition still enters when hasReportActionsIDsChange is true. The Search screen subscribes to the global report-actions collection and passes it into this hook (src/components/Search/index.tsx:170, src/components/Search/index.tsx:234-244), so sending or receiving any new chat action while an expense search page is open re-runs the current expense search even though no transaction changed; because newIDs below is the transaction key list, the later filter cannot recognize the report action as irrelevant. Keep report-action changes inside the chat branch.

Useful? React with 👍 / 👎.

}
}

const newIDs = isChat ? reportActionsIDs : transactionsIDs;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Compare transaction IDs instead of Onyx keys

In the non-chat branch transactionsIDs are Onyx collection keys from ONYXKEYS.COLLECTION.TRANSACTION (for example transactions_123), while extractTransactionIDsFromSearchResults() returns bare transactionID values (for example 123). Comparing these at line 125 makes every existing local transaction look absent from the results, so adding a transaction that is already in the snapshot—or keeping any filtered-out transaction in Onyx—bypasses the “no genuinely new ID” skip and re-fires an identical query. Build newIDs from transaction.transactionID for added/current transactions instead.

Useful? React with 👍 / 👎.

@mountiny
mountiny merged commit 08f4b61 into main Aug 13, 2026
46 of 47 checks passed
@mountiny
mountiny deleted the revert-98305-fix/search-refire-highlight-hook branch August 13, 2026 07:26
@github-actions

Copy link
Copy Markdown
Contributor

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

OSBotify pushed a commit that referenced this pull request Aug 13, 2026
…re-highlight-hook

[CP Staging] Revert "fix: stop Search re-firing an identical query on unrelated Onyx updates"

(cherry picked from commit 08f4b61)

(cherry-picked to staging by mountiny)
@OSBotify OSBotify added the CP Staging marks PRs that have been CP'd to staging label Aug 13, 2026
@OSBotify

Copy link
Copy Markdown
Contributor

🔁 Filed a Staging retest request for deploy blockers #98494, #98502, #98543, #98544 after this PR was cherry-picked to staging.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.53-2 🚀

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

@MelvinBot

Copy link
Copy Markdown
Contributor

Help site review: no changes required

I reviewed the changes in this PR and no updates to Expensify's help site (docs/articles) are needed.

Why: This PR reverts #98305 and only touches internal implementation:

  • src/hooks/useSearchHighlightAndScroll.ts — the logic controlling when Search re-fires an identical query on Onyx updates (a behind-the-scenes highlight/scroll hook).
  • tests/unit/useSearchHighlightAndScrollTest.ts — the corresponding unit tests.

There is no change to any user-facing feature, UI label, tab, setting, button, or workflow that the help site documents. The Search feature's behavior from a user's perspective is unchanged — this is purely about the internal condition under which the query re-fires. Since no HelpDot article describes this internal behavior, there is nothing to document, so I did not create a draft docs PR.

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

Labels

CP Staging marks PRs that have been CP'd to staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants