-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix custom columns search syntax #78864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@ShridharGoel 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] |
src/libs/SearchAutocompleteUtils.ts
Outdated
| const hasList = Object.values(CONST.SEARCH.HAS_VALUES) as string[]; | ||
| const isList = Object.values(CONST.SEARCH.IS_VALUES) as string[]; | ||
|
|
||
| const columnList = Object.entries(CONST.SEARCH.SEARCH_USER_FRIENDLY_VALUES_MAP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ PERF-2 (docs)
The columnList array is being created on every call to filterOutRangesWithCorrectValue. Since this function is marked as a worklet and is called inside a .filter() operation (line 261), the array creation happens repeatedly for each range item being filtered.
Move the columnList computation outside the function to the module level, similar to how userFriendlyExpenseTypeList, userFriendlyGroupByList, and userFriendlyStatusList are defined (lines 129-136):
const userFriendlyColumnList = Object.entries(CONST.SEARCH.SEARCH_USER_FRIENDLY_VALUES_MAP)
.filter(([key]) => Object.values(CONST.SEARCH.TABLE_COLUMNS).includes(key as SearchColumnType))
.map(([, value]) => value);
function filterOutRangesWithCorrectValue(
// ... params
) {
'worklet';
// ... other code
// Then use userFriendlyColumnList instead of columnList
case CONST.SEARCH.SYNTAX_ROOT_KEYS.COLUMNS:
return userFriendlyColumnList.includes(range.value);
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
Crash on clicking the search input: Screen.Recording.2026-01-06.at.11.53.29.PM.mov |
|
Fixed @ShridharGoel , nice catch |
src/libs/SearchAutocompleteUtils.ts
Outdated
| const hasList = Object.values(CONST.SEARCH.HAS_VALUES) as string[]; | ||
| const isList = Object.values(CONST.SEARCH.IS_VALUES) as string[]; | ||
|
|
||
| const columnList = Object.entries(CONST.SEARCH.SEARCH_USER_FRIENDLY_VALUES_MAP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this
|
Is this change in column positioning and display expected based on addition and removal of Screen.Recording.2026-01-07.at.1.21.52.AM.mov |
|
No, we should always include total |
|
@JS00001 we should address this one #78864 (comment) |
|
And is this one valid? #78864 (comment) |
|
Sorry, I've been caught up in some other priorities today, yes those are valid, I'll update those when I get the chance, thank you for the thorough review! |

Explanation of Change
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/584296
Tests
Open the reports page > expenses
Customize your columns using the 'columns' button
Ensure that the search syntax is not visible
Enter this query into the searchbar:
columns:merchant,exchange-rate,amountEnsure that the selected columns are visible, and also show up in the custom columns RHP
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Screen.Recording.2026-01-06.at.12.18.14.PM.mov