admin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix advanced filters (#10665)

Browse Source 
This PR partially fixes advanced filters that were not working even with
feature flag activated.

Bugs fixed here : 

- Advanced filters are not applied
- Root advanced filters cannot be created
- Cannot close advanced filters dropdown
- Can create multiple times the same non-advanced filter (reserved for
advanced filters)

upsertRecordFilter and removeRecordFilter have been refactored to take
record filter id instead of field metadata id, because the user should
be allowed to apply multiple filters for the same field.

We now base view filter CRUD directly on id, otherwise it could lead to
inconsistencies between advanced filters and simple filters.

This PR also refactors an important hook :
computeRecordGqlOperationFilter, so that it takes an object instead of
multiple params.

There are still bugs left, they will be taken in other PRs.
This commit is contained in:
Lucas Bordeau
2025-03-05 15:01:07 +01:00
committed by GitHub
parent 3fa915ac99
commit e838dfc68b
38 changed files with 367 additions and 377 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
packages/twenty-front/src/modules/views/utils/__tests__/getViewFiltersToCreate.test.ts
View File

@ -17,8 +17,11 @@ describe('getViewFiltersToCreate', () => {
it('should return all filters when current filters array is empty', () => {
const currentViewFilters: ViewFilter[] = [];
const newViewFilters: ViewFilter[] = [
{ ...baseFilter },
{ ...baseFilter, id: 'filter-2', fieldMetadataId: 'field-2' },
{ ...baseFilter } satisfies ViewFilter,
{
...baseFilter,
id: 'filter-2',
} satisfies ViewFilter,
];
const result = getViewFiltersToCreate(currentViewFilters, newViewFilters);
@ -40,8 +43,7 @@ describe('getViewFiltersToCreate', () => {
const newFilterWithDifferentFieldMetadata = {
...baseFilter,
id: 'filter-2',
fieldMetadataId: 'field-2',
};
} satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [existingFilter];
@ -55,25 +57,6 @@ describe('getViewFiltersToCreate', () => {
expect(result).toEqual([newFilterWithDifferentFieldMetadata]);
});
it('should handle filters with different viewFilterGroupIds', () => {
const existingFilter = { ...baseFilter };
const filterWithDifferentGroup = {
...baseFilter,
viewFilterGroupId: 'group-2',
};
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [
existingFilter,
filterWithDifferentGroup,
];
const result = getViewFiltersToCreate(currentViewFilters, newViewFilters);
expect(result).toEqual([filterWithDifferentGroup]);
});
it('should handle empty arrays for both inputs', () => {
const currentViewFilters: ViewFilter[] = [];
const newViewFilters: ViewFilter[] = [];
@ -82,36 +65,4 @@ describe('getViewFiltersToCreate', () => {
expect(result).toEqual([]);
});
it('should consider filters with same fieldMetadataId but different viewFilterGroupId as new', () => {
const currentViewFilters: ViewFilter[] = [baseFilter];
const filterWithSameFieldMetadataIdButDifferentGroup = {
...baseFilter,
id: 'filter-2',
viewFilterGroupId: 'group-2',
};
const newViewFilters: ViewFilter[] = [
filterWithSameFieldMetadataIdButDifferentGroup,
];
const result = getViewFiltersToCreate(currentViewFilters, newViewFilters);
expect(result).toEqual([filterWithSameFieldMetadataIdButDifferentGroup]);
});
it('should consider filters with same viewFilterGroupId but different fieldMetadataId as new', () => {
const currentViewFilters: ViewFilter[] = [baseFilter];
const filterWithSameGroupButDifferentFieldMetadata = {
...baseFilter,
id: 'filter-2',
fieldMetadataId: 'field-2',
};
const newViewFilters: ViewFilter[] = [
filterWithSameGroupButDifferentFieldMetadata,
];
const result = getViewFiltersToCreate(currentViewFilters, newViewFilters);
expect(result).toEqual([filterWithSameGroupButDifferentFieldMetadata]);
});
});

42
packages/twenty-front/src/modules/views/utils/__tests__/getViewFiltersToDelete.test.ts
View File

@ -38,8 +38,7 @@ describe('getViewFiltersToDelete', () => {
const filterToKeep = {
...baseFilter,
id: 'filter-2',
fieldMetadataId: 'field-2',
};
} satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [filterToDelete, filterToKeep];
const newViewFilters: ViewFilter[] = [filterToKeep];
@ -57,43 +56,4 @@ describe('getViewFiltersToDelete', () => {
expect(result).toEqual([]);
});
it('should identify filters to delete based on fieldMetadataId and viewFilterGroupId', () => {
const filterInGroup1 = { ...baseFilter };
const filterInGroup2 = {
...baseFilter,
viewFilterGroupId: 'group-2',
};
const filterWithDifferentField = {
...baseFilter,
fieldMetadataId: 'field-2',
};
const currentViewFilters: ViewFilter[] = [
filterInGroup1,
filterInGroup2,
filterWithDifferentField,
];
const newViewFilters: ViewFilter[] = [filterInGroup1];
const result = getViewFiltersToDelete(currentViewFilters, newViewFilters);
expect(result).toEqual([filterInGroup2, filterWithDifferentField]);
});
it('should not delete filters that match in both fieldMetadataId and viewFilterGroupId', () => {
const existingFilter = { ...baseFilter };
const matchingFilter = {
...baseFilter,
value: 'different-value',
displayValue: 'different-value',
};
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [matchingFilter];
const result = getViewFiltersToDelete(currentViewFilters, newViewFilters);
expect(result).toEqual([]);
});
});

48
packages/twenty-front/src/modules/views/utils/__tests__/getViewFiltersToUpdate.test.ts
View File

@ -33,12 +33,12 @@ describe('getViewFiltersToUpdate', () => {
});
it('should return filters that exist in both arrays but have different values', () => {
const existingFilter = { ...baseFilter };
const existingFilter = { ...baseFilter } satisfies ViewFilter;
const updatedFilter = {
...baseFilter,
value: 'updated-value',
displayValue: 'updated-value',
};
} satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [updatedFilter];
@ -49,8 +49,8 @@ describe('getViewFiltersToUpdate', () => {
});
it('should not return filters that exist in both arrays with same values', () => {
const existingFilter = { ...baseFilter };
const sameFilter = { ...baseFilter };
const existingFilter = { ...baseFilter } satisfies ViewFilter;
const sameFilter = { ...baseFilter } satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [sameFilter];
@ -69,44 +69,12 @@ describe('getViewFiltersToUpdate', () => {
expect(result).toEqual([]);
});
it('should not update filters with same fieldMetadataId but different viewFilterGroupId', () => {
const existingFilter = { ...baseFilter };
const filterInDifferentGroup = {
...baseFilter,
viewFilterGroupId: 'group-2',
value: 'updated-value',
};
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [filterInDifferentGroup];
const result = getViewFiltersToUpdate(currentViewFilters, newViewFilters);
expect(result).toEqual([]);
});
it('should not update filters with same viewFilterGroupId but different fieldMetadataId', () => {
const existingFilter = { ...baseFilter };
const filterWithDifferentField = {
...baseFilter,
fieldMetadataId: 'field-2',
value: 'updated-value',
};
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [filterWithDifferentField];
const result = getViewFiltersToUpdate(currentViewFilters, newViewFilters);
expect(result).toEqual([]);
});
it('should update filter when operand changes', () => {
const existingFilter = { ...baseFilter };
const existingFilter = { ...baseFilter } satisfies ViewFilter;
const filterWithNewOperand = {
...baseFilter,
operand: ViewFilterOperand.DoesNotContain,
};
} satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [filterWithNewOperand];
@ -117,11 +85,11 @@ describe('getViewFiltersToUpdate', () => {
});
it('should update filter when position changes', () => {
const existingFilter = { ...baseFilter };
const existingFilter = { ...baseFilter } satisfies ViewFilter;
const filterWithNewPosition = {
...baseFilter,
positionInViewFilterGroup: 1,
};
} satisfies ViewFilter;
const currentViewFilters: ViewFilter[] = [existingFilter];
const newViewFilters: ViewFilter[] = [filterWithNewPosition];

20
packages/twenty-front/src/modules/views/utils/getQueryVariablesFromView.ts
View File

@ -4,8 +4,9 @@ import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { turnSortsIntoOrderBy } from '@/object-record/object-sort-dropdown/utils/turnSortsIntoOrderBy';
import { RecordFilterValueDependencies } from '@/object-record/record-filter/types/RecordFilterValueDependencies';
import { computeViewRecordGqlOperationFilter } from '@/object-record/record-filter/utils/computeViewRecordGqlOperationFilter';
import { computeRecordGqlOperationFilter } from '@/object-record/record-filter/utils/computeViewRecordGqlOperationFilter';
import { View } from '@/views/types/View';
import { mapViewFilterGroupsToRecordFilterGroups } from '@/views/utils/mapViewFilterGroupsToRecordFilterGroups';
import { mapViewFiltersToFilters } from '@/views/utils/mapViewFiltersToFilters';
import { mapViewSortsToSorts } from '@/views/utils/mapViewSortsToSorts';
import { isDefined } from 'twenty-shared';
@ -30,13 +31,22 @@ export const getQueryVariablesFromView = ({
const { viewFilterGroups, viewFilters, viewSorts } = view;
const filter = computeViewRecordGqlOperationFilter(
filterValueDependencies,
mapViewFiltersToFilters(viewFilters, fieldMetadataItems),
objectMetadataItem?.fields ?? [],
const recordFilterGroups = mapViewFilterGroupsToRecordFilterGroups(
viewFilterGroups ?? [],
);
const recordFilters = mapViewFiltersToFilters(
viewFilters,
fieldMetadataItems,
);
const filter = computeRecordGqlOperationFilter({
fields: objectMetadataItem?.fields ?? [],
filterValueDependencies,
recordFilterGroups,
recordFilters,
});
const orderBy = turnSortsIntoOrderBy(
objectMetadataItem,
mapViewSortsToSorts(viewSorts),

7
packages/twenty-front/src/modules/views/utils/getViewFiltersToCreate.ts
View File

@ -1,5 +1,6 @@
import { ViewFilter } from '@/views/types/ViewFilter';
import { isDefined } from 'twenty-shared';
import { compareStrictlyExceptForNullAndUndefined } from '~/utils/compareStrictlyExceptForNullAndUndefined';
export const getViewFiltersToCreate = (
currentViewFilters: ViewFilter[],
@ -8,8 +9,10 @@ export const getViewFiltersToCreate = (
return newViewFilters.filter((newViewFilter) => {
const correspondingViewFilter = currentViewFilters.find(
(currentViewFilter) =>
currentViewFilter.fieldMetadataId === newViewFilter.fieldMetadataId &&
currentViewFilter.viewFilterGroupId === newViewFilter.viewFilterGroupId,
compareStrictlyExceptForNullAndUndefined(
currentViewFilter.id,
newViewFilter.id,
),
);
const shouldCreateBecauseViewFilterIsNew = !isDefined(

11
packages/twenty-front/src/modules/views/utils/getViewFiltersToDelete.ts
View File

@ -1,4 +1,5 @@
import { ViewFilter } from '@/views/types/ViewFilter';
import { compareStrictlyExceptForNullAndUndefined } from '~/utils/compareStrictlyExceptForNullAndUndefined';
export const getViewFiltersToDelete = (
currentViewFilters: ViewFilter[],
@ -6,11 +7,11 @@ export const getViewFiltersToDelete = (
) => {
return currentViewFilters.filter(
(currentViewFilter) =>
!newViewFilters.some(
(newViewFilter) =>
newViewFilter.fieldMetadataId === currentViewFilter.fieldMetadataId &&
newViewFilter.viewFilterGroupId ===
currentViewFilter.viewFilterGroupId,
!newViewFilters.some((newViewFilter) =>
compareStrictlyExceptForNullAndUndefined(
currentViewFilter.id,
newViewFilter.id,
),
),
);
};

7
packages/twenty-front/src/modules/views/utils/getViewFiltersToUpdate.ts
View File

@ -1,6 +1,7 @@
import { ViewFilter } from '@/views/types/ViewFilter';
import { areViewFiltersEqual } from '@/views/utils/areViewFiltersEqual';
import { isDefined } from 'twenty-shared';
import { compareStrictlyExceptForNullAndUndefined } from '~/utils/compareStrictlyExceptForNullAndUndefined';
export const getViewFiltersToUpdate = (
currentViewFilters: ViewFilter[],
@ -9,8 +10,10 @@ export const getViewFiltersToUpdate = (
return newViewFilters.filter((newViewFilter) => {
const correspondingViewFilter = currentViewFilters.find(
(currentViewFilter) =>
currentViewFilter.fieldMetadataId === newViewFilter.fieldMetadataId &&
currentViewFilter.viewFilterGroupId === newViewFilter.viewFilterGroupId,
compareStrictlyExceptForNullAndUndefined(
currentViewFilter.id,
newViewFilter.id,
),
);
if (!isDefined(correspondingViewFilter)) {