From d7469091841c36ad83e16b4a862dbe5aa9bcadcb Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Wed, 8 Jul 2026 16:25:47 +0200 Subject: [PATCH] feat(sdk): validate graph page-layout widgets at build time (#22559) When an app defines a graph widget (aggregate, pie, bar or line chart), the built manifest can carry the wrong key and the server rejects it at sync time with a confusing "aggregate field is required" error. The SDK type already requires `aggregateFieldMetadataUniversalIdentifier` and renames the raw `aggregateFieldMetadataId` at compile time. But the manifest build runs esbuild with no type checking, so a wrong or missing key slips through and only fails later on the server. This adds a build-time check that mirrors the server validator, with a hint pointing at the right key when the raw one was used. It is non-breaking since correctly authored apps already use the universal key. Tests: unit tests on the validator, plus a real graph widget added to the rich-app fixture so the integration and e2e suites cover the happy path. --- .../extra-tab.page-layout-tab.ts | 15 +++ .../app-dev/expected-manifest.ts | 13 +++ .../__tests__/manifest-validate.spec.ts | 107 +++++++++++++++++- .../build/manifest/manifest-validate.ts | 78 ++++++++++++- packages/twenty-shared/src/types/index.ts | 2 + .../graph-widget-configuration-type.ts | 11 ++ 6 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 packages/twenty-shared/src/types/page-layout/graph-widget-configuration-type.ts diff --git a/packages/twenty-apps/fixtures/rich-app/src/page-layout-tabs/extra-tab.page-layout-tab.ts b/packages/twenty-apps/fixtures/rich-app/src/page-layout-tabs/extra-tab.page-layout-tab.ts index fe59adcdf2..89c052cfd8 100644 --- a/packages/twenty-apps/fixtures/rich-app/src/page-layout-tabs/extra-tab.page-layout-tab.ts +++ b/packages/twenty-apps/fixtures/rich-app/src/page-layout-tabs/extra-tab.page-layout-tab.ts @@ -1,8 +1,11 @@ import { + AggregateOperations, definePageLayoutTab, PageLayoutTabLayoutMode, } from 'twenty-sdk/define'; +import { POST_CARD_UNIVERSAL_IDENTIFIER } from '../objects/post-card.object'; + export default definePageLayoutTab({ universalIdentifier: 'b0b1b2b3-b4b5-4000-8000-000000000010', pageLayoutUniversalIdentifier: 'b0b1b2b3-b4b5-4000-8000-000000000020', @@ -21,5 +24,17 @@ export default definePageLayoutTab({ '370ae182-743f-4ecb-b625-7ac48e21f0e5', }, }, + { + universalIdentifier: 'b0b1b2b3-b4b5-4000-8000-000000000012', + title: 'Total Priority', + type: 'GRAPH', + objectUniversalIdentifier: POST_CARD_UNIVERSAL_IDENTIFIER, + configuration: { + configurationType: 'AGGREGATE_CHART', + aggregateFieldMetadataUniversalIdentifier: + '7b57bd63-5a4c-46ca-9d52-42c8f02d1df6', + aggregateOperation: AggregateOperations.SUM, + }, + }, ], }); diff --git a/packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__integration__/app-dev/expected-manifest.ts b/packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__integration__/app-dev/expected-manifest.ts index ecf18135fc..61c8286e1f 100644 --- a/packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__integration__/app-dev/expected-manifest.ts +++ b/packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__integration__/app-dev/expected-manifest.ts @@ -2,6 +2,7 @@ import { FieldType } from '@/sdk/define'; import type { Manifest } from 'twenty-shared/application'; import { SystemPermissionFlag } from 'twenty-shared/constants'; import { + AggregateOperations, FieldMetadataType, NavigationMenuItemType, PageLayoutTabLayoutMode, @@ -34,6 +35,18 @@ export const EXPECTED_MANIFEST: Manifest = { '370ae182-743f-4ecb-b625-7ac48e21f0e5', }, }, + { + universalIdentifier: 'b0b1b2b3-b4b5-4000-8000-000000000012', + title: 'Total Priority', + type: 'GRAPH', + objectUniversalIdentifier: '54b589ca-eeed-4950-a176-358418b85c05', + configuration: { + configurationType: 'AGGREGATE_CHART', + aggregateFieldMetadataUniversalIdentifier: + '7b57bd63-5a4c-46ca-9d52-42c8f02d1df6', + aggregateOperation: AggregateOperations.SUM, + }, + }, ], }, ], diff --git a/packages/twenty-sdk/src/cli/utilities/build/manifest/__tests__/manifest-validate.spec.ts b/packages/twenty-sdk/src/cli/utilities/build/manifest/__tests__/manifest-validate.spec.ts index 321f3a2bd3..3e396d356b 100644 --- a/packages/twenty-sdk/src/cli/utilities/build/manifest/__tests__/manifest-validate.spec.ts +++ b/packages/twenty-sdk/src/cli/utilities/build/manifest/__tests__/manifest-validate.spec.ts @@ -2,8 +2,14 @@ import { type ApplicationManifest, type FieldManifest, type Manifest, + type PageLayoutTabManifest, + type PageLayoutWidgetManifest, } from 'twenty-shared/application'; -import { FieldMetadataType, RelationType } from 'twenty-shared/types'; +import { + AggregateOperations, + FieldMetadataType, + RelationType, +} from 'twenty-shared/types'; import { manifestValidate } from '@/cli/utilities/build/manifest/manifest-validate'; const validApplication: ApplicationManifest = { @@ -494,4 +500,103 @@ describe('manifestValidate', () => { expect(versionErrors).toHaveLength(1); }); }); + + describe('graph widget validation', () => { + const makeGraphWidgetTab = ( + configuration: PageLayoutWidgetManifest['configuration'], + ): PageLayoutTabManifest => ({ + universalIdentifier: 'b0a5f0f2-6c2e-4d1c-9d0b-2f8a4c3e1a01', + title: 'Dashboard', + position: 0, + widgets: [ + { + universalIdentifier: 'b0a5f0f2-6c2e-4d1c-9d0b-2f8a4c3e1a02', + title: 'Total opportunities', + type: 'GRAPH', + configuration, + }, + ], + }); + + it('should pass when a graph widget has aggregateFieldMetadataUniversalIdentifier', () => { + const result = manifestValidate({ + ...validManifest, + pageLayoutTabs: [ + makeGraphWidgetTab({ + configurationType: 'AGGREGATE_CHART', + aggregateFieldMetadataUniversalIdentifier: + 'b0a5f0f2-6c2e-4d1c-9d0b-2f8a4c3e1a03', + aggregateOperation: AggregateOperations.COUNT, + }), + ], + }); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it('should error when a graph widget is missing the aggregate field identifier', () => { + const result = manifestValidate({ + ...validManifest, + pageLayoutTabs: [ + makeGraphWidgetTab({ + configurationType: 'AGGREGATE_CHART', + aggregateFieldMetadataUniversalIdentifier: null, + aggregateOperation: AggregateOperations.COUNT, + }), + ], + }); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual( + expect.stringContaining( + 'is missing aggregateFieldMetadataUniversalIdentifier', + ), + ); + }); + + it('should hint at the correct key when the raw aggregateFieldMetadataId was used', () => { + const configurationWithRawKey = { + configurationType: 'AGGREGATE_CHART', + aggregateFieldMetadataId: 'b0a5f0f2-6c2e-4d1c-9d0b-2f8a4c3e1a03', + aggregateOperation: AggregateOperations.COUNT, + } as unknown as PageLayoutWidgetManifest['configuration']; + + const result = manifestValidate({ + ...validManifest, + pageLayoutTabs: [makeGraphWidgetTab(configurationWithRawKey)], + }); + + expect(result.isValid).toBe(false); + expect(result.errors).toContainEqual( + expect.stringContaining( + 'not "aggregateFieldMetadataId"', + ), + ); + }); + + it('should ignore non-graph widgets that have no aggregate field', () => { + const result = manifestValidate({ + ...validManifest, + pageLayoutTabs: [ + makeGraphWidgetTab({ configurationType: 'TIMELINE' }), + ], + }); + + expect(result.isValid).toBe(true); + expect(result.errors).toHaveLength(0); + }); + + it('should not crash on a widget with a missing configuration', () => { + const nullConfiguration = + null as unknown as PageLayoutWidgetManifest['configuration']; + + const result = manifestValidate({ + ...validManifest, + pageLayoutTabs: [makeGraphWidgetTab(nullConfiguration)], + }); + + expect(result.isValid).toBe(true); + }); + }); }); diff --git a/packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-validate.ts b/packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-validate.ts index ff64918d45..08c6231d27 100644 --- a/packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-validate.ts +++ b/packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-validate.ts @@ -1,7 +1,19 @@ +import { isNonEmptyString } from '@sniptt/guards'; import { validate as uuidValidate, version as uuidVersion } from 'uuid'; -import { type FieldManifest, type Manifest } from 'twenty-shared/application'; -import { FieldMetadataType, RelationType } from 'twenty-shared/types'; +import { + type FieldManifest, + type Manifest, + type PageLayoutWidgetManifest, +} from 'twenty-shared/application'; +import { + FieldMetadataType, + GRAPH_WIDGET_CONFIGURATION_TYPES, + type GraphWidgetConfigurationType, + type PageLayoutWidgetUniversalConfiguration, + RelationType, +} from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; const MIN_UUID_VERSION = 4; @@ -15,6 +27,22 @@ const VALID_RELATION_TYPES: string[] = [ RelationType.ONE_TO_MANY, ]; +const RAW_AGGREGATE_FIELD_METADATA_ID_KEY = 'aggregateFieldMetadataId'; + +type GraphPageLayoutWidgetUniversalConfiguration = Extract< + PageLayoutWidgetUniversalConfiguration, + { configurationType: GraphWidgetConfigurationType } +>; + +const isGraphWidgetConfiguration = ( + configuration: PageLayoutWidgetUniversalConfiguration | null | undefined, +): configuration is GraphPageLayoutWidgetUniversalConfiguration => + isDefined(configuration) && + GRAPH_WIDGET_CONFIGURATION_TYPES.some( + (configurationType) => + configurationType === configuration.configurationType, + ); + const extractDuplicates = (values: string[]): string[] => { const seen = new Set(); const duplicates = new Set(); @@ -102,6 +130,50 @@ const validateRelationFields = ( return errors; }; +const collectPageLayoutWidgets = ( + manifest: Pick, +): PageLayoutWidgetManifest[] => { + const widgetsFromPageLayouts = manifest.pageLayouts.flatMap( + (pageLayout) => pageLayout.tabs?.flatMap((tab) => tab.widgets ?? []) ?? [], + ); + + const widgetsFromStandaloneTabs = manifest.pageLayoutTabs.flatMap( + (tab) => tab.widgets ?? [], + ); + + return [...widgetsFromPageLayouts, ...widgetsFromStandaloneTabs]; +}; + +const validateGraphWidgets = ( + widgets: PageLayoutWidgetManifest[], +): string[] => { + const errors: string[] = []; + + for (const widget of widgets) { + const configuration = widget.configuration; + + if (!isGraphWidgetConfiguration(configuration)) { + continue; + } + + if ( + !isNonEmptyString(configuration.aggregateFieldMetadataUniversalIdentifier) + ) { + const usedRawKey = RAW_AGGREGATE_FIELD_METADATA_ID_KEY in configuration; + + const hint = usedRawKey + ? ` Reference the aggregate field with "aggregateFieldMetadataUniversalIdentifier" (not "${RAW_AGGREGATE_FIELD_METADATA_ID_KEY}").` + : ''; + + errors.push( + `Graph widget "${widget.title}" is missing aggregateFieldMetadataUniversalIdentifier.${hint}`, + ); + } + } + + return errors; +}; + const invalidUniversalIdentifierVersions = ( identifiers: string[], ): string[] => { @@ -163,5 +235,7 @@ export const manifestValidate = (manifest: Manifest) => { errors.push(...validateRelationFields(allFields)); + errors.push(...validateGraphWidgets(collectPageLayoutWidgets(manifest))); + return { errors, warnings, isValid: errors.length === 0 }; }; diff --git a/packages/twenty-shared/src/types/index.ts b/packages/twenty-shared/src/types/index.ts index 0f54e5cfba..7fa8a4a438 100644 --- a/packages/twenty-shared/src/types/index.ts +++ b/packages/twenty-shared/src/types/index.ts @@ -181,6 +181,8 @@ export type { ChartFilter, UniversalChartFilter, } from './page-layout/chart-filter.type'; +export type { GraphWidgetConfigurationType } from './page-layout/graph-widget-configuration-type'; +export { GRAPH_WIDGET_CONFIGURATION_TYPES } from './page-layout/graph-widget-configuration-type'; export type { GridPosition } from './page-layout/grid-position.type'; export type { AggregateChartConfiguration, diff --git a/packages/twenty-shared/src/types/page-layout/graph-widget-configuration-type.ts b/packages/twenty-shared/src/types/page-layout/graph-widget-configuration-type.ts new file mode 100644 index 0000000000..fa3e46a969 --- /dev/null +++ b/packages/twenty-shared/src/types/page-layout/graph-widget-configuration-type.ts @@ -0,0 +1,11 @@ +import { type PageLayoutWidgetConfiguration } from './page-layout-widget-configuration.type'; + +export const GRAPH_WIDGET_CONFIGURATION_TYPES = [ + 'AGGREGATE_CHART', + 'PIE_CHART', + 'BAR_CHART', + 'LINE_CHART', +] as const satisfies readonly PageLayoutWidgetConfiguration['configurationType'][]; + +export type GraphWidgetConfigurationType = + (typeof GRAPH_WIDGET_CONFIGURATION_TYPES)[number];