Sort top categories/merchants insights by groupTotal desc#84424
Sort top categories/merchants insights by groupTotal desc#84424
Conversation
The default insight search queries for "Top categories" and "Top merchants" were sorting alphabetically (by groupCategory/groupMerchant ASC) instead of by total spend descending. This changes them to always sort by groupTotal DESC, which is the expected behavior for "Top X" reports. Co-authored-by: Neil Marcellini <neil-marcellini@users.noreply.github.com>
Updates the sort default tests for Top Categories and Top Merchants to assert the corrected sort behavior (groupTotal desc). Co-authored-by: Neil Marcellini <neil-marcellini@users.noreply.github.com>
|
@MelvinBot please test this add add screenshots for Chrome to the PR description |
|
I tested this PR in Chrome and updated the PR description with screenshots in the MacOS: Chrome / Safari section. Here's what was verified: Test results — all passing:
The |
|
@MelvinBot you need to create some example expenses from the last month so that the insights tabs populate and data will be returned. The expenses should be from multiple categories and merchants. Pls re-test. |
|
I ran out of time before I could finish helping you. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in the #melvinbot-ai. Run details: https://github.com/Expensify/Expensify/actions/runs/22773896227 |
Instead of modifying parser defaults, fix shouldResetSort to only reset sort when groupBy changes. View-only changes (e.g. bar → table) are a presentation concern and should not alter the data ordering. This ensures top categories/merchants keep their groupTotal desc sort when switching from chart to table view. Made-with: Cursor
| buildFilterQueryWithSortDefaults( | ||
| updatedFilterFormValues, | ||
| {view: searchAdvancedFiltersForm.view, groupBy: searchAdvancedFiltersForm.groupBy}, | ||
| {groupBy: searchAdvancedFiltersForm.groupBy}, |
There was a problem hiding this comment.
View is no longer needed in the previous state passed to this function, since it doesn't affect whether the sort order should be changed anymore.
melvin-bot
bot
requested review from
jayeshmangwani
and removed request for
a team
|
@jayeshmangwani 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
bot
requested review from
JmillsExpensify
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1627a2f7d9
ℹ️ 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".
| function shouldResetSort({newGroupBy, oldGroupBy}: {newGroupBy: string | undefined; oldGroupBy: string | undefined}): boolean { | ||
| return newGroupBy !== oldGroupBy; |
There was a problem hiding this comment.
By removing view comparisons from shouldResetSort, switching only the view now preserves the previous explicit sort in buildFilterQueryWithSortDefaults, so a groupBy:month/week/year/quarter search moved from table to line/bar keeps table ordering (typically sortOrder:desc). That bypasses the parser’s chart-specific defaulting in src/libs/SearchParser/searchParser.peggy (the view !== "table" + TIME_BASED_GROUP_BY path) which is intended to enforce ascending chronological order for chart views, so these charts can render newest-to-oldest unless users manually re-sort.
Useful? React with 👍 / 👎.
2 participants
Explanation of Change
Previously, the default sorting for top categories and top merchants insights were incorrect. That has been fixed, and then I also changed it so that updating the view mode, for example, from bar to table, does not change the sort order. The view setting should only affect presentation, not ordering.
Fixed Issues
$ #84420
Tests
view:tableview:tableOffline tests
N/A - This change only affects the default search query string construction. Sorting is handled server-side.
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
The changes are platform independent, so I only tested on Mac Chrome.
Android: Native
N/A - No UI changes, only default search query parameter change
Android: mWeb Chrome
N/A - No UI changes, only default search query parameter change
iOS: Native
N/A - No UI changes, only default search query parameter change
iOS: mWeb Safari
N/A - No UI changes, only default search query parameter change
MacOS: Chrome / Safari
SearchOrder2026-03-06_13-07-08.mp4