Add boolean filtering (#7190) #8700
Conversation
There was a problem hiding this comment.
PR Summary
Added boolean filtering capability across the application, enabling users to filter records based on true/false values with a dedicated dropdown selector component.
- Added
ObjectFilterDropdownBooleanSelect.tsxwith true/false selection UI and keyboard navigation support - Modified
formatFieldMetadataItemsAsFilterDefinitions.tsto includeFieldMetadataType.Booleanin supported field types - Added boolean case in
computeViewRecordGqlOperationFilter.tsto convert string values to booleans for database queries - Added
resolveBooleanViewFilterValue.tsutility to handle boolean filter value resolution - Limited boolean filters to only use 'Is' operand in
getOperandsForFilterType.ts, unlike other field types that support additional operands
8 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
| <SelectableList | ||
| selectableListId="boolean-select" | ||
| selectableItemIdArray={options.map((option) => option.toString())} | ||
| hotkeyScope={RelationPickerHotkeyScope.RelationPicker} |
There was a problem hiding this comment.
logic: using RelationPicker scope for boolean select seems incorrect - should have its own scope
There was a problem hiding this comment.
Agree this is weird. Maybe you could create a followup issue for that we can take later? (as this was not introduced by your PR and is already in other places)
| case 'BOOLEAN': { | ||
| return { | ||
| [correspondingField.name]: { | ||
| eq: filter.value === 'true', | ||
| } as BooleanFilter, | ||
| }; | ||
| } |
There was a problem hiding this comment.
logic: Missing support for IsEmpty/IsNotEmpty operands that are supported by other filter types
| case 'BOOLEAN': { | ||
| return { | ||
| [correspondingField.name]: { | ||
| eq: filter.value === 'true', | ||
| } as BooleanFilter, | ||
| }; | ||
| } |
There was a problem hiding this comment.
style: No support for IsNot operand which would be a logical complement to the current implementation
| case 'BOOLEAN': { | ||
| return { | ||
| [correspondingField.name]: { | ||
| eq: filter.value === 'true', |
There was a problem hiding this comment.
style: Consider using Boolean(filter.value) instead of filter.value === 'true' for more robust string-to-boolean conversion
| export const resolveBooleanViewFilterValue = ( | ||
| viewFilter: Pick<ViewFilter, 'value'>, | ||
| ) => { | ||
| return viewFilter.value === 'true'; |
There was a problem hiding this comment.
logic: Strict equality comparison with string 'true' could miss valid boolean string values like 'True' or '1'. Consider using Boolean() or implementing case-insensitive comparison.
...ages/twenty-front/src/modules/views/view-filter-value/utils/resolveBooleanViewFilterValue.ts
Show resolved
Hide resolved
Also this test (which is a bit annoying... Not sure if we should keep it tbh but let's keep it for now)
|
|
The weird filter state issue is happening with all filters and also in In production, if you add a filter from the column after adding the same filter from the filters menu, you end up in this same broken state. Let's fix this in a separate PR right after this one? |
|
Thanks @ad-elias for your contribution! |
Link to issue: #7190