From 30e4fdbd06e3561ab00667214f6e39cf537f9b5c Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Fri, 7 Feb 2025 11:03:13 +0100 Subject: [PATCH] Refactor duplication of hard coded soft delete filter logic (#10058) This PR adds a useCheckIsSoftDeleteFilter hook instead of the undocumented in-place logic to retrieve the soft delete filter. Also took the opportunity to refactor a recent change of @prastoin with it. Split VariantFilterChip into SoftDeleteFilterChip and RecordFilterChip to separate concerns about this soft delete filtering. --- .../hooks/useDeleteMultipleRecordsAction.tsx | 10 +-- .../computeContextStoreFilters.test.ts | 1 - .../hooks/useCheckIsSoftDeleteFilter.ts | 35 ++++++++++ .../record-filter/types/RecordFilter.ts | 1 - .../hooks/useHandleToggleTrashColumnFilter.ts | 3 +- .../RecordTableEmptyStateSoftDelete.tsx | 12 ++-- .../isSoftDeleteFilterActiveComponentState.ts | 4 +- ...antFilterChip.tsx => RecordFilterChip.tsx} | 31 +------- .../views/components/SoftDeleteFilterChip.tsx | 48 +++++++++++++ .../views/components/SortOrFilterChip.tsx | 8 +-- .../views/components/ViewBarDetails.tsx | 70 +++++++++---------- 11 files changed, 138 insertions(+), 85 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts rename packages/twenty-front/src/modules/views/components/{VariantFilterChip.tsx => RecordFilterChip.tsx} (55%) create mode 100644 packages/twenty-front/src/modules/views/components/SoftDeleteFilterChip.tsx diff --git a/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/useDeleteMultipleRecordsAction.tsx b/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/useDeleteMultipleRecordsAction.tsx index ba7769eeb..49daa3f73 100644 --- a/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/useDeleteMultipleRecordsAction.tsx +++ b/packages/twenty-front/src/modules/action-menu/actions/record-actions/multiple-records/hooks/useDeleteMultipleRecordsAction.tsx @@ -8,8 +8,8 @@ import { BACKEND_BATCH_REQUEST_MAX_COUNT } from '@/object-record/constants/Backe import { DEFAULT_QUERY_PAGE_SIZE } from '@/object-record/constants/DefaultQueryPageSize'; import { useDeleteManyRecords } from '@/object-record/hooks/useDeleteManyRecords'; import { useLazyFetchAllRecords } from '@/object-record/hooks/useLazyFetchAllRecords'; +import { useCheckIsSoftDeleteFilter } from '@/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter'; import { useFilterValueDependencies } from '@/object-record/record-filter/hooks/useFilterValueDependencies'; -import { RecordFilterOperand } from '@/object-record/record-filter/types/RecordFilterOperand'; import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable'; import { ConfirmationModal } from '@/ui/layout/modal/components/ConfirmationModal'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; @@ -50,14 +50,10 @@ export const useDeleteMultipleRecordsAction: ActionHookWithObjectMetadataItem = filterValueDependencies, ); - const deletedAtFieldMetadata = objectMetadataItem.fields.find( - (field) => field.name === 'deletedAt', - ); + const { checkIsSoftDeleteFilter } = useCheckIsSoftDeleteFilter(); const isDeletedFilterActive = contextStoreFilters.some( - (filter) => - filter.fieldMetadataId === deletedAtFieldMetadata?.id && - filter.operand === RecordFilterOperand.IsNotEmpty, + checkIsSoftDeleteFilter, ); const { fetchAllRecords: fetchAllRecordIds } = useLazyFetchAllRecords({ diff --git a/packages/twenty-front/src/modules/context-store/utils/__tests__/computeContextStoreFilters.test.ts b/packages/twenty-front/src/modules/context-store/utils/__tests__/computeContextStoreFilters.test.ts index 65f0ed024..ec8f07171 100644 --- a/packages/twenty-front/src/modules/context-store/utils/__tests__/computeContextStoreFilters.test.ts +++ b/packages/twenty-front/src/modules/context-store/utils/__tests__/computeContextStoreFilters.test.ts @@ -45,7 +45,6 @@ describe('computeContextStoreFilters', () => { const contextStoreFilters: RecordFilter[] = [ { id: 'name-filter', - variant: 'default', fieldMetadataId: personObjectMetadataItem.fields.find( (field) => field.name === 'name', )!.id, 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 new file mode 100644 index 000000000..518e03162 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter.ts @@ -0,0 +1,35 @@ +import { useFilterableFieldMetadataItems } from '@/object-record/record-filter/hooks/useFilterableFieldMetadataItems'; +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'; +import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; +import { isDefined } from 'twenty-shared'; + +export const useCheckIsSoftDeleteFilter = () => { + const { filterableFieldMetadataItems } = useFilterableFieldMetadataItems(); + + const isSoftDeleteFilterActive = useRecoilComponentValueV2( + isSoftDeleteFilterActiveComponentState, + ); + + const checkIsSoftDeleteFilter = (recordFilter: RecordFilter) => { + const foundFieldMetadataItem = filterableFieldMetadataItems.find( + (fieldMetadataItem) => + fieldMetadataItem.id === recordFilter.fieldMetadataId, + ); + + if (!isDefined(foundFieldMetadataItem)) { + throw new Error( + `Field metadata item not found for field metadata id: ${recordFilter.fieldMetadataId}`, + ); + } + + return ( + foundFieldMetadataItem.name === 'deletedAt' && + isSoftDeleteFilterActive && + recordFilter.operand === RecordFilterOperand.IsNotEmpty + ); + }; + + return { checkIsSoftDeleteFilter }; +}; diff --git a/packages/twenty-front/src/modules/object-record/record-filter/types/RecordFilter.ts b/packages/twenty-front/src/modules/object-record/record-filter/types/RecordFilter.ts index 1065310be..e837c0769 100644 --- a/packages/twenty-front/src/modules/object-record/record-filter/types/RecordFilter.ts +++ b/packages/twenty-front/src/modules/object-record/record-filter/types/RecordFilter.ts @@ -4,7 +4,6 @@ import { ViewFilterOperand } from '@/views/types/ViewFilterOperand'; export type RecordFilter = { id: string; - variant?: 'default' | 'danger'; fieldMetadataId: string; value: string; displayValue: string; diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleToggleTrashColumnFilter.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleToggleTrashColumnFilter.ts index 523fd072f..8cda40e45 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleToggleTrashColumnFilter.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleToggleTrashColumnFilter.ts @@ -59,10 +59,11 @@ export const useHandleToggleTrashColumnFilter = ({ const newFilter: RecordFilter = { id: v4(), - variant: 'danger', fieldMetadataId: trashFieldMetadata.id, operand: ViewFilterOperand.IsNotEmpty, displayValue: '', + type: filterType, + label: `Deleted`, definition: { label: `Deleted`, iconName: 'IconTrash', diff --git a/packages/twenty-front/src/modules/object-record/record-table/empty-state/components/RecordTableEmptyStateSoftDelete.tsx b/packages/twenty-front/src/modules/object-record/record-table/empty-state/components/RecordTableEmptyStateSoftDelete.tsx index 6a31da197..71aba34a1 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/empty-state/components/RecordTableEmptyStateSoftDelete.tsx +++ b/packages/twenty-front/src/modules/object-record/record-table/empty-state/components/RecordTableEmptyStateSoftDelete.tsx @@ -1,6 +1,7 @@ import { IconFilterOff } from 'twenty-ui'; import { useObjectLabel } from '@/object-metadata/hooks/useObjectLabel'; +import { useCheckIsSoftDeleteFilter } from '@/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter'; import { useRemoveRecordFilter } from '@/object-record/record-filter/hooks/useRemoveRecordFilter'; import { useHandleToggleTrashColumnFilter } from '@/object-record/record-index/hooks/useHandleToggleTrashColumnFilter'; import { useRecordTableContextOrThrow } from '@/object-record/record-table/contexts/RecordTableContext'; @@ -8,6 +9,7 @@ import { RecordTableEmptyStateDisplay } from '@/object-record/record-table/empty import { tableFiltersComponentState } from '@/object-record/record-table/states/tableFiltersComponentState'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { useDeleteCombinedViewFilters } from '@/views/hooks/useDeleteCombinedViewFilters'; +import { isDefined } from 'twenty-shared'; export const RecordTableEmptyStateSoftDelete = () => { const { objectMetadataItem, objectNameSingular, recordTableId } = @@ -28,12 +30,12 @@ export const RecordTableEmptyStateSoftDelete = () => { const { removeRecordFilter } = useRemoveRecordFilter(); - const handleButtonClick = async () => { - const deletedFilter = tableFilters.find( - (filter) => filter.label === 'Deleted' && filter.operand === 'isNotEmpty', - ); + const { checkIsSoftDeleteFilter } = useCheckIsSoftDeleteFilter(); - if (!deletedFilter) { + const handleButtonClick = async () => { + const deletedFilter = tableFilters.find(checkIsSoftDeleteFilter); + + if (!isDefined(deletedFilter)) { throw new Error('Deleted filter not found'); } diff --git a/packages/twenty-front/src/modules/object-record/record-table/states/isSoftDeleteFilterActiveComponentState.ts b/packages/twenty-front/src/modules/object-record/record-table/states/isSoftDeleteFilterActiveComponentState.ts index 24fbfa680..16e77936f 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/states/isSoftDeleteFilterActiveComponentState.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/states/isSoftDeleteFilterActiveComponentState.ts @@ -1,9 +1,9 @@ -import { RecordTableComponentInstanceContext } from '@/object-record/record-table/states/context/RecordTableComponentInstanceContext'; +import { RecordFiltersComponentInstanceContext } from '@/object-record/record-filter/states/context/RecordFiltersComponentInstanceContext'; import { createComponentStateV2 } from '@/ui/utilities/state/component-state/utils/createComponentStateV2'; export const isSoftDeleteFilterActiveComponentState = createComponentStateV2({ key: 'isSoftDeleteFilterActiveComponentState', defaultValue: false, - componentInstanceContext: RecordTableComponentInstanceContext, + componentInstanceContext: RecordFiltersComponentInstanceContext, }); diff --git a/packages/twenty-front/src/modules/views/components/VariantFilterChip.tsx b/packages/twenty-front/src/modules/views/components/RecordFilterChip.tsx similarity index 55% rename from packages/twenty-front/src/modules/views/components/VariantFilterChip.tsx rename to packages/twenty-front/src/modules/views/components/RecordFilterChip.tsx index 32077b7f8..f05a89e53 100644 --- a/packages/twenty-front/src/modules/views/components/VariantFilterChip.tsx +++ b/packages/twenty-front/src/modules/views/components/RecordFilterChip.tsx @@ -1,36 +1,18 @@ import { useIcons } from 'twenty-ui'; import { useFieldMetadataItemById } from '@/object-metadata/hooks/useFieldMetadataItemById'; -import { useObjectNameSingularFromPlural } from '@/object-metadata/hooks/useObjectNameSingularFromPlural'; import { useRemoveRecordFilter } from '@/object-record/record-filter/hooks/useRemoveRecordFilter'; import { RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; -import { useHandleToggleTrashColumnFilter } from '@/object-record/record-index/hooks/useHandleToggleTrashColumnFilter'; import { SortOrFilterChip } from '@/views/components/SortOrFilterChip'; import { useDeleteCombinedViewFilters } from '@/views/hooks/useDeleteCombinedViewFilters'; -import { useParams } from 'react-router-dom'; -type VariantFilterChipProps = { +type RecordFilterChipProps = { recordFilter: RecordFilter; - viewBarId: string; }; -export const VariantFilterChip = ({ - recordFilter, - viewBarId, -}: VariantFilterChipProps) => { +export const RecordFilterChip = ({ recordFilter }: RecordFilterChipProps) => { const { deleteCombinedViewFilter } = useDeleteCombinedViewFilters(); - const { objectNamePlural } = useParams(); - - const { objectNameSingular } = useObjectNameSingularFromPlural({ - objectNamePlural: objectNamePlural ?? '', - }); - - const { toggleSoftDeleteFilterState } = useHandleToggleTrashColumnFilter({ - objectNameSingular, - viewBarId, - }); - const { fieldMetadataItem } = useFieldMetadataItemById( recordFilter.fieldMetadataId, ); @@ -44,20 +26,11 @@ export const VariantFilterChip = ({ const handleRemoveClick = () => { deleteCombinedViewFilter(recordFilter.id); removeRecordFilter(recordFilter.fieldMetadataId); - - if ( - recordFilter.label === 'Deleted' && - recordFilter.operand === 'isNotEmpty' - ) { - toggleSoftDeleteFilterState(false); - } }; return ( { + const { deleteCombinedViewFilter } = useDeleteCombinedViewFilters(); + + const setIsSoftDeleteFilterActive = useSetRecoilComponentStateV2( + isSoftDeleteFilterActiveComponentState, + viewBarId, + ); + + const { removeRecordFilter } = useRemoveRecordFilter(); + + const { getIcon } = useIcons(); + + const handleRemoveClick = () => { + deleteCombinedViewFilter(recordFilter.id); + removeRecordFilter(recordFilter.fieldMetadataId); + + setIsSoftDeleteFilterActive(false); + }; + + const ChipIcon = getIcon('IconTrash'); + + return ( + + ); +}; diff --git a/packages/twenty-front/src/modules/views/components/SortOrFilterChip.tsx b/packages/twenty-front/src/modules/views/components/SortOrFilterChip.tsx index de40713b6..d83f29c7f 100644 --- a/packages/twenty-front/src/modules/views/components/SortOrFilterChip.tsx +++ b/packages/twenty-front/src/modules/views/components/SortOrFilterChip.tsx @@ -2,7 +2,7 @@ import { useTheme } from '@emotion/react'; import styled from '@emotion/styled'; import { IconComponent, IconX } from 'twenty-ui'; -const StyledChip = styled.div<{ variant: SortOrFitlerChipVariant }>` +const StyledChip = styled.div<{ variant: SortOrFilterChipVariant }>` align-items: center; background-color: ${({ theme, variant }) => { switch (variant) { @@ -55,7 +55,7 @@ const StyledIcon = styled.div` display: flex; `; -const StyledDelete = styled.button<{ variant: SortOrFitlerChipVariant }>` +const StyledDelete = styled.button<{ variant: SortOrFilterChipVariant }>` box-sizing: border-box; height: 20px; width: 20px; @@ -89,12 +89,12 @@ const StyledLabelKey = styled.div` font-weight: ${({ theme }) => theme.font.weight.medium}; `; -type SortOrFitlerChipVariant = 'default' | 'danger'; +export type SortOrFilterChipVariant = 'default' | 'danger'; type SortOrFilterChipProps = { labelKey?: string; labelValue: string; - variant?: SortOrFitlerChipVariant; + variant?: SortOrFilterChipVariant; Icon?: IconComponent; onRemove: () => void; onClick?: () => void; diff --git a/packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx b/packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx index 2fbfb10ff..7939ff949 100644 --- a/packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx +++ b/packages/twenty-front/src/modules/views/components/ViewBarDetails.tsx @@ -13,7 +13,9 @@ import { EditableSortChip } from '@/views/components/EditableSortChip'; import { ViewBarFilterEffect } from '@/views/components/ViewBarFilterEffect'; import { useViewFromQueryParams } from '@/views/hooks/internal/useViewFromQueryParams'; +import { useCheckIsSoftDeleteFilter } from '@/object-record/record-filter/hooks/useCheckIsSoftDeleteFilter'; import { currentRecordFiltersComponentState } from '@/object-record/record-filter/states/currentRecordFiltersComponentState'; +import { SoftDeleteFilterChip } from '@/views/components/SoftDeleteFilterChip'; import { useApplyCurrentViewFiltersToCurrentRecordFilters } from '@/views/hooks/useApplyCurrentViewFiltersToCurrentRecordFilters'; import { useAreViewFiltersDifferentFromRecordFilters } from '@/views/hooks/useAreViewFiltersDifferentFromRecordFilters'; import { useAreViewSortsDifferentFromRecordSorts } from '@/views/hooks/useAreViewSortsDifferentFromRecordSorts'; @@ -22,8 +24,8 @@ import { useResetUnsavedViewStates } from '@/views/hooks/useResetUnsavedViewStat import { availableSortDefinitionsComponentState } from '@/views/states/availableSortDefinitionsComponentState'; import { isViewBarExpandedComponentState } from '@/views/states/isViewBarExpandedComponentState'; import { mapViewSortsToSorts } from '@/views/utils/mapViewSortsToSorts'; +import { isNonEmptyArray } from '@sniptt/guards'; import { isDefined } from 'twenty-shared'; -import { VariantFilterChip } from './VariantFilterChip'; export type ViewBarDetailsProps = { hasFilterButton?: boolean; @@ -144,22 +146,19 @@ export const ViewBarDetails = ({ viewSortsAreDifferentFromRecordSorts) && !hasFiltersQueryParams; - const otherViewFilters = useMemo(() => { - return currentRecordFilters.filter( - (viewFilter) => - viewFilter.variant && - viewFilter.variant !== 'default' && - !viewFilter.viewFilterGroupId, - ); - }, [currentRecordFilters]); + const { checkIsSoftDeleteFilter } = useCheckIsSoftDeleteFilter(); - const defaultViewFilters = useMemo(() => { + const softDeleteFilter = currentRecordFilters.find((recordFilter) => + checkIsSoftDeleteFilter(recordFilter), + ); + + const recordFilters = useMemo(() => { return currentRecordFilters.filter( - (viewFilter) => - (!viewFilter.variant || viewFilter.variant === 'default') && - !viewFilter.viewFilterGroupId, + (recordFilter) => + !recordFilter.viewFilterGroupId && + !checkIsSoftDeleteFilter(recordFilter), ); - }, [currentRecordFilters]); + }, [currentRecordFilters, checkIsSoftDeleteFilter]); const { applyCurrentViewFiltersToCurrentRecordFilters } = useApplyCurrentViewFiltersToCurrentRecordFilters(); @@ -190,45 +189,46 @@ export const ViewBarDetails = ({ - {otherViewFilters.map((viewFilter) => ( - - ))} - {!!otherViewFilters.length && - !!currentViewWithCombinedFiltersAndSorts?.viewSorts?.length && ( - - - - )} + )} + {isDefined(softDeleteFilter) && ( + + + + )} {mapViewSortsToSorts( currentViewWithCombinedFiltersAndSorts?.viewSorts ?? [], availableSortDefinitions, ).map((sort) => ( ))} - {!!currentViewWithCombinedFiltersAndSorts?.viewSorts?.length && - !!defaultViewFilters.length && ( + {isNonEmptyArray(recordFilters) && + isNonEmptyArray( + currentViewWithCombinedFiltersAndSorts?.viewSorts, + ) && ( )} {showAdvancedFilterDropdownButton && } - {defaultViewFilters.map((viewFilter) => ( + {recordFilters.map((recordFilter) => ( - - + +