diff --git a/packages/twenty-front/src/modules/page-layout/hooks/useSavePageLayout.ts b/packages/twenty-front/src/modules/page-layout/hooks/useSavePageLayout.ts index a1a39718ea..1a91d8fa99 100644 --- a/packages/twenty-front/src/modules/page-layout/hooks/useSavePageLayout.ts +++ b/packages/twenty-front/src/modules/page-layout/hooks/useSavePageLayout.ts @@ -1,3 +1,4 @@ +import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems'; import { useCreatePendingFieldsWidgetViews } from '@/page-layout/hooks/useCreatePendingFieldsWidgetViews'; import { useCreatePendingRecordTableWidgetViews } from '@/page-layout/hooks/useCreatePendingRecordTableWidgetViews'; import { useUpdatePageLayoutWithTabsAndWidgets } from '@/page-layout/hooks/useUpdatePageLayoutWithTabsAndWidgets'; @@ -8,6 +9,7 @@ import { pageLayoutPersistedComponentState } from '@/page-layout/states/pageLayo import { type PageLayout } from '@/page-layout/types/PageLayout'; import { convertPageLayoutDraftToUpdateInput } from '@/page-layout/utils/convertPageLayoutDraftToUpdateInput'; import { convertPageLayoutToTabLayouts } from '@/page-layout/utils/convertPageLayoutToTabLayouts'; +import { sanitizeChartFiltersInPageLayoutDraft } from '@/page-layout/utils/sanitizeChartFiltersInPageLayoutDraft'; import { transformPageLayout } from '@/page-layout/utils/transformPageLayout'; import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow'; import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState'; @@ -46,6 +48,8 @@ export const useSavePageLayout = (pageLayoutIdFromProps: string) => { const { createPendingRecordTableWidgetViews } = useCreatePendingRecordTableWidgetViews(); + const { objectMetadataItems } = useObjectMetadataItems(); + const store = useStore(); const savePageLayout = useCallback(async () => { @@ -53,7 +57,26 @@ export const useSavePageLayout = (pageLayoutIdFromProps: string) => { await createPendingRecordTableWidgetViews(pageLayoutId); const pageLayoutDraft = store.get(pageLayoutDraftCallbackState); - const updateInput = convertPageLayoutDraftToUpdateInput(pageLayoutDraft); + + const validFieldMetadataIdsByObjectMetadataId = new Map( + objectMetadataItems.map((objectMetadataItem) => [ + objectMetadataItem.id, + new Set( + objectMetadataItem.fields + .filter((fieldMetadataItem) => fieldMetadataItem.isActive) + .map((fieldMetadataItem) => fieldMetadataItem.id), + ), + ]), + ); + + const sanitizedPageLayoutDraft = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft, + validFieldMetadataIdsByObjectMetadataId, + }); + + const updateInput = convertPageLayoutDraftToUpdateInput( + sanitizedPageLayoutDraft, + ); const result = await updatePageLayoutWithTabsAndWidgets( pageLayoutId, @@ -80,6 +103,7 @@ export const useSavePageLayout = (pageLayoutIdFromProps: string) => { }, [ createPendingFieldsWidgetViews, createPendingRecordTableWidgetViews, + objectMetadataItems, pageLayoutCurrentLayoutsCallbackState, pageLayoutDraftCallbackState, pageLayoutId, diff --git a/packages/twenty-front/src/modules/page-layout/utils/__tests__/sanitizeChartFiltersInPageLayoutDraft.test.ts b/packages/twenty-front/src/modules/page-layout/utils/__tests__/sanitizeChartFiltersInPageLayoutDraft.test.ts new file mode 100644 index 0000000000..62929121c4 --- /dev/null +++ b/packages/twenty-front/src/modules/page-layout/utils/__tests__/sanitizeChartFiltersInPageLayoutDraft.test.ts @@ -0,0 +1,218 @@ +import { type DraftPageLayout } from '@/page-layout/types/DraftPageLayout'; +import { type PageLayoutTab } from '@/page-layout/types/PageLayoutTab'; +import { type PageLayoutWidget } from '@/page-layout/types/PageLayoutWidget'; +import { sanitizeChartFiltersInPageLayoutDraft } from '@/page-layout/utils/sanitizeChartFiltersInPageLayoutDraft'; +import { type RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; +import { type ChartFilters } from '@/side-panel/pages/page-layout/types/ChartFilters'; +import { + RecordFilterGroupLogicalOperator, + ViewFilterOperand, +} from 'twenty-shared/types'; +import { + FieldMetadataType, + PageLayoutType, +} from '~/generated-metadata/graphql'; +import { + TEST_BAR_CHART_CONFIGURATION, + TEST_FIELDS_CONFIGURATION, + TEST_FIELD_METADATA_ID_1, + TEST_FIELD_METADATA_ID_2, + TEST_OBJECT_METADATA_ID, + createTestWidget, +} from '~/testing/mock-data/widget-configurations'; + +const ACTIVE_FIELD_ID = TEST_FIELD_METADATA_ID_1; +const DELETED_FIELD_ID = TEST_FIELD_METADATA_ID_2; + +const buildRecordFilter = ( + overrides: Partial & + Pick, +): RecordFilter => ({ + value: '', + displayValue: '', + type: FieldMetadataType.NUMBER, + operand: ViewFilterOperand.IS, + label: 'Filter', + ...overrides, +}); + +const buildChartFilters = (recordFilters: RecordFilter[]): ChartFilters => ({ + recordFilters, + recordFilterGroups: [], +}); + +const buildDraft = (widgets: PageLayoutWidget[]): DraftPageLayout => { + const tab: PageLayoutTab = { + __typename: 'PageLayoutTab', + id: 'tab-1', + applicationId: 'test-application-id', + title: 'Tab 1', + position: 0, + pageLayoutId: 'page-layout-1', + isActive: true, + createdAt: '2024-01-01', + updatedAt: '2024-01-01', + widgets, + }; + + return { + id: 'page-layout-1', + name: 'Test Page Layout', + type: PageLayoutType.DASHBOARD, + objectMetadataId: TEST_OBJECT_METADATA_ID, + defaultTabToFocusOnMobileAndSidePanelId: null, + tabs: [tab], + }; +}; + +const buildValidFieldsMap = (fieldIds: string[]) => + new Map>([[TEST_OBJECT_METADATA_ID, new Set(fieldIds)]]); + +describe('sanitizeChartFiltersInPageLayoutDraft', () => { + it('should drop chart filter rules that reference deactivated or deleted fields on save', () => { + const validFilter = buildRecordFilter({ + id: 'filter-1', + fieldMetadataId: ACTIVE_FIELD_ID, + }); + + const widget = createTestWidget({ + id: 'chart-widget', + configuration: { + ...TEST_BAR_CHART_CONFIGURATION, + filter: buildChartFilters([ + validFilter, + buildRecordFilter({ + id: 'filter-2', + fieldMetadataId: DELETED_FIELD_ID, + }), + ]), + }, + }); + + const result = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft: buildDraft([widget]), + validFieldMetadataIdsByObjectMetadataId: buildValidFieldsMap([ + ACTIVE_FIELD_ID, + ]), + }); + + const sanitizedConfiguration = result.tabs[0].widgets[0].configuration as { + filter: ChartFilters; + }; + + expect(sanitizedConfiguration.filter.recordFilters).toEqual([validFilter]); + }); + + it('should keep chart filters untouched when all referenced fields are still active', () => { + const validFilter = buildRecordFilter({ + id: 'filter-1', + fieldMetadataId: ACTIVE_FIELD_ID, + }); + + const widget = createTestWidget({ + id: 'chart-widget', + configuration: { + ...TEST_BAR_CHART_CONFIGURATION, + filter: buildChartFilters([validFilter]), + }, + }); + + const result = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft: buildDraft([widget]), + validFieldMetadataIdsByObjectMetadataId: buildValidFieldsMap([ + ACTIVE_FIELD_ID, + ]), + }); + + const sanitizedConfiguration = result.tabs[0].widgets[0].configuration as { + filter: ChartFilters; + }; + + expect(sanitizedConfiguration.filter.recordFilters).toEqual([validFilter]); + }); + + it('should drop orphaned filter groups left behind once invalid filters are removed', () => { + const widget = createTestWidget({ + id: 'chart-widget', + configuration: { + ...TEST_BAR_CHART_CONFIGURATION, + filter: { + recordFilters: [ + buildRecordFilter({ + id: 'filter-1', + fieldMetadataId: DELETED_FIELD_ID, + recordFilterGroupId: 'root', + }), + ], + recordFilterGroups: [ + { + id: 'root', + parentRecordFilterGroupId: undefined, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + ], + } satisfies ChartFilters, + }, + }); + + const result = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft: buildDraft([widget]), + validFieldMetadataIdsByObjectMetadataId: buildValidFieldsMap([ + ACTIVE_FIELD_ID, + ]), + }); + + const sanitizedConfiguration = result.tabs[0].widgets[0].configuration as { + filter: ChartFilters; + }; + + expect(sanitizedConfiguration.filter.recordFilters).toEqual([]); + expect(sanitizedConfiguration.filter.recordFilterGroups).toEqual([]); + }); + + it('should leave non-chart widgets untouched', () => { + const widget = createTestWidget({ + id: 'fields-widget', + configuration: TEST_FIELDS_CONFIGURATION, + }); + + const result = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft: buildDraft([widget]), + validFieldMetadataIdsByObjectMetadataId: buildValidFieldsMap([ + ACTIVE_FIELD_ID, + ]), + }); + + expect(result.tabs[0].widgets[0].configuration).toEqual( + TEST_FIELDS_CONFIGURATION, + ); + }); + + it('should leave chart filters untouched when the object metadata cannot be resolved', () => { + const invalidFilter = buildRecordFilter({ + id: 'filter-1', + fieldMetadataId: DELETED_FIELD_ID, + }); + + const widget = createTestWidget({ + id: 'chart-widget', + configuration: { + ...TEST_BAR_CHART_CONFIGURATION, + filter: buildChartFilters([invalidFilter]), + }, + }); + + const result = sanitizeChartFiltersInPageLayoutDraft({ + pageLayoutDraft: buildDraft([widget]), + validFieldMetadataIdsByObjectMetadataId: new Map(), + }); + + const sanitizedConfiguration = result.tabs[0].widgets[0].configuration as { + filter: ChartFilters; + }; + + expect(sanitizedConfiguration.filter.recordFilters).toEqual([ + invalidFilter, + ]); + }); +}); diff --git a/packages/twenty-front/src/modules/page-layout/utils/sanitizeChartFiltersInPageLayoutDraft.ts b/packages/twenty-front/src/modules/page-layout/utils/sanitizeChartFiltersInPageLayoutDraft.ts new file mode 100644 index 0000000000..f128400e00 --- /dev/null +++ b/packages/twenty-front/src/modules/page-layout/utils/sanitizeChartFiltersInPageLayoutDraft.ts @@ -0,0 +1,55 @@ +import { type DraftPageLayout } from '@/page-layout/types/DraftPageLayout'; +import { type ChartFilters } from '@/side-panel/pages/page-layout/types/ChartFilters'; +import { dropChartRecordFiltersWithDeletedFields } from '@/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields'; +import { isWidgetConfigurationOfTypeGraph } from '@/side-panel/pages/page-layout/utils/isWidgetConfigurationOfTypeGraph'; +import { isDefined } from 'twenty-shared/utils'; + +export const sanitizeChartFiltersInPageLayoutDraft = ({ + pageLayoutDraft, + validFieldMetadataIdsByObjectMetadataId, +}: { + pageLayoutDraft: DraftPageLayout; + validFieldMetadataIdsByObjectMetadataId: Map>; +}): DraftPageLayout => { + return { + ...pageLayoutDraft, + tabs: pageLayoutDraft.tabs.map((tab) => ({ + ...tab, + widgets: tab.widgets.map((widget) => { + if (!isWidgetConfigurationOfTypeGraph(widget.configuration)) { + return widget; + } + + const chartFilters = widget.configuration.filter as + | ChartFilters + | null + | undefined; + + if (!isDefined(chartFilters)) { + return widget; + } + + const validFieldMetadataIds = isDefined(widget.objectMetadataId) + ? validFieldMetadataIdsByObjectMetadataId.get(widget.objectMetadataId) + : undefined; + + if (!isDefined(validFieldMetadataIds)) { + return widget; + } + + const sanitizedChartFilters = dropChartRecordFiltersWithDeletedFields({ + chartFilters, + validFieldMetadataIds, + }); + + return { + ...widget, + configuration: { + ...widget.configuration, + filter: sanitizedChartFilters, + }, + }; + }), + })), + }; +}; diff --git a/packages/twenty-front/src/modules/page-layout/widgets/graph/hooks/useGraphWidgetQueryCommon.ts b/packages/twenty-front/src/modules/page-layout/widgets/graph/hooks/useGraphWidgetQueryCommon.ts index 63482db784..cb803334b7 100644 --- a/packages/twenty-front/src/modules/page-layout/widgets/graph/hooks/useGraphWidgetQueryCommon.ts +++ b/packages/twenty-front/src/modules/page-layout/widgets/graph/hooks/useGraphWidgetQueryCommon.ts @@ -46,7 +46,9 @@ export const useGraphWidgetQueryCommon = ({ ); const objectFieldMetadataIds = new Set( - objectMetadataItem.fields.map((field: { id: string }) => field.id), + objectMetadataItem.fields + .filter((field) => field.isActive) + .map((field) => field.id), ); const { recordFilters: sanitizedRecordFilters } = diff --git a/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersDeletedFieldsWarning.tsx b/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersDeletedFieldsWarning.tsx index 9704410176..ce38424cf2 100644 --- a/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersDeletedFieldsWarning.tsx +++ b/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersDeletedFieldsWarning.tsx @@ -24,7 +24,7 @@ export const ChartFiltersDeletedFieldsWarning = ({ return ( ); }; diff --git a/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersSettings.tsx b/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersSettings.tsx index 83a682f408..b1ddf3d6cd 100644 --- a/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersSettings.tsx +++ b/packages/twenty-front/src/modules/side-panel/pages/page-layout/components/ChartFiltersSettings.tsx @@ -4,6 +4,7 @@ import { usePageLayoutIdFromContextStore } from '@/side-panel/pages/page-layout/ import { useUpdateCurrentWidgetConfig } from '@/side-panel/pages/page-layout/hooks/useUpdateCurrentWidgetConfig'; import { type ChartWidget } from '@/side-panel/pages/page-layout/types/ChartWidget'; import { type ChartWidgetConfiguration } from '@/side-panel/pages/page-layout/types/ChartWidgetConfiguration'; +import { dropChartRecordFiltersWithDeletedFields } from '@/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields'; import { getChartFiltersSettingsInstanceId } from '@/side-panel/pages/page-layout/utils/getChartFiltersSettingsInstanceId'; import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; @@ -75,12 +76,23 @@ export const ChartFiltersSettings = ({ const existingRecordFilters = store.get(currentRecordFilters); const existingRecordFilterGroups = store.get(currentRecordFilterGroups); + const { + recordFilters: sanitizedRecordFilters, + recordFilterGroups: sanitizedRecordFilterGroups, + } = dropChartRecordFiltersWithDeletedFields({ + chartFilters: { + recordFilters: existingRecordFilters, + recordFilterGroups: existingRecordFilterGroups, + }, + validFieldMetadataIds, + }); + updateCurrentWidgetConfig({ objectMetadataId: objectMetadataItem.id, configToUpdate: { filter: { - recordFilters: existingRecordFilters, - recordFilterGroups: existingRecordFilterGroups, + recordFilters: sanitizedRecordFilters, + recordFilterGroups: sanitizedRecordFilterGroups, }, } satisfies Partial, }); diff --git a/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/__tests__/dropChartRecordFiltersWithDeletedFields.test.ts b/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/__tests__/dropChartRecordFiltersWithDeletedFields.test.ts index 37616800ac..5149d2c9de 100644 --- a/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/__tests__/dropChartRecordFiltersWithDeletedFields.test.ts +++ b/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/__tests__/dropChartRecordFiltersWithDeletedFields.test.ts @@ -29,9 +29,15 @@ describe('dropChartRecordFiltersWithDeletedFields', () => { ]); }); - it('should preserve record filter groups', () => { + it('should preserve record filter groups that still contain valid filters', () => { const chartFilters: ChartFilters = { - recordFilters: [], + recordFilters: [ + { + id: 'filter-1', + fieldMetadataId: 'valid-field', + recordFilterGroupId: 'root', + }, + ], recordFilterGroups: [ { id: 'root', @@ -43,7 +49,7 @@ describe('dropChartRecordFiltersWithDeletedFields', () => { const result = dropChartRecordFiltersWithDeletedFields({ chartFilters, - validFieldMetadataIds: new Set(), + validFieldMetadataIds: new Set(['valid-field']), }); expect(result.recordFilterGroups).toEqual([ @@ -81,4 +87,112 @@ describe('dropChartRecordFiltersWithDeletedFields', () => { expect(result.recordFilters).toEqual([]); }); + + it('should remove orphaned root group when all its filters are dropped', () => { + const chartFilters: ChartFilters = { + recordFilters: [ + { + id: 'filter-1', + fieldMetadataId: 'deleted-field', + recordFilterGroupId: 'root', + }, + ], + recordFilterGroups: [ + { + id: 'root', + parentRecordFilterGroupId: undefined, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + ], + } as ChartFilters; + + const result = dropChartRecordFiltersWithDeletedFields({ + chartFilters, + validFieldMetadataIds: new Set(), + }); + + expect(result.recordFilters).toEqual([]); + expect(result.recordFilterGroups).toEqual([]); + }); + + it('should remove orphaned child group when its only filter is dropped, but keep root group if it has other valid children', () => { + const chartFilters: ChartFilters = { + recordFilters: [ + { + id: 'filter-1', + fieldMetadataId: 'valid-field', + recordFilterGroupId: 'root', + }, + { + id: 'filter-2', + fieldMetadataId: 'deleted-field', + recordFilterGroupId: 'child', + }, + ], + recordFilterGroups: [ + { + id: 'root', + parentRecordFilterGroupId: undefined, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + { + id: 'child', + parentRecordFilterGroupId: 'root', + logicalOperator: RecordFilterGroupLogicalOperator.OR, + }, + ], + } as ChartFilters; + + const result = dropChartRecordFiltersWithDeletedFields({ + chartFilters, + validFieldMetadataIds: new Set(['valid-field']), + }); + + expect(result.recordFilters).toEqual([ + { + id: 'filter-1', + fieldMetadataId: 'valid-field', + recordFilterGroupId: 'root', + }, + ]); + expect(result.recordFilterGroups).toEqual([ + { + id: 'root', + parentRecordFilterGroupId: undefined, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + ]); + }); + + it('should remove both child and root group when all filters are dropped', () => { + const chartFilters: ChartFilters = { + recordFilters: [ + { + id: 'filter-1', + fieldMetadataId: 'deleted-field', + recordFilterGroupId: 'child', + }, + ], + recordFilterGroups: [ + { + id: 'root', + parentRecordFilterGroupId: undefined, + logicalOperator: RecordFilterGroupLogicalOperator.AND, + }, + { + id: 'child', + parentRecordFilterGroupId: 'root', + logicalOperator: RecordFilterGroupLogicalOperator.OR, + }, + ], + } as ChartFilters; + + const result = dropChartRecordFiltersWithDeletedFields({ + chartFilters, + validFieldMetadataIds: new Set(), + }); + + expect(result.recordFilters).toEqual([]); + expect(result.recordFilterGroups).toEqual([]); + }); }); diff --git a/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields.ts b/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields.ts index bda247383e..fd6209a9ca 100644 --- a/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields.ts +++ b/packages/twenty-front/src/modules/side-panel/pages/page-layout/utils/dropChartRecordFiltersWithDeletedFields.ts @@ -1,4 +1,5 @@ import { type ChartFilters } from '@/side-panel/pages/page-layout/types/ChartFilters'; +import { isDefined } from 'twenty-shared/utils'; export const dropChartRecordFiltersWithDeletedFields = ({ chartFilters, @@ -6,9 +7,52 @@ export const dropChartRecordFiltersWithDeletedFields = ({ }: { chartFilters: ChartFilters; validFieldMetadataIds: Set; -}): ChartFilters => ({ - ...chartFilters, - recordFilters: (chartFilters.recordFilters ?? []).filter((recordFilter) => - validFieldMetadataIds.has(recordFilter.fieldMetadataId), - ), -}); +}): ChartFilters => { + const validRecordFilters = (chartFilters.recordFilters ?? []).filter( + (recordFilter) => validFieldMetadataIds.has(recordFilter.fieldMetadataId), + ); + + const recordFilterGroups = chartFilters.recordFilterGroups ?? []; + + let remainingGroups = [...recordFilterGroups]; + let changed = true; + + while (changed) { + changed = false; + const remainingGroupIds = new Set(remainingGroups.map((g) => g.id)); + + const nonEmptyGroupIds = new Set( + [ + ...validRecordFilters + .map((f) => f.recordFilterGroupId) + .filter(isDefined), + ...remainingGroups + .map((g) => g.parentRecordFilterGroupId) + .filter(isDefined), + ].filter((id) => remainingGroupIds.has(id)), + ); + + const nextGroups = remainingGroups.filter((g) => + nonEmptyGroupIds.has(g.id), + ); + + if (nextGroups.length !== remainingGroups.length) { + remainingGroups = nextGroups; + changed = true; + } + } + + const validRecordFiltersWithDroppedOrphanedGroups = validRecordFilters.filter( + (f) => { + const groupId = f.recordFilterGroupId; + if (!isDefined(groupId)) return true; + return remainingGroups.some((g) => g.id === groupId); + }, + ); + + return { + ...chartFilters, + recordFilters: validRecordFiltersWithDroppedOrphanedGroups, + recordFilterGroups: remainingGroups, + }; +};