Fix/sanitize chart filters on save (#21958)

# Fix: sanitize chart filters referencing deactivated/deleted fields

## Summary

When a field used in a chart (graph) widget filter was later
**deactivated or deleted**, saving the page layout failed with a backend
error such as:

> Chart "...": One of the chart filters uses "...", but it was deleted.
Please remove or replace this filter rule.

This happened even after the user tried to remove the offending filter
rule, because the invalid filter could still end up in the saved
configuration. This PR makes invalid chart filters get cleaned up
reliably — both as the user edits filters and, as a safety net, at save
time.

## Root causes

- **Edit-time persistence kept invalid filters.** `handleFiltersUpdate`
persisted the current filter state to the page layout draft without
sanitizing it against the object's active fields. An invalid filter
(referencing a deactivated/deleted field) was re-saved on every update,
blocking the configuration from being accepted.
- **Save never enforced the cleanup.** `useSavePageLayout` serialized
the draft as-is. The "filters referencing deactivated/deleted fields
will be automatically removed on save" promise shown in the warning
banner was only honored reactively (when the filter panel was actively
edited), never at the actual save boundary. A chart whose filter panel
wasn't touched kept its stale invalid filter in the payload.
- **Query time treated inactive fields as valid.**
`useGraphWidgetQueryCommon` considered all fields (including inactive
ones) valid, so deactivated-field filters were never dropped when
running the chart query.

## Changes

### Edit-time (keeps draft and UI in sync as you edit)
- `ChartFiltersSettings` — sanitize filters in `handleFiltersUpdate`
before writing to the draft, dropping any filter whose `fieldMetadataId`
is not in the active-fields set.
- `dropChartRecordFiltersWithDeletedFields` — enhanced to also clean up
filter groups left orphaned once invalid filters are removed
(iteratively removing empty groups and re-parenting checks).
- `useGraphWidgetQueryCommon` — restrict valid field IDs to `isActive`
fields so deactivated-field filters are silently dropped at query
execution.
- `ChartFiltersDeletedFieldsWarning` — updated copy to mention both
deactivated and deleted fields.

### Save-time safety net (guarantees no invalid filter is ever
persisted)
- New `sanitizeChartFiltersInPageLayoutDraft` util — walks every chart
widget in the draft and drops record filters (and now-orphaned groups)
whose `fieldMetadataId` is not in the widget object's set of active
fields. It leaves non-chart widgets untouched and leaves filters intact
when the object metadata can't be resolved (avoids wiping valid filters
during metadata loading).
- `useSavePageLayout` — builds a `Map<objectMetadataId,
Set<activeFieldId>>` from `useObjectMetadataItems()` and sanitizes the
draft before converting it to the update input.

This layer only ever removes filters whose field is genuinely
deactivated/deleted — the exact set the backend rejects — and never
removes filters pointing at valid fields.

## Tests

- `dropChartRecordFiltersWithDeletedFields.test.ts` — extended coverage
for orphaned filter-group cleanup.
- `sanitizeChartFiltersInPageLayoutDraft.test.ts` — new: drops
deactivated/deleted-field filters on save, keeps valid filters, cleans
up orphaned groups, leaves non-chart widgets alone, and leaves filters
untouched when object metadata is unresolved.


<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/21958?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->

---------

Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
Marie
2026-06-22 18:15:33 +02:00
committed by GitHub
parent 6ce7d04f9d
commit 96a2987610
8 changed files with 483 additions and 14 deletions
@@ -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,
@@ -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<RecordFilter> &
Pick<RecordFilter, 'id' | 'fieldMetadataId'>,
): 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<string, Set<string>>([[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,
]);
});
});
@@ -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<string, Set<string>>;
}): 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,
},
};
}),
})),
};
};
@@ -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 } =
@@ -24,7 +24,7 @@ export const ChartFiltersDeletedFieldsWarning = ({
return (
<SidePanelInformationBanner
variant="warning"
message={t`This rule filters on one or more deleted fields that should be removed.`}
message={t`One or more filters reference deactivated or deleted fields and will be automatically removed on save.`}
/>
);
};
@@ -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<ChartWidgetConfiguration>,
});
@@ -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([]);
});
});
@@ -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<string>;
}): 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,
};
};