Add filter negation feature#93909
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@DylanDylann 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] |
|
Can I run the translation script myself? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5746a1607
ℹ️ 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".
|
Btw, when we apply a negated HAS filter with a Tag value, it will be applied as "Tag: No tag". w.mp4It's from this code. It causes weird buggy behavior with the Has filter component, as shown in the video above. App/src/libs/SearchQueryUtils.ts Lines 1110 to 1112 in e5928ed Is that expected? |
|
🚧 @dubielzyk-expensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
This is looking great already @bernhardoj. Let's make this text
|
|
I'm not convinced we should use the is/is not in the chip itself:
I have to go and will have a noodle, but I think we might want an icon or do cc @Expensify/design @trjExpensify @JmillsExpensify |
Done |
I'm fine with what I'm seeing in the adhoc. |
|
@bernhardoj Looks like we're still missing support for
|
|
Status and workspace are still having their BE PRs worked on: https://github.com/Expensify/Auth/pull/22241 |
|
Can we determine if its negated or not by its key and show any negated filters? |
|
@JS00001 the reason it doesn't show is actually because I gated it through App/src/types/form/SearchAdvancedFiltersForm.ts Lines 46 to 56 in 646f5f5
It's the same filtering logic used for the filter bars/chips. Do we want to remove the filtering? This will show the filter bar and the applied filters on the search page for every negated filter.
But it won't have the negation switch.
|
|
@bernhardoj I think that makes sense, since they can be negated via the search query |
|
@JS00001 but is it fine that the popup won't have the negation (Filter type) switch? (the left image). Notice that no option is selected, because it only works with non-negated value. |
|
Yes, I think the popup not showing the negation is fine, that's the current behavior |
|
Cool, updated! |
|
Yeah, I also think that's fine. |
|
@aimane-chnaif back to you |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
A few issues found: Bug 1:
Screen.Recording.2026-07-17.at.4.18.12.PM.movBug 2:
Screen.Recording.2026-07-17.at.4.25.15.PM.mov |
|
For the 1st bug, it's the result of showing all negated filters, even without the negation/Filter type switch. But I think even though it doesn't have that switch, we can just show the selection for the negated value. So, any update to the filter will update the negated value. @JS00001 @JmillsExpensify let me know if you disagree. For the 2nd bug, I think both non-negated and negated filters shouldn't be applied at the same time in the first place. When the user writes the same filter with both non-negated and negated types, I think we should take the rightmost one. For example, if we type then the final category filter is (I think the 2nd bug shouldn't block this PR if we want to go forward because we can apply both non-negated and negated filters already on prod) UPDATE: I just checked on prod, and when we write 2 same filter keys, for example, @JS00001 @JmillsExpensify what do you guys think? (also please check this one 😄) |
That makes sense to me.
Also makes sense to me, since the right-most is the most recent.
Given that this is already on production, I'd handle this separately and not block this PR on it. |
|
@aimane-chnaif just merged with main, please continue the review @JmillsExpensify bump for this one please 😄 |
| value: SearchAdvancedFiltersForm[K] | undefined; | ||
| } { | ||
| const negatedFilterKey = `${filterKey}${CONST.SEARCH.NOT_MODIFIER}` as const; | ||
| if (!isFilterNegatable(filterKey) || !values || !hasKey(values, negatedFilterKey)) { |
There was a problem hiding this comment.
Removing !isFilterNegatable(filterKey) can lead to a loop hole where users can type in -category:Car for example and land on a negated category query without any visual indicator in the pickers.
Screen.Recording.2026-07-21.at.1.26.35.PM.mov
There was a problem hiding this comment.
Yeah, we discussed it here and decide to allow that
| return filterKey.endsWith(CONST.SEARCH.NOT_MODIFIER); | ||
| } | ||
|
|
||
| function getFilterFormValues<K extends ListFilterContentProps['baseFilterKey'] | TextInputFilterContentProps['baseFilterKey']>( |
There was a problem hiding this comment.
Can we add a test for this function? It's pretty central to the negated util.
| [FILTER_KEYS.HAS_NOT]: HasFilterValues; | ||
|
|
||
| [FILTER_KEYS.IS]: IsFilterValues; | ||
| [FILTER_KEYS.IS_NOT]: string[]; |
There was a problem hiding this comment.
Can we tighten the type for this one too?









Explanation of Change
Fixed Issues
$ #93208
PROPOSAL:
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
-For steps 3-7, if you want more test steps, follow this:
No filter is applied yet
For filter chip/bar:
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
web.mp4