Display SearchForReports results in proper tiers order - #94362
Display SearchForReports results in proper tiers order#94362carlosmiceli wants to merge 20 commits into
Conversation
|
@codex review |
|
@MelvinBot fix eslint |
Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
🤖 Fixed the failing ESLint check. The error was an unnecessary type assertion in Verified locally with |
There was a problem hiding this comment.
💡 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".
…' into cm-searchforreports-result-order
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@sobitneupane 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] |
|
@sobitneupane this will not work fully until we deploy the BE PRs, but it should also be backwards compatible, so let's test that too. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e2f2669c5
ℹ️ 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".
|
👍 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Screen.Recording.2026-06-29.at.09.29.25.mov@carlosmiceli Tag me when the backend PR is merged. I'll review this PR again. |
|
@huult sounds good! Thanks! |
|
@carlosmiceli Tag me when the backend is ready. |
|
@carlosmiceli The unit test is failing. Could you take a look? |
|
@carlosmiceli Tag me when the backend is ready. |
|
@carlosmiceli Can I ask if the backend changes are ready yet? If so, can I re-review this PR? |
|
@carlosmiceli Could you sync this branch with main? |
|
@carlosmiceli could you please sync with main to resolve the conflict? |
|
@MelvinBot please merge main and fix conflicts. @huult sorry for being slow here, we're still needing to confirm some BE improvements before we can proceed with the App PR. Will let you know when it's ready to review 🙇 |
…esult-order Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com> # Conflicts: # src/components/Search/SearchAutocompleteList.tsx
|
🤖 Merged There was one conflict, in The PR now shows as mergeable. Note on local validationI couldn't run the local lint/typecheck suite in this environment, so I verified the resolution statically: both dependencies are in scope ( |
| } | ||
| } else { | ||
| // Active search: split rows into local (frozen order) and server sections. | ||
| // Active search without a server order yet: split rows into local (frozen order) and server sections. |
There was a problem hiding this comment.
Can we remove this else branch?
There was a problem hiding this comment.
I think there's a risk that rows already on screen get reshuffled while the user is already seeing reports stored locally, but maybe @huult can confirm.
|
@MelvinBot merge main and fix tests if failing. |
|
@huult the BE PRs have been merged, but still not deployed, will let you know when they are so you can start reviewing/testing. |
…esult-order Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
🤖 Merged
CI will now run the full suite on the updated branch. |
Thanks! I’ll review the PR once the backend is ready. |
Explanation of Change
The chat-finder ordered results entirely client-side (report kind + recency) and ignored the order
SearchForReportsreturns, so the backend's tier ranking was discarded.The backend now returns an ordered
searchResultReportIDslist reflecting its search tiers:This PR orders the chat-finder by that list when present, and renders the selfDM first (it's gets it from Onyxand the rest as a single ordered list.
Screen.Recording.2026-06-25.at.8.56.37.AM.mov
Fixed Issues
https://github.com/Expensify/Expensify/issues/530969
PROPOSAL:
Tests
#admins), then same-domain DMs, then other-workspace chats, then other DMs.Offline tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari