From e47c19e86ff42452a7e9aab9c6120b304381fba1 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Wed, 2 Apr 2025 10:25:51 +0200 Subject: [PATCH] Advanced filter bug bash (#11327) This PR fixes some bugs on advanced filters dropdown. See reference master issue : https://github.com/twentyhq/core-team-issues/issues/648 Fixes https://github.com/twentyhq/core-team-issues/issues/726 Fixes https://github.com/twentyhq/core-team-issues/issues/724 Fixes https://github.com/twentyhq/core-team-issues/issues/655 --- ...AdvancedFilterValueInputDropdownButton.tsx | 10 + .../constants/SoftDeleteFilterFieldName.ts | 1 + .../hooks/useCheckIsSoftDeleteFilter.ts | 3 +- ...lterDescendantsOfRecordFilterGroup.test.ts | 184 ++++++++++++++++++ ...ordFilterDescendantsOfRecordFilterGroup.ts | 43 ++++ .../views/components/AdvancedFilterChip.tsx | 53 ++++- 6 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-filter/constants/SoftDeleteFilterFieldName.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/getAllRecordFilterDescendantsOfRecordFilterGroup.test.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-filter/utils/getAllRecordFilterDescendantsOfRecordFilterGroup.ts diff --git a/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterValueInputDropdownButton.tsx b/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterValueInputDropdownButton.tsx index 190bf207a..224b720c3 100644 --- a/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterValueInputDropdownButton.tsx +++ b/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterValueInputDropdownButton.tsx @@ -2,6 +2,7 @@ import { AdvancedFilterValueInputDropdownButtonClickableSelect } from '@/object- import { DEFAULT_ADVANCED_FILTER_DROPDOWN_OFFSET } from '@/object-record/advanced-filter/constants/DefaultAdvancedFilterDropdownOffset'; import { ObjectFilterDropdownFilterInput } from '@/object-record/object-filter-dropdown/components/ObjectFilterDropdownFilterInput'; import { fieldMetadataItemIdUsedInDropdownComponentState } from '@/object-record/object-filter-dropdown/states/fieldMetadataItemIdUsedInDropdownComponentState'; +import { objectFilterDropdownSearchInputComponentState } from '@/object-record/object-filter-dropdown/states/objectFilterDropdownSearchInputComponentState'; import { selectedFilterComponentState } from '@/object-record/object-filter-dropdown/states/selectedFilterComponentState'; import { selectedOperandInDropdownComponentState } from '@/object-record/object-filter-dropdown/states/selectedOperandInDropdownComponentState'; import { configurableViewFilterOperands } from '@/object-record/object-filter-dropdown/utils/configurableViewFilterOperands'; @@ -35,6 +36,10 @@ export const AdvancedFilterValueInputDropdownButton = ({ const isDisabled = !filter?.fieldMetadataId || !filter.operand; + const setObjectFilterDropdownSearchInput = useSetRecoilComponentStateV2( + objectFilterDropdownSearchInputComponentState, + ); + const setFieldMetadataItemIdUsedInDropdown = useSetRecoilComponentStateV2( fieldMetadataItemIdUsedInDropdownComponentState, ); @@ -50,6 +55,10 @@ export const AdvancedFilterValueInputDropdownButton = ({ const operandHasNoInput = filter && !configurableViewFilterOperands.has(filter.operand); + const handleFilterValueDropdownClose = () => { + setObjectFilterDropdownSearchInput(''); + }; + return ( {operandHasNoInput ? ( @@ -78,6 +87,7 @@ export const AdvancedFilterValueInputDropdownButton = ({ dropdownOffset={DEFAULT_ADVANCED_FILTER_DROPDOWN_OFFSET} dropdownPlacement="bottom-start" dropdownMenuWidth={280} + onClose={handleFilterValueDropdownClose} /> )} diff --git a/packages/twenty-front/src/modules/object-record/record-filter/constants/SoftDeleteFilterFieldName.ts b/packages/twenty-front/src/modules/object-record/record-filter/constants/SoftDeleteFilterFieldName.ts new file mode 100644 index 000000000..4f9959ca8 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-filter/constants/SoftDeleteFilterFieldName.ts @@ -0,0 +1 @@ +export const SOFT_DELETE_FILTER_FIELD_NAME = 'deletedAt'; diff --git a/packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts b/packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts index 87eda85c9..691b2d177 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts @@ -1,4 +1,5 @@ import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; +import { SOFT_DELETE_FILTER_FIELD_NAME } from '@/object-record/record-filter/constants/SoftDeleteFilterFieldName'; import { RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; import { RecordFilterOperand } from '@/object-record/record-filter/types/RecordFilterOperand'; import { isSoftDeleteFilterActiveComponentState } from '@/object-record/record-table/states/isSoftDeleteFilterActiveComponentState'; @@ -29,7 +30,7 @@ export const useCheckIsSoftDeleteFilter = () => { } return ( - foundFieldMetadataItem.name === 'deletedAt' && + foundFieldMetadataItem.name === SOFT_DELETE_FILTER_FIELD_NAME && isSoftDeleteFilterActive && recordFilter.operand === RecordFilterOperand.IsNotEmpty ); diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/getAllRecordFilterDescendantsOfRecordFilterGroup.test.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/getAllRecordFilterDescendantsOfRecordFilterGroup.test.ts new file mode 100644 index 000000000..2e4c09e9e --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/__tests__/getAllRecordFilterDescendantsOfRecordFilterGroup.test.ts @@ -0,0 +1,184 @@ +import { RecordFilterGroup } from '@/object-record/record-filter-group/types/RecordFilterGroup'; +import { RecordFilterGroupLogicalOperator } from '@/object-record/record-filter-group/types/RecordFilterGroupLogicalOperator'; +import { RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; +import { RecordFilterOperand } from '@/object-record/record-filter/types/RecordFilterOperand'; +import { sortByProperty } from '~/utils/array/sortByProperty'; +import { getAllRecordFilterDescendantsOfRecordFilterGroup } from '../getAllRecordFilterDescendantsOfRecordFilterGroup'; + +const MOCK_RECORD_FILTER_FIELDS: RecordFilter = { + id: 'filter-1', + recordFilterGroupId: 'root-group', + fieldMetadataId: 'field-1', + operand: RecordFilterOperand.Contains, + value: 'value-1', + displayValue: 'Display Value 1', + label: 'Label 1', + type: 'TEXT', +}; + +describe('getAllRecordFilterDescendantsOfRecordFilterGroup', () => { + it('should return an empty array if the recordFilterGroupId does not exist', () => { + const recordFilterGroups: RecordFilterGroup[] = []; + const recordFilters: RecordFilter[] = []; + const recordFilterGroupId = 'nonexistent-id'; + + const result = getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId, + recordFilterGroups, + recordFilters, + }); + + expect(result).toEqual([]); + }); + + it('should return all direct child record filters of the given recordFilterGroupId', () => { + const recordFilterGroups: RecordFilterGroup[] = [ + { + id: 'root-group', + parentRecordFilterGroupId: null, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + ]; + + const recordFiltersDescendants: RecordFilter[] = [ + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-1', + recordFilterGroupId: 'root-group', + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-2', + recordFilterGroupId: 'root-group', + }, + ]; + + const recordFilterGroupId = 'root-group'; + + const result = getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId, + recordFilterGroups, + recordFilters: recordFiltersDescendants, + }); + + expect(result).toEqual(recordFiltersDescendants); + }); + + it('should return all descendant record filters recursively', () => { + const recordFilterGroups: RecordFilterGroup[] = [ + { + id: 'root-group', + parentRecordFilterGroupId: null, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + { + id: 'child-group-1', + parentRecordFilterGroupId: 'root-group', + logicalOperator: RecordFilterGroupLogicalOperator.OR, + }, + { + id: 'grand-child-group-1', + parentRecordFilterGroupId: 'child-group-1', + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + { + id: 'grand-child-group-2', + parentRecordFilterGroupId: 'child-group-1', + logicalOperator: RecordFilterGroupLogicalOperator.OR, + }, + { + id: 'child-group-2', + parentRecordFilterGroupId: 'root-group', + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + ]; + + const recordFiltersWithoutGroup: RecordFilter[] = [ + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-1', + recordFilterGroupId: undefined, + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-2', + recordFilterGroupId: undefined, + }, + ]; + + const recordFiltersDescendants: RecordFilter[] = [ + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-3', + recordFilterGroupId: 'root-group', + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-4', + recordFilterGroupId: 'child-group-1', + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-5', + recordFilterGroupId: 'child-group-2', + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-6', + recordFilterGroupId: 'grand-child-group-1', + }, + { + ...MOCK_RECORD_FILTER_FIELDS, + id: 'filter-7', + recordFilterGroupId: 'grand-child-group-2', + }, + ]; + + const combinedRecordFilters = [ + ...recordFiltersWithoutGroup, + ...recordFiltersDescendants, + ]; + + const recordFilterGroupId = 'root-group'; + + const allDescendantOfRootRecordFilterGroup = + getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId, + recordFilterGroups, + recordFilters: combinedRecordFilters, + }); + + const result = [...allDescendantOfRootRecordFilterGroup].sort( + sortByProperty('id'), + ); + + const expectedResult = [...recordFiltersDescendants].sort( + sortByProperty('id'), + ); + + expect(result).toEqual(expectedResult); + }); + + it('should return an empty array if the group has no children', () => { + const recordFilterGroups: RecordFilterGroup[] = [ + { + id: 'empty-group-id', + parentRecordFilterGroupId: 'parent-group-id', + logicalOperator: RecordFilterGroupLogicalOperator.AND, + positionInRecordFilterGroup: 0, + }, + ]; + + const recordFilters: RecordFilter[] = []; + + const recordFilterGroupId = 'empty-group-id'; + + const result = getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId, + recordFilterGroups, + recordFilters, + }); + + expect(result).toEqual([]); + }); +}); diff --git a/packages/twenty-front/src/modules/object-record/record-filter/utils/getAllRecordFilterDescendantsOfRecordFilterGroup.ts b/packages/twenty-front/src/modules/object-record/record-filter/utils/getAllRecordFilterDescendantsOfRecordFilterGroup.ts new file mode 100644 index 000000000..252674418 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-filter/utils/getAllRecordFilterDescendantsOfRecordFilterGroup.ts @@ -0,0 +1,43 @@ +import { RecordFilterGroup } from '@/object-record/record-filter-group/types/RecordFilterGroup'; +import { RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; + +export const getAllRecordFilterDescendantsOfRecordFilterGroup = ({ + recordFilterGroupId, + recordFilterGroups, + recordFilters, +}: { + recordFilterGroupId: string; + recordFilterGroups: RecordFilterGroup[]; + recordFilters: RecordFilter[]; +}): RecordFilter[] => { + const foundRecordFilterGroup = recordFilterGroups.find( + (recordFilterGroup) => recordFilterGroup.id === recordFilterGroupId, + ); + + if (!foundRecordFilterGroup) { + return []; + } + + const childRecordFilters = recordFilters.filter( + (recordFilter) => + recordFilter.recordFilterGroupId === foundRecordFilterGroup.id, + ); + + const childRecordFilterGroups = recordFilterGroups.filter( + (recordFilterGroup) => + recordFilterGroup.parentRecordFilterGroupId === foundRecordFilterGroup.id, + ); + + for (const childRecordFilterGroup of childRecordFilterGroups) { + const childRecordFilterGroupDescendants = + getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId: childRecordFilterGroup.id, + recordFilterGroups, + recordFilters, + }); + + childRecordFilters.push(...childRecordFilterGroupDescendants); + } + + return childRecordFilters; +}; diff --git a/packages/twenty-front/src/modules/views/components/AdvancedFilterChip.tsx b/packages/twenty-front/src/modules/views/components/AdvancedFilterChip.tsx index 6d27c0aff..12c1c6b78 100644 --- a/packages/twenty-front/src/modules/views/components/AdvancedFilterChip.tsx +++ b/packages/twenty-front/src/modules/views/components/AdvancedFilterChip.tsx @@ -1,15 +1,22 @@ import { IconFilter } from 'twenty-ui'; +import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; +import { useChildRecordFiltersAndRecordFilterGroups } from '@/object-record/advanced-filter/hooks/useChildRecordFiltersAndRecordFilterGroups'; +import { rootLevelRecordFilterGroupComponentSelector } from '@/object-record/advanced-filter/states/rootLevelRecordFilterGroupComponentSelector'; import { useRemoveRecordFilterGroup } from '@/object-record/record-filter-group/hooks/useRemoveRecordFilterGroup'; import { useRemoveRootRecordFilterGroupIfEmpty } from '@/object-record/record-filter-group/hooks/useRemoveRootRecordFilterGroupIfEmpty'; import { currentRecordFilterGroupsComponentState } from '@/object-record/record-filter-group/states/currentRecordFilterGroupsComponentState'; + +import { SOFT_DELETE_FILTER_FIELD_NAME } from '@/object-record/record-filter/constants/SoftDeleteFilterFieldName'; import { useRemoveRecordFilter } from '@/object-record/record-filter/hooks/useRemoveRecordFilter'; import { currentRecordFiltersComponentState } from '@/object-record/record-filter/states/currentRecordFiltersComponentState'; +import { getAllRecordFilterDescendantsOfRecordFilterGroup } from '@/object-record/record-filter/utils/getAllRecordFilterDescendantsOfRecordFilterGroup'; import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { SortOrFilterChip } from '@/views/components/SortOrFilterChip'; import { ADVANCED_FILTER_DROPDOWN_ID } from '@/views/constants/AdvancedFilterDropdownId'; import { plural } from 'pluralize'; +import { useMemo } from 'react'; import { isDefined } from 'twenty-shared/utils'; export const AdvancedFilterChip = () => { @@ -33,6 +40,15 @@ export const AdvancedFilterChip = () => { const { removeRootRecordFilterGroupIfEmpty } = useRemoveRootRecordFilterGroupIfEmpty(); + const rootRecordFilterGroup = useRecoilComponentValueV2( + rootLevelRecordFilterGroupComponentSelector, + ); + + const { childRecordFiltersAndRecordFilterGroups } = + useChildRecordFiltersAndRecordFilterGroups({ + recordFilterGroupId: rootRecordFilterGroup?.id, + }); + const handleRemoveClick = () => { closeDropdown(); @@ -51,11 +67,45 @@ export const AdvancedFilterChip = () => { removeRootRecordFilterGroupIfEmpty(); }; - const advancedFilterCount = advancedRecordFilterIds.length; + const advancedFilterCount = childRecordFiltersAndRecordFilterGroups.length; const labelText = 'advanced rule'; const chipLabel = `${advancedFilterCount} ${advancedFilterCount === 1 ? labelText : plural(labelText)}`; + const { objectMetadataItems } = useObjectMetadataItems(); + + const hasAnyDeletedAtFilterInAdvancedFilters = useMemo(() => { + const recordFiltersDescendantOfRootGroup = rootRecordFilterGroup?.id + ? getAllRecordFilterDescendantsOfRecordFilterGroup({ + recordFilterGroupId: rootRecordFilterGroup?.id, + recordFilterGroups: currentRecordFilterGroups, + recordFilters: currentRecordFilters, + }) + : []; + + const fieldMetadataItems = objectMetadataItems.flatMap( + (item) => item.fields, + ); + + return recordFiltersDescendantOfRootGroup.some((recordFilter) => { + const correspondingMetadataItem = fieldMetadataItems.find( + (fieldMetadataItem) => + fieldMetadataItem.id === recordFilter.fieldMetadataId, + ); + + if (isDefined(correspondingMetadataItem)) { + return correspondingMetadataItem.name === SOFT_DELETE_FILTER_FIELD_NAME; + } + + return false; + }); + }, [ + currentRecordFilterGroups, + currentRecordFilters, + rootRecordFilterGroup, + objectMetadataItems, + ]); + return ( { labelValue="" Icon={IconFilter} onRemove={handleRemoveClick} + variant={hasAnyDeletedAtFilterInAdvancedFilters ? 'danger' : 'default'} /> ); };