Fix many bugs on advanced filters for CRUD to work (#10772)

This PR fixes many bugs on advanced filters, the goal here was to have
CRUD working with other simple filters and sorts.

In order to test this PR you'll have to run a sync metadata.

Fixes https://github.com/twentyhq/core-team-issues/issues/560

## Changed positionInViewFilterGroup field metadata type 

This PR changes the type of positionInViewFilterGroup to NUMERIC instead
of POSITION, there certainly was a confusion during the initial
development, where POSITION type seemed relevant but it is not for this
particular feature because the position in a view filter group is not
the position of a record, which is used for displaying and re-ordering
purpose in table and kanban views.

Here the positionInViewFilterGroup is a specific position concept tied
to a custom feature, and it is handled by the specific logic of this
advanced filter dropdown layout.

## Create new ids when duplicating a view

When we use create view from an existing view, the logic in
useCreateViewFromCurrentView will copy over filters, filter groups and
sorts. The problem is that it copies it with the same ids, and that if
the backend manages somehow to create new ids, the ids that are put in
parentViewFilterGroupId are corresponding to the old filter groups not
the duplicated new ones.

So we had to create a map of old id => new id so that everything that
has to be sent to the backend for creation already has the same mapping
of parent id but with new ids generated by the frontend.

## Bug with creating a simple filter

We couldn't create a simple filter when advanced filters were set, this
was because of findDuplicateRecordFilterInNonAdvancedRecordFilters which
wasn't doing what it's naming tells, it wasn't filtering on simple
filters only before looking for duplicates.

## Clean code

- Use lastChildPosition directly from
useChildRecordFiltersAndRecordFilterGroups instead of drilling it down
- Refactored AdvancedFilterDropdownButton to extract the code lower
where it is really needed in AdvancedFilterChip
- Renamed a few View to Record naming where relevant
This commit is contained in:
Lucas Bordeau
2025-03-12 11:23:41 +01:00
committed by GitHub
parent 8d1a4672de
commit 911647a279
12 changed files with 224 additions and 129 deletions

View File

@ -1,4 +1,5 @@
import { getFilterTypeFromFieldType } from '@/object-metadata/utils/formatFieldMetadataItemsAsFilterDefinitions';
import { useChildRecordFiltersAndRecordFilterGroups } from '@/object-record/advanced-filter/hooks/useChildRecordFiltersAndRecordFilterGroups';
import { useDefaultFieldMetadataItemForFilter } from '@/object-record/advanced-filter/hooks/useDefaultFieldMetadataItemForFilter';
import { getAdvancedFilterAddFilterRuleSelectDropdownId } from '@/object-record/advanced-filter/utils/getAdvancedFilterAddFilterRuleSelectDropdownId';
import { useUpsertRecordFilterGroup } from '@/object-record/record-filter-group/hooks/useUpsertRecordFilterGroup';
@ -16,12 +17,10 @@ import { v4 } from 'uuid';
type AdvancedFilterAddFilterRuleSelectProps = {
recordFilterGroup: RecordFilterGroup;
lastChildPosition?: number;
};
export const AdvancedFilterAddFilterRuleSelect = ({
recordFilterGroup,
lastChildPosition = 0,
}: AdvancedFilterAddFilterRuleSelectProps) => {
const dropdownId = getAdvancedFilterAddFilterRuleSelectDropdownId(
recordFilterGroup.id,
@ -33,6 +32,10 @@ export const AdvancedFilterAddFilterRuleSelect = ({
const { upsertRecordFilter } = useUpsertRecordFilter();
const { lastChildPosition } = useChildRecordFiltersAndRecordFilterGroups({
recordFilterGroupId: recordFilterGroup.id,
});
const newPositionInRecordFilterGroup = lastChildPosition + 1;
const { closeDropdown } = useDropdown(dropdownId);

View File

@ -34,13 +34,10 @@ type AdvancedFilterRecordFilterGroupProps = {
export const AdvancedFilterRecordFilterGroup = ({
recordFilterGroupId,
}: AdvancedFilterRecordFilterGroupProps) => {
const {
currentRecordFilterGroup,
childRecordFiltersAndRecordFilterGroups,
lastChildPosition,
} = useChildRecordFiltersAndRecordFilterGroups({
recordFilterGroupId,
});
const { currentRecordFilterGroup, childRecordFiltersAndRecordFilterGroups } =
useChildRecordFiltersAndRecordFilterGroups({
recordFilterGroupId,
});
if (!currentRecordFilterGroup) {
return null;
@ -66,7 +63,6 @@ export const AdvancedFilterRecordFilterGroup = ({
))}
<AdvancedFilterAddFilterRuleSelect
recordFilterGroup={currentRecordFilterGroup}
lastChildPosition={lastChildPosition}
/>
</StyledContainer>
);

View File

@ -6,6 +6,8 @@ import { AdvancedFilterRecordFilterGroup } from '@/object-record/advanced-filter
import { AdvancedFilterViewFilter } from '@/object-record/advanced-filter/components/AdvancedFilterViewFilter';
import { useChildRecordFiltersAndRecordFilterGroups } from '@/object-record/advanced-filter/hooks/useChildRecordFiltersAndRecordFilterGroups';
import { isRecordFilterGroupChildARecordFilterGroup } from '@/object-record/advanced-filter/utils/isRecordFilterGroupChildARecordFilterGroup';
import { currentRecordFilterGroupsComponentState } from '@/object-record/record-filter-group/states/currentRecordFilterGroupsComponentState';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
import styled from '@emotion/styled';
import { isDefined } from 'twenty-shared';
@ -29,19 +31,20 @@ const StyledContainer = styled.div<{ isGrayBackground?: boolean }>`
overflow: hidden;
`;
type AdvancedFilterRootLevelViewFilterGroupProps = {
rootLevelRecordFilterGroupId: string;
};
export const AdvancedFilterRootLevelViewFilterGroup = () => {
const currentRecordFilterGroups = useRecoilComponentValueV2(
currentRecordFilterGroupsComponentState,
);
const rootRecordFilterGroupId = currentRecordFilterGroups.find(
(recordFilterGroup) => !recordFilterGroup.parentRecordFilterGroupId,
)?.id;
export const AdvancedFilterRootLevelViewFilterGroup = ({
rootLevelRecordFilterGroupId,
}: AdvancedFilterRootLevelViewFilterGroupProps) => {
const {
currentRecordFilterGroup: rootLevelRecordFilterGroup,
childRecordFiltersAndRecordFilterGroups,
lastChildPosition,
} = useChildRecordFiltersAndRecordFilterGroups({
recordFilterGroupId: rootLevelRecordFilterGroupId,
recordFilterGroupId: rootRecordFilterGroupId,
});
if (!isDefined(rootLevelRecordFilterGroup)) {
@ -50,34 +53,38 @@ export const AdvancedFilterRootLevelViewFilterGroup = ({
return (
<StyledContainer>
{childRecordFiltersAndRecordFilterGroups.map((child, i) =>
isRecordFilterGroupChildARecordFilterGroup(child) ? (
<StyledRow key={child.id}>
<AdvancedFilterLogicalOperatorCell
index={i}
recordFilterGroup={rootLevelRecordFilterGroup}
/>
<AdvancedFilterRecordFilterGroup recordFilterGroupId={child.id} />
<AdvancedFilterRecordFilterGroupChildOptionsDropdown
recordFilterGroupChild={child}
/>
</StyledRow>
) : (
<StyledRow key={child.id}>
<AdvancedFilterLogicalOperatorCell
index={i}
recordFilterGroup={rootLevelRecordFilterGroup}
/>
<AdvancedFilterViewFilter viewFilterId={child.id} />
<AdvancedFilterRecordFilterGroupChildOptionsDropdown
recordFilterGroupChild={child}
/>
</StyledRow>
),
{childRecordFiltersAndRecordFilterGroups.map(
(recordFilterGroupChild, recordFilterGroupChildIndex) =>
isRecordFilterGroupChildARecordFilterGroup(recordFilterGroupChild) ? (
<StyledRow key={recordFilterGroupChild.id}>
<AdvancedFilterLogicalOperatorCell
index={recordFilterGroupChildIndex}
recordFilterGroup={rootLevelRecordFilterGroup}
/>
<AdvancedFilterRecordFilterGroup
recordFilterGroupId={recordFilterGroupChild.id}
/>
<AdvancedFilterRecordFilterGroupChildOptionsDropdown
recordFilterGroupChild={recordFilterGroupChild}
/>
</StyledRow>
) : (
<StyledRow key={recordFilterGroupChild.id}>
<AdvancedFilterLogicalOperatorCell
index={recordFilterGroupChildIndex}
recordFilterGroup={rootLevelRecordFilterGroup}
/>
<AdvancedFilterViewFilter
viewFilterId={recordFilterGroupChild.id}
/>
<AdvancedFilterRecordFilterGroupChildOptionsDropdown
recordFilterGroupChild={recordFilterGroupChild}
/>
</StyledRow>
),
)}
<AdvancedFilterAddFilterRuleSelect
recordFilterGroup={rootLevelRecordFilterGroup}
lastChildPosition={lastChildPosition}
/>
</StyledContainer>
);

View File

@ -91,14 +91,14 @@ export const AdvancedFilterButton = () => {
const alreadyHasAdvancedFilterGroup = currentRecordFilterGroups.length > 0;
if (!alreadyHasAdvancedFilterGroup) {
const newViewFilterGroup = {
const newRecordFilterGroup = {
id: v4(),
viewId: currentView.id,
logicalOperator: ViewFilterGroupLogicalOperator.AND,
};
upsertRecordFilterGroup({
id: newViewFilterGroup.id,
id: newRecordFilterGroup.id,
logicalOperator: RecordFilterGroupLogicalOperator.AND,
});
@ -127,9 +127,10 @@ export const AdvancedFilterButton = () => {
operand: firstOperand,
value: '',
displayValue: '',
recordFilterGroupId: newViewFilterGroup.id,
recordFilterGroupId: newRecordFilterGroup.id,
type: getFilterTypeFromFieldType(defaultFieldMetadataItem.type),
label: defaultFieldMetadataItem.label,
positionInRecordFilterGroup: 1,
});
}

View File

@ -69,6 +69,8 @@ export const ObjectFilterDropdownTextSearchInput = () => {
fieldMetadataItemUsedInDropdown.type,
),
label: fieldMetadataItemUsedInDropdown.label,
positionInRecordFilterGroup:
selectedFilter?.positionInRecordFilterGroup,
});
}}
/>

View File

@ -1,4 +1,5 @@
import { RecordFilter } from '@/object-record/record-filter/types/RecordFilter';
import { isDefined } from 'twenty-shared';
import { compareStrictlyExceptForNullAndUndefined } from '~/utils/compareStrictlyExceptForNullAndUndefined';
export const findDuplicateRecordFilterInNonAdvancedRecordFilters = ({
@ -10,17 +11,19 @@ export const findDuplicateRecordFilterInNonAdvancedRecordFilters = ({
fieldMetadataItemId: string;
subFieldName?: string | null | undefined;
}): RecordFilter | undefined => {
const duplicateFilterInCurrentRecordFilters = recordFilters.find(
(recordFilter) =>
compareStrictlyExceptForNullAndUndefined(
recordFilter.fieldMetadataId,
fieldMetadataItemId,
) &&
compareStrictlyExceptForNullAndUndefined(
recordFilter.subFieldName,
subFieldName,
),
);
const duplicateFilterInCurrentRecordFilters = recordFilters
.filter((recordFilter) => !isDefined(recordFilter.recordFilterGroupId))
.find(
(recordFilter) =>
compareStrictlyExceptForNullAndUndefined(
recordFilter.fieldMetadataId,
fieldMetadataItemId,
) &&
compareStrictlyExceptForNullAndUndefined(
recordFilter.subFieldName,
subFieldName,
),
);
return duplicateFilterInCurrentRecordFilters;
};

View File

@ -1,27 +1,62 @@
import { IconFilterCog } from 'twenty-ui';
import { useRemoveRecordFilterGroup } from '@/object-record/record-filter-group/hooks/useRemoveRecordFilterGroup';
import { currentRecordFilterGroupsComponentState } from '@/object-record/record-filter-group/states/currentRecordFilterGroupsComponentState';
import { useRemoveRecordFilter } from '@/object-record/record-filter/hooks/useRemoveRecordFilter';
import { currentRecordFiltersComponentState } from '@/object-record/record-filter/states/currentRecordFiltersComponentState';
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 { isDefined } from 'twenty-shared';
import { isNonEmptyArray } from '~/utils/isNonEmptyArray';
type AdvancedFilterChipProps = {
onRemove: () => void;
advancedFilterCount?: number;
};
export const AdvancedFilterChip = () => {
const currentRecordFilterGroups = useRecoilComponentValueV2(
currentRecordFilterGroupsComponentState,
);
const currentRecordFilters = useRecoilComponentValueV2(
currentRecordFiltersComponentState,
);
const advancedRecordFilterIds = currentRecordFilters
.filter((recordFilter) => isDefined(recordFilter.recordFilterGroupId))
.map((recordFilter) => recordFilter.id);
const { removeRecordFilter } = useRemoveRecordFilter();
const { removeRecordFilterGroup } = useRemoveRecordFilterGroup();
const handleRemoveClick = () => {
if (!isNonEmptyArray(advancedRecordFilterIds)) {
throw new Error('No advanced view filters to remove');
}
const viewFilterGroupIds = currentRecordFilterGroups.map(
(recordFilterGroup) => recordFilterGroup.id,
);
for (const viewFilterGroupId of viewFilterGroupIds) {
removeRecordFilterGroup(viewFilterGroupId);
}
for (const recordFilterId of advancedRecordFilterIds) {
removeRecordFilter({ recordFilterId });
}
};
const advancedFilterCount = advancedRecordFilterIds.length;
export const AdvancedFilterChip = ({
onRemove,
advancedFilterCount,
}: AdvancedFilterChipProps) => {
const labelText = 'advanced rule';
const chipLabel = `${advancedFilterCount ?? 0} ${advancedFilterCount === 1 ? labelText : plural(labelText)}`;
const chipLabel = `${advancedFilterCount} ${advancedFilterCount === 1 ? labelText : plural(labelText)}`;
return (
<SortOrFilterChip
testId={ADVANCED_FILTER_DROPDOWN_ID}
labelKey={chipLabel}
labelValue=""
Icon={IconFilterCog}
onRemove={onRemove}
onRemove={handleRemoveClick}
/>
);
};

View File

@ -1,58 +1,16 @@
import { useCallback } from 'react';
import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown';
import { AdvancedFilterRootLevelViewFilterGroup } from '@/object-record/advanced-filter/components/AdvancedFilterRootLevelViewFilterGroup';
import { useRemoveRecordFilterGroup } from '@/object-record/record-filter-group/hooks/useRemoveRecordFilterGroup';
import { currentRecordFilterGroupsComponentState } from '@/object-record/record-filter-group/states/currentRecordFilterGroupsComponentState';
import { useRemoveRecordFilter } from '@/object-record/record-filter/hooks/useRemoveRecordFilter';
import { currentRecordFiltersComponentState } from '@/object-record/record-filter/states/currentRecordFiltersComponentState';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
import { AdvancedFilterChip } from '@/views/components/AdvancedFilterChip';
import { ADVANCED_FILTER_DROPDOWN_ID } from '@/views/constants/AdvancedFilterDropdownId';
import { isDefined } from 'twenty-shared';
export const AdvancedFilterDropdownButton = () => {
const { removeRecordFilterGroup } = useRemoveRecordFilterGroup();
const currentRecordFilterGroups = useRecoilComponentValueV2(
currentRecordFilterGroupsComponentState,
);
const currentRecordFilters = useRecoilComponentValueV2(
currentRecordFiltersComponentState,
);
const advancedRecordFilterIds = currentRecordFilters
.filter((recordFilter) => isDefined(recordFilter.recordFilterGroupId))
.map((recordFilter) => recordFilter.id);
const { removeRecordFilter } = useRemoveRecordFilter();
const removeAdvancedFilter = useCallback(async () => {
if (!advancedRecordFilterIds) {
throw new Error('No advanced view filters to remove');
}
const viewFilterGroupIds =
currentRecordFilterGroups?.map(
(recordFilterGroup) => recordFilterGroup.id,
) ?? [];
for (const viewFilterGroupId of viewFilterGroupIds) {
removeRecordFilterGroup(viewFilterGroupId);
}
for (const recordFilterId of advancedRecordFilterIds) {
removeRecordFilter({ recordFilterId });
}
}, [
advancedRecordFilterIds,
removeRecordFilterGroup,
removeRecordFilter,
currentRecordFilterGroups,
]);
const outermostRecordFilterGroupId = currentRecordFilterGroups.find(
(recordFilterGroup) => !recordFilterGroup.parentRecordFilterGroupId,
)?.id;
@ -64,17 +22,8 @@ export const AdvancedFilterDropdownButton = () => {
return (
<Dropdown
dropdownId={ADVANCED_FILTER_DROPDOWN_ID}
clickableComponent={
<AdvancedFilterChip
onRemove={removeAdvancedFilter}
advancedFilterCount={advancedRecordFilterIds?.length}
/>
}
dropdownComponents={
<AdvancedFilterRootLevelViewFilterGroup
rootLevelRecordFilterGroupId={outermostRecordFilterGroupId}
/>
}
clickableComponent={<AdvancedFilterChip />}
dropdownComponents={<AdvancedFilterRootLevelViewFilterGroup />}
dropdownHotkeyScope={{ scope: ADVANCED_FILTER_DROPDOWN_ID }}
dropdownOffset={{ y: 8, x: 0 }}
dropdownPlacement="bottom-start"

View File

@ -19,7 +19,9 @@ import { isPersistingViewFieldsState } from '@/views/states/isPersistingViewFiel
import { GraphQLView } from '@/views/types/GraphQLView';
import { View } from '@/views/types/View';
import { ViewGroup } from '@/views/types/ViewGroup';
import { ViewSort } from '@/views/types/ViewSort';
import { ViewType } from '@/views/types/ViewType';
import { duplicateViewFiltersAndViewFilterGroups } from '@/views/utils/duplicateViewFiltersAndViewFilterGroups';
import { mapRecordFilterGroupToViewFilterGroup } from '@/views/utils/mapRecordFilterGroupToViewFilterGroup';
import { mapRecordFilterToViewFilter } from '@/views/utils/mapRecordFilterToViewFilter';
import { mapRecordSortToViewSort } from '@/views/utils/mapRecordSortToViewSort';
@ -167,7 +169,7 @@ export const useCreateViewFromCurrentView = (viewBarComponentId?: string) => {
}
if (shouldCopyFiltersAndSortsAndAggregate === true) {
const viewFilterGroupsToCreate = currentRecordFilterGroups.map(
const viewFilterGroupsToCopy = currentRecordFilterGroups.map(
(recordFilterGroup) =>
mapRecordFilterGroupToViewFilterGroup({
recordFilterGroup,
@ -175,16 +177,31 @@ export const useCreateViewFromCurrentView = (viewBarComponentId?: string) => {
}),
);
const viewSortsToCreate = currentRecordSorts.map(
mapRecordSortToViewSort,
);
const viewFiltersToCreate = currentRecordFilters.map(
const viewFiltersToCopy = currentRecordFilters.map(
mapRecordFilterToViewFilter,
);
await createViewSortRecords(viewSortsToCreate, newView);
await createViewFilterRecords(viewFiltersToCreate, newView);
const {
duplicatedViewFilterGroups: viewFilterGroupsToCreate,
duplicatedViewFilters: viewFiltersToCreate,
} = duplicateViewFiltersAndViewFilterGroups({
viewFilterGroupsToDuplicate: viewFilterGroupsToCopy,
viewFiltersToDuplicate: viewFiltersToCopy,
});
const viewSortsToCreate = currentRecordSorts
.map(mapRecordSortToViewSort)
.map(
(viewSort) =>
({
...viewSort,
id: v4(),
}) satisfies ViewSort,
);
await createViewFilterGroupRecords(viewFilterGroupsToCreate, newView);
await createViewFilterRecords(viewFiltersToCreate, newView);
await createViewSortRecords(viewSortsToCreate, newView);
}
await findManyRecords();

View File

@ -0,0 +1,82 @@
import { ViewFilter } from '@/views/types/ViewFilter';
import { ViewFilterGroup } from '@/views/types/ViewFilterGroup';
import { isDefined } from 'twenty-shared';
import { v4 } from 'uuid';
export const duplicateViewFiltersAndViewFilterGroups = ({
viewFilterGroupsToDuplicate,
viewFiltersToDuplicate,
}: {
viewFiltersToDuplicate: ViewFilter[];
viewFilterGroupsToDuplicate: ViewFilterGroup[];
}) => {
const oldViewFilterGroupIdToNewViewFilterGroupIdMap = new Map<
string,
string
>();
for (const viewFilterGroupToCopy of viewFilterGroupsToDuplicate) {
oldViewFilterGroupIdToNewViewFilterGroupIdMap.set(
viewFilterGroupToCopy.id,
v4(),
);
}
const duplicatedViewFilterGroups = viewFilterGroupsToDuplicate.map(
(viewFilterGroupToCopy) => {
const newViewFilterGroupId =
oldViewFilterGroupIdToNewViewFilterGroupIdMap.get(
viewFilterGroupToCopy.id,
);
if (
!isDefined(viewFilterGroupToCopy.id) ||
!isDefined(newViewFilterGroupId)
) {
throw new Error(
`Failed to find view filter group to copy for id ${viewFilterGroupToCopy.id} this shouldn't happen`,
);
}
const parentViewFilterGroupIdToCopy =
viewFilterGroupToCopy.parentViewFilterGroupId;
const newParentViewFilterGroupId = isDefined(
parentViewFilterGroupIdToCopy,
)
? oldViewFilterGroupIdToNewViewFilterGroupIdMap.get(
parentViewFilterGroupIdToCopy,
)
: undefined;
const newViewFilterGroup = {
...viewFilterGroupToCopy,
id: newViewFilterGroupId,
parentViewFilterGroupId: newParentViewFilterGroupId,
} satisfies ViewFilterGroup;
return newViewFilterGroup;
},
);
const duplicatedViewFilters = viewFiltersToDuplicate.map((viewFilter) => {
const parentViewFilterGroupIdToCopy = viewFilter.viewFilterGroupId;
const newParentViewFilterGroupId = isDefined(parentViewFilterGroupIdToCopy)
? oldViewFilterGroupIdToNewViewFilterGroupIdMap.get(
parentViewFilterGroupIdToCopy,
)
: undefined;
return {
...viewFilter,
id: v4(),
viewFilterGroupId: newParentViewFilterGroupId,
} satisfies ViewFilter;
});
return {
duplicatedViewFilterGroups,
duplicatedViewFilters,
};
};

View File

@ -85,7 +85,7 @@ export class ViewFilterGroupWorkspaceEntity extends BaseWorkspaceEntity {
@WorkspaceField({
standardId: VIEW_FILTER_GROUP_STANDARD_FIELD_IDS.positionInViewFilterGroup,
type: FieldMetadataType.POSITION,
type: FieldMetadataType.NUMBER,
label: msg`Position in view filter group`,
description: msg`Position in the parent view filter group`,
icon: 'IconHierarchy2',

View File

@ -87,7 +87,7 @@ export class ViewFilterWorkspaceEntity extends BaseWorkspaceEntity {
@WorkspaceField({
standardId: VIEW_FILTER_STANDARD_FIELD_IDS.positionInViewFilterGroup,
type: FieldMetadataType.POSITION,
type: FieldMetadataType.NUMBER,
label: msg`Position in view filter group`,
description: msg`Position in the view filter group`,
icon: 'IconHierarchy2',