Skip to content

Display SearchForReports results in Auth's tier order#94362

Draft
carlosmiceli wants to merge 5 commits into
mainfrom
cm-searchforreports-result-order
Draft

Display SearchForReports results in Auth's tier order#94362
carlosmiceli wants to merge 5 commits into
mainfrom
cm-searchforreports-result-order

Conversation

@carlosmiceli

Copy link
Copy Markdown
Contributor

Explanation of Change

NewDot's chat-finder builds its result order entirely client-side (report kind + recency) and never read the order Auth returns for SearchForReports, so the backend's tier ranking (self → primary workspace → primary domain → other shared workspaces → everyone else) was discarded.

With the backend now sending an ordered searchResultReportIDs list (companion Web-E PR), this:

  • Adds a RAM-only RAM_ONLY_SEARCH_RESULT_REPORT_IDS Onyx key for that list.
  • Orders the chat-finder's results by it when present.
  • When a server order exists, renders a single list in that order instead of the "Recent chats" / "Search results" two-section split. The split grouped locally-cached matches separately and broke the global tier order — e.g. workspace rooms like #admins were pushed to the bottom by the client kind-sort even though Auth ranks them near the top. The cache is still used for instant display; the list only visibly reorders when the cached order disagrees with Auth (a no-op when they already match).

Companion PRs:

Note: this changes the two-section "stable chat switcher" behavior (claude-stableTwoSectionChatSwitcherV2) when a server order is present — flagging for that area's reviewers.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/530969
PROPOSAL:

Tests

  1. Open the search router (Cmd+K).
  2. Search a term matching chats across multiple tiers (your primary workspace + DMs + other workspaces).
  3. Verify results display in the backend's tier order: self, then your primary workspace chats (including rooms like #admins), then same-domain DMs, then other-workspace chats — not by recency, and rooms are not pushed to the bottom.
  4. Verify the list updates to that order once the search returns, and does not jump when the cached order already matches.
  • Verify that no errors appear in the JS console

Offline tests

  1. Go offline, open the search router, type a query.
  2. Verify locally-known chats still appear (no server order is fetched offline) and no errors occur.

QA Steps

  1. On staging, with an account that has chats in multiple workspaces and DMs, open the search router and search a term matching several of them.
  2. Verify results appear in tier order (self → primary workspace → same-domain DMs → other workspaces), with workspace rooms not sinking to the bottom.
  • 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions
  • I verified the change does not regress the getValidOptions/search options used elsewhere (mentions, share, search filters)

@carlosmiceli

Copy link
Copy Markdown
Contributor Author

@codex review

@carlosmiceli

Copy link
Copy Markdown
Contributor Author

@MelvinBot fix eslint

Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Fixed the failing ESLint check.

The error was an unnecessary type assertion in tests/unit/SearchAutocompleteListTest.tsx:476 (@typescript-eslint/no-unnecessary-type-assertion): the typeof el.props.children === 'string' check already narrows the type to string, so the as string cast was redundant. Removed it.

Verified locally with ./scripts/lint.sh on all changed files — clean.

@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: 9c2c1d4039

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


// When the server has returned a tier-ranked order for this search, display results in that order
// instead of the client-side kind/recency order. Reports absent from the list sort to the end.
if (searchResultReportIDs && searchResultReportIDs.length > 0) {

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 Clear stale server order on new searches

This applies any non-empty RAM_ONLY_SEARCH_RESULT_REPORT_IDS to every non-empty autocomplete query, but the new RAM-only key is global and is not cleared when autocompleteQueryValue changes or when a new SearchForReports starts/fails. After one Cmd+K search receives an Auth order, typing a different query (or searching while offline/failing before a new response arrives) reuses the previous query's IDs, incorrectly reordering overlapping reports and switching the UI into the single "Search results" section before the current query has a server order. The order needs to be tied to the active query or cleared before starting/aborting a new search.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants