diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget-field-validation.exception.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget-field-validation.exception.ts new file mode 100644 index 0000000000..df9c802eee --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget-field-validation.exception.ts @@ -0,0 +1,6 @@ +export class PageLayoutWidgetFieldValidationException extends Error { + constructor(message: string) { + super(message); + this.name = 'PageLayoutWidgetFieldValidationException'; + } +} diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/services/page-layout-widget.service.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/services/page-layout-widget.service.ts index 6b09d53bad..27f6444e4f 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/services/page-layout-widget.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/services/page-layout-widget.service.ts @@ -19,7 +19,6 @@ import { CreatePageLayoutWidgetInput } from 'src/engine/metadata-modules/page-la import { UpdatePageLayoutWidgetInput } from 'src/engine/metadata-modules/page-layout-widget/dtos/inputs/update-page-layout-widget.input'; import { type PageLayoutWidgetDTO } from 'src/engine/metadata-modules/page-layout-widget/dtos/page-layout-widget.dto'; import { WidgetConfigurationType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-configuration-type.type'; -import { WidgetType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-type.enum'; import { PageLayoutWidgetException, PageLayoutWidgetExceptionCode, @@ -28,8 +27,7 @@ import { } from 'src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget.exception'; import { type AllPageLayoutWidgetConfiguration } from 'src/engine/metadata-modules/page-layout-widget/types/all-page-layout-widget-configuration.type'; import { fromFlatPageLayoutWidgetToPageLayoutWidgetDto } from 'src/engine/metadata-modules/page-layout-widget/utils/from-flat-page-layout-widget-to-page-layout-widget-dto.util'; -import { isChartFieldsForValidation } from 'src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util'; -import { validateChartConfigurationFieldReferences } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util'; +import { validateChartConfigurationFieldReferencesOrThrow } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util'; import { WorkspaceMigrationBuilderException } from 'src/engine/workspace-manager/workspace-migration/exceptions/workspace-migration-builder-exception'; import { WorkspaceMigrationValidateBuildAndRunService } from 'src/engine/workspace-manager/workspace-migration/services/workspace-migration-validate-build-and-run-service'; import { DashboardSyncService } from 'src/modules/dashboard-sync/services/dashboard-sync.service'; @@ -122,25 +120,17 @@ export class PageLayoutWidgetService { } } - private async validateChartFieldReferencesIfApplicable({ + private async validateChartFieldReferences({ configuration, objectMetadataId, - widgetType, + widgetTitle, workspaceId, }: { configuration: AllPageLayoutWidgetConfiguration; objectMetadataId?: string | null; - widgetType?: WidgetType | null; + widgetTitle?: string | null; workspaceId: string; }): Promise { - const needsChartValidation = - isChartFieldsForValidation(configuration) || - widgetType === WidgetType.GRAPH; - - if (!needsChartValidation) { - return; - } - const { flatFieldMetadataMaps, flatObjectMetadataMaps } = await this.workspaceManyOrAllFlatEntityMapsCacheService.getOrRecomputeManyOrAllFlatEntityMaps( { @@ -149,20 +139,13 @@ export class PageLayoutWidgetService { }, ); - try { - validateChartConfigurationFieldReferences({ - configuration, - objectMetadataId, - widgetType, - flatFieldMetadataMaps, - flatObjectMetadataMaps, - }); - } catch (error) { - throw new PageLayoutWidgetException( - error instanceof Error ? error.message : String(error), - PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, - ); - } + validateChartConfigurationFieldReferencesOrThrow({ + widgetConfiguration: configuration, + widgetObjectMetadataId: objectMetadataId, + widgetTitle, + flatFieldMetadataMaps, + flatObjectMetadataMaps, + }); } async findByPageLayoutTabId({ @@ -275,10 +258,10 @@ export class PageLayoutWidgetService { }); if (isDefined(createInput.configuration)) { - await this.validateChartFieldReferencesIfApplicable({ + await this.validateChartFieldReferences({ configuration: createInput.configuration, objectMetadataId: createInput.objectMetadataId ?? null, - widgetType: createInput.type, + widgetTitle: createInput.title, workspaceId, }); } @@ -405,14 +388,14 @@ export class PageLayoutWidgetService { const effectiveObjectMetadataId = isObjectMetadataIdBeingUpdated ? processedUpdateData.objectMetadataId : existingWidget.objectMetadataId; - const effectiveWidgetType = - processedUpdateData.type ?? existingWidget.type; + const effectiveWidgetTitle = + processedUpdateData.title ?? existingWidget.title; if (isDefined(effectiveConfiguration)) { - await this.validateChartFieldReferencesIfApplicable({ + await this.validateChartFieldReferences({ configuration: effectiveConfiguration, objectMetadataId: effectiveObjectMetadataId, - widgetType: effectiveWidgetType, + widgetTitle: effectiveWidgetTitle, workspaceId, }); } diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-fields-for-validation.type.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-referencing-field-in-configuration.type.ts similarity index 94% rename from packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-fields-for-validation.type.ts rename to packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-referencing-field-in-configuration.type.ts index b1df69523a..1f9617f2c8 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-fields-for-validation.type.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/types/chart-referencing-field-in-configuration.type.ts @@ -4,7 +4,7 @@ import { type GaugeChartConfigurationDTO } from 'src/engine/metadata-modules/pag import { type LineChartConfigurationDTO } from 'src/engine/metadata-modules/page-layout-widget/dtos/line-chart-configuration.dto'; import { type PieChartConfigurationDTO } from 'src/engine/metadata-modules/page-layout-widget/dtos/pie-chart-configuration.dto'; -export type ChartFieldsForValidation = +export type ChartReferencingFieldInConfiguration = | AggregateChartConfigurationDTO | BarChartConfigurationDTO | GaugeChartConfigurationDTO diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-referencing-field-in-configuration.util.ts similarity index 59% rename from packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util.ts rename to packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-referencing-field-in-configuration.util.ts index 541052dcad..bbcad718f6 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/is-chart-referencing-field-in-configuration.util.ts @@ -1,8 +1,8 @@ import { GRAPH_CONFIGURATION_TYPES } from 'src/engine/metadata-modules/page-layout-widget/constants/graph-configuration-types.constant'; import { type AllPageLayoutWidgetConfiguration } from 'src/engine/metadata-modules/page-layout-widget/types/all-page-layout-widget-configuration.type'; -import { type ChartFieldsForValidation } from 'src/engine/metadata-modules/page-layout-widget/types/chart-fields-for-validation.type'; +import { type ChartReferencingFieldInConfiguration } from 'src/engine/metadata-modules/page-layout-widget/types/chart-referencing-field-in-configuration.type'; -export const isChartFieldsForValidation = ( +export const isChartReferencingFieldInConfiguration = ( configuration: AllPageLayoutWidgetConfiguration, -): configuration is ChartFieldsForValidation => +): configuration is ChartReferencingFieldInConfiguration => GRAPH_CONFIGURATION_TYPES.has(configuration.configurationType); diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util.ts index 1bcdc45631..293a5c04f1 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util.ts @@ -1,3 +1,4 @@ +import { msg } from '@lingui/core/macro'; import { isDefined } from 'twenty-shared/utils'; import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; @@ -5,54 +6,85 @@ import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; import { WidgetConfigurationType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-configuration-type.type'; -import { WidgetType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-type.enum'; +import { + PageLayoutWidgetException, + PageLayoutWidgetExceptionCode, +} from 'src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget.exception'; +import { PageLayoutWidgetFieldValidationException } from 'src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget-field-validation.exception'; import { type AllPageLayoutWidgetConfiguration } from 'src/engine/metadata-modules/page-layout-widget/types/all-page-layout-widget-configuration.type'; import { findActiveFlatFieldMetadataById } from 'src/engine/metadata-modules/page-layout-widget/utils/find-active-flat-field-metadata-by-id.util'; -import { isChartFieldsForValidation } from 'src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util'; -import { validateGroupByField } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util'; +import { isChartReferencingFieldInConfiguration } from 'src/engine/metadata-modules/page-layout-widget/utils/is-chart-referencing-field-in-configuration.util'; +import { validateGroupByFieldOrThrow } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util'; -export const validateChartConfigurationFieldReferences = ({ - configuration, - objectMetadataId, - widgetType, - flatObjectMetadataMaps, - flatFieldMetadataMaps, -}: { - configuration?: AllPageLayoutWidgetConfiguration | null; - objectMetadataId?: string | null; - widgetType?: WidgetType | null; - flatObjectMetadataMaps: FlatEntityMaps; - flatFieldMetadataMaps: FlatEntityMaps; -}): void => { - if (!isDefined(configuration)) return; +const buildChartFieldValidationException = ( + message: string, + widgetTitle?: string | null, +): PageLayoutWidgetException => { + const prefix = isDefined(widgetTitle) ? `Chart "${widgetTitle}": ` : ''; + const fullMessage = prefix + message; - if (!isChartFieldsForValidation(configuration)) { - if (widgetType === WidgetType.GRAPH) { - throw new Error( - 'GRAPH widgets require configurationType AGGREGATE_CHART, BAR_CHART, GAUGE_CHART, LINE_CHART, or PIE_CHART.', - ); + return new PageLayoutWidgetException( + fullMessage, + PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, + { + userFriendlyMessage: msg`${fullMessage}`, + }, + ); +}; + +const validateGroupByFieldAsChartFieldOrThrow = ( + params: Parameters[0], + widgetTitle?: string | null, +): void => { + try { + validateGroupByFieldOrThrow(params); + } catch (error) { + if (!(error instanceof PageLayoutWidgetFieldValidationException)) { + throw error; } + throw buildChartFieldValidationException(error.message, widgetTitle); + } +}; + +export const validateChartConfigurationFieldReferencesOrThrow = ({ + widgetConfiguration, + widgetObjectMetadataId, + widgetTitle, + flatFieldMetadataMaps, + flatObjectMetadataMaps, +}: { + widgetConfiguration?: AllPageLayoutWidgetConfiguration | null; + widgetObjectMetadataId?: string | null; + widgetTitle?: string | null; + flatFieldMetadataMaps: FlatEntityMaps; + flatObjectMetadataMaps: FlatEntityMaps; +}): void => { + if (!isDefined(widgetConfiguration)) { return; } - if (widgetType && widgetType !== WidgetType.GRAPH) { - throw new Error( - `Graph configuration is only valid for widgets of type GRAPH.`, + if (!isChartReferencingFieldInConfiguration(widgetConfiguration)) { + return; + } + + if (!isDefined(widgetObjectMetadataId)) { + throw buildChartFieldValidationException( + 'objectMetadataId is required for graph widgets.', + widgetTitle, ); } - if (!isDefined(objectMetadataId)) { - throw new Error('objectMetadataId is required for graph widgets.'); - } - const objectMetadata = findFlatEntityByIdInFlatEntityMaps({ - flatEntityId: objectMetadataId, + flatEntityId: widgetObjectMetadataId, flatEntityMaps: flatObjectMetadataMaps, }); if (!isDefined(objectMetadata) || !objectMetadata.isActive) { - throw new Error(`objectMetadataId "${objectMetadataId}" not found.`); + throw buildChartFieldValidationException( + `objectMetadataId "${widgetObjectMetadataId}" not found.`, + widgetTitle, + ); } const allFields = Object.values(flatFieldMetadataMaps.byUniversalIdentifier) @@ -69,70 +101,116 @@ export const validateChartConfigurationFieldReferences = ({ }); const aggregateField = findActiveFlatFieldMetadataById( - configuration.aggregateFieldMetadataId, + widgetConfiguration.aggregateFieldMetadataId, flatFieldMetadataMaps, ); if (!isDefined(aggregateField)) { - throw new Error( - `aggregateFieldMetadataId "${configuration.aggregateFieldMetadataId}" not found.`, + throw buildChartFieldValidationException( + `aggregateFieldMetadataId "${widgetConfiguration.aggregateFieldMetadataId}" not found.`, + widgetTitle, ); } - if (aggregateField.objectMetadataId !== objectMetadataId) { - throw new Error( - `aggregateFieldMetadataId must belong to objectMetadataId "${objectMetadataId}".`, + if (aggregateField.objectMetadataId !== widgetObjectMetadataId) { + throw buildChartFieldValidationException( + `aggregateFieldMetadataId must belong to objectMetadataId "${widgetObjectMetadataId}".`, + widgetTitle, ); } - switch (configuration.configurationType) { + switch (widgetConfiguration.configurationType) { case WidgetConfigurationType.BAR_CHART: case WidgetConfigurationType.LINE_CHART: { - validateGroupByField({ - fieldId: configuration.primaryAxisGroupByFieldMetadataId, - subFieldName: configuration.primaryAxisGroupBySubFieldName, - paramName: 'primaryAxisGroupByFieldMetadataId', - objectMetadataId, - flatFieldMetadataMaps, - allFields, - fieldsByObjectId, - }); + validateGroupByFieldAsChartFieldOrThrow( + { + fieldId: widgetConfiguration.primaryAxisGroupByFieldMetadataId, + subFieldName: widgetConfiguration.primaryAxisGroupBySubFieldName, + paramName: 'primaryAxisGroupByFieldMetadataId', + objectMetadataId: widgetObjectMetadataId, + flatFieldMetadataMaps, + allFields, + fieldsByObjectId, + }, + widgetTitle, + ); - if (isDefined(configuration.secondaryAxisGroupBySubFieldName)) { - if (!isDefined(configuration.secondaryAxisGroupByFieldMetadataId)) { - throw new Error( + if (isDefined(widgetConfiguration.secondaryAxisGroupBySubFieldName)) { + if ( + !isDefined(widgetConfiguration.secondaryAxisGroupByFieldMetadataId) + ) { + throw buildChartFieldValidationException( 'secondaryAxisGroupByFieldMetadataId is required when secondaryAxisGroupBySubFieldName is provided.', + widgetTitle, ); } } - if (isDefined(configuration.secondaryAxisGroupByFieldMetadataId)) { - validateGroupByField({ - fieldId: configuration.secondaryAxisGroupByFieldMetadataId, - subFieldName: configuration.secondaryAxisGroupBySubFieldName, - paramName: 'secondaryAxisGroupByFieldMetadataId', - objectMetadataId, - flatFieldMetadataMaps, - allFields, - fieldsByObjectId, - }); + if (isDefined(widgetConfiguration.secondaryAxisGroupByFieldMetadataId)) { + validateGroupByFieldAsChartFieldOrThrow( + { + fieldId: widgetConfiguration.secondaryAxisGroupByFieldMetadataId, + subFieldName: widgetConfiguration.secondaryAxisGroupBySubFieldName, + paramName: 'secondaryAxisGroupByFieldMetadataId', + objectMetadataId: widgetObjectMetadataId, + flatFieldMetadataMaps, + allFields, + fieldsByObjectId, + }, + widgetTitle, + ); } break; } case WidgetConfigurationType.PIE_CHART: { - validateGroupByField({ - fieldId: configuration.groupByFieldMetadataId, - subFieldName: configuration.groupBySubFieldName, - paramName: 'groupByFieldMetadataId', - objectMetadataId, - flatFieldMetadataMaps, - allFields, - fieldsByObjectId, - }); + validateGroupByFieldAsChartFieldOrThrow( + { + fieldId: widgetConfiguration.groupByFieldMetadataId, + subFieldName: widgetConfiguration.groupBySubFieldName, + paramName: 'groupByFieldMetadataId', + objectMetadataId: widgetObjectMetadataId, + flatFieldMetadataMaps, + allFields, + fieldsByObjectId, + }, + widgetTitle, + ); break; } case WidgetConfigurationType.AGGREGATE_CHART: default: break; } + + if (isDefined(widgetConfiguration.filter?.recordFilters)) { + for (const recordFilter of widgetConfiguration.filter.recordFilters) { + const filterField = findActiveFlatFieldMetadataById( + recordFilter.fieldMetadataId, + flatFieldMetadataMaps, + ); + + if (!isDefined(filterField)) { + const inactiveOrMissingField = findFlatEntityByIdInFlatEntityMaps({ + flatEntityId: recordFilter.fieldMetadataId, + flatEntityMaps: flatFieldMetadataMaps, + }); + + const fieldLabel = inactiveOrMissingField + ? `"${inactiveOrMissingField.label}"` + : `field id "${recordFilter.fieldMetadataId}"`; + + throw buildChartFieldValidationException( + `One of the chart filters uses ${fieldLabel}, but it was deleted. Please remove or replace this filter rule.`, + widgetTitle, + ); + } + + if (filterField.objectMetadataId !== widgetObjectMetadataId) { + throw buildChartFieldValidationException( + `Filter field "${recordFilter.fieldMetadataId}" must belong to objectMetadataId "${widgetObjectMetadataId}".`, + widgetTitle, + ); + } + } + } }; diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util.ts index 39ebd4b6f4..b472b28d6d 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout-widget/utils/validate-group-by-field.util.ts @@ -4,11 +4,24 @@ import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field- import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util'; +import { PageLayoutWidgetFieldValidationException } from 'src/engine/metadata-modules/page-layout-widget/exceptions/page-layout-widget-field-validation.exception'; import { findActiveFlatFieldMetadataById } from 'src/engine/metadata-modules/page-layout-widget/utils/find-active-flat-field-metadata-by-id.util'; import { validateCompositeSubfield } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-composite-subfield.util'; import { validateRelationSubfield } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-relation-subfield.util'; -export const validateGroupByField = ({ +const toGroupByFieldValidationException = ( + error: unknown, +): PageLayoutWidgetFieldValidationException => { + if (error instanceof PageLayoutWidgetFieldValidationException) { + return error; + } + + return new PageLayoutWidgetFieldValidationException( + error instanceof Error ? error.message : String(error), + ); +}; + +export const validateGroupByFieldOrThrow = ({ fieldId, subFieldName, paramName, @@ -26,44 +39,58 @@ export const validateGroupByField = ({ fieldsByObjectId: Map; }): void => { if (!isDefined(fieldId)) { - throw new Error(`${paramName} is required.`); + throw new PageLayoutWidgetFieldValidationException( + `${paramName} is required.`, + ); } const field = findActiveFlatFieldMetadataById(fieldId, flatFieldMetadataMaps); if (!isDefined(field)) { - throw new Error(`${paramName} "${fieldId}" not found.`); + throw new PageLayoutWidgetFieldValidationException( + `${paramName} "${fieldId}" not found.`, + ); } if (field.objectMetadataId !== objectMetadataId) { - throw new Error( + throw new PageLayoutWidgetFieldValidationException( `${paramName} must belong to objectMetadataId "${objectMetadataId}".`, ); } if (isCompositeFieldMetadataType(field.type)) { - validateCompositeSubfield({ - field, - subFieldName, - paramName: field.name, - }); + try { + validateCompositeSubfield({ + field, + subFieldName, + paramName: field.name, + }); + } catch (error) { + throw toGroupByFieldValidationException(error); + } return; } if (isMorphOrRelationFlatFieldMetadata(field)) { - validateRelationSubfield({ - field, - subFieldName, - paramName: field.name, - allFields, - fieldsByObjectId, - }); + try { + validateRelationSubfield({ + field, + subFieldName, + paramName: field.name, + allFields, + fieldsByObjectId, + }); + } catch (error) { + throw toGroupByFieldValidationException(error); + } return; } if (isDefined(subFieldName)) { - throw new Error(`Field "${field.name}" does not support subfields.`); + throw new PageLayoutWidgetFieldValidationException( + `Field "${field.name}" does not support subfields.`, + ); } }; diff --git a/packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts b/packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts index e4e3a6da2e..a940e5a8fb 100644 --- a/packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/page-layout/services/page-layout-update.service.ts @@ -22,6 +22,7 @@ import { reconstructFlatPageLayoutWithTabsAndWidgets } from 'src/engine/metadata import { UpdatePageLayoutTabWithWidgetsInput } from 'src/engine/metadata-modules/page-layout-tab/dtos/inputs/update-page-layout-tab-with-widgets.input'; import { UpdatePageLayoutWidgetWithIdInput } from 'src/engine/metadata-modules/page-layout-widget/dtos/inputs/update-page-layout-widget-with-id.input'; import { WidgetConfigurationType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-configuration-type.type'; +import { validateChartConfigurationFieldReferencesOrThrow } from 'src/engine/metadata-modules/page-layout-widget/utils/validate-chart-configuration-field-references.util'; import { UpdatePageLayoutWithTabsInput } from 'src/engine/metadata-modules/page-layout/dtos/inputs/update-page-layout-with-tabs.input'; import { PageLayoutDTO } from 'src/engine/metadata-modules/page-layout/dtos/page-layout.dto'; import { @@ -474,6 +475,14 @@ export class PageLayoutUpdateService { widgetsToCreate: FlatPageLayoutWidget[]; widgetsToUpdate: FlatPageLayoutWidget[]; } { + for (const widgetInput of widgets) { + this.validateChartFieldReferences({ + widgetInput, + flatFieldMetadataMaps, + flatObjectMetadataMaps, + }); + } + const existingWidgets = Object.values( flatPageLayoutWidgetMaps.byUniversalIdentifier, ) @@ -634,6 +643,28 @@ export class PageLayoutUpdateService { }; } + private validateChartFieldReferences({ + widgetInput, + flatFieldMetadataMaps, + flatObjectMetadataMaps, + }: { + widgetInput: UpdatePageLayoutWidgetWithIdInput; + flatFieldMetadataMaps: AllFlatEntityMaps['flatFieldMetadataMaps']; + flatObjectMetadataMaps: AllFlatEntityMaps['flatObjectMetadataMaps']; + }): void { + if (!isDefined(widgetInput.configuration)) { + return; + } + + validateChartConfigurationFieldReferencesOrThrow({ + widgetConfiguration: widgetInput.configuration, + widgetObjectMetadataId: widgetInput.objectMetadataId, + widgetTitle: widgetInput.title, + flatFieldMetadataMaps, + flatObjectMetadataMaps, + }); + } + private collectOrphanedViewIdsFromDeletedWidgets({ widgetsToUpdate, tabsToUpdate, @@ -649,7 +680,6 @@ export class PageLayoutUpdateService { const viewIdsToDelete = new Set(); const directlyDeletedWidgetIds = new Set(); - // Collect viewIds from directly deleted FIELDS widgets for (const widget of widgetsToUpdate) { if (isDefined(widget.deletedAt)) { directlyDeletedWidgetIds.add(widget.id); @@ -661,7 +691,6 @@ export class PageLayoutUpdateService { } } - // Collect viewIds from FIELDS widgets in deleted tabs const deletedTabIds = new Set( tabsToUpdate .filter((tab) => isDefined(tab.deletedAt)) @@ -685,7 +714,6 @@ export class PageLayoutUpdateService { } } - // Filter out viewIds still referenced by surviving widgets for (const widget of allExistingWidgets) { if ( !isDefined(widget.deletedAt) && diff --git a/packages/twenty-server/src/modules/dashboard/tools/get-dashboard.tool.ts b/packages/twenty-server/src/modules/dashboard/tools/get-dashboard.tool.ts index cd451bb7b4..9db60b95e2 100644 --- a/packages/twenty-server/src/modules/dashboard/tools/get-dashboard.tool.ts +++ b/packages/twenty-server/src/modules/dashboard/tools/get-dashboard.tool.ts @@ -5,7 +5,7 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m import { WidgetConfigurationType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-configuration-type.type'; import { WidgetType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-type.enum'; import { findActiveFlatFieldMetadataById } from 'src/engine/metadata-modules/page-layout-widget/utils/find-active-flat-field-metadata-by-id.util'; -import { isChartFieldsForValidation } from 'src/engine/metadata-modules/page-layout-widget/utils/is-chart-fields-for-validation.util'; +import { isChartReferencingFieldInConfiguration } from 'src/engine/metadata-modules/page-layout-widget/utils/is-chart-referencing-field-in-configuration.util'; import { buildSystemAuthContext } from 'src/engine/twenty-orm/utils/build-system-auth-context.util'; import { type DashboardToolContext, @@ -114,7 +114,7 @@ export const createGetDashboardTool = ( tab.widgets?.map((w) => { if ( w.type !== WidgetType.GRAPH || - !isChartFieldsForValidation(w.configuration) + !isChartReferencingFieldInConfiguration(w.configuration) ) { return { id: w.id, diff --git a/packages/twenty-server/test/integration/metadata/suites/page-layout/failing-page-layout-with-tabs-update.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/page-layout/failing-page-layout-with-tabs-update.integration-spec.ts index d598c9f60f..ea7dbfde42 100644 --- a/packages/twenty-server/test/integration/metadata/suites/page-layout/failing-page-layout-with-tabs-update.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/page-layout/failing-page-layout-with-tabs-update.integration-spec.ts @@ -1,8 +1,23 @@ import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { createOneFieldMetadata } from 'test/integration/metadata/suites/field-metadata/utils/create-one-field-metadata.util'; +import { deleteOneFieldMetadata } from 'test/integration/metadata/suites/field-metadata/utils/delete-one-field-metadata.util'; +import { updateOneFieldMetadata } from 'test/integration/metadata/suites/field-metadata/utils/update-one-field-metadata.util'; +import { createOnePageLayoutTab } from 'test/integration/metadata/suites/page-layout-tab/utils/create-one-page-layout-tab.util'; +import { destroyOnePageLayoutTab } from 'test/integration/metadata/suites/page-layout-tab/utils/destroy-one-page-layout-tab.util'; +import { + fetchTestFieldMetadataIds, + type TestFieldMetadataIds, +} from 'test/integration/metadata/suites/page-layout-widget/utils/fetch-test-field-metadata-ids.util'; +import { createOnePageLayout } from 'test/integration/metadata/suites/page-layout/utils/create-one-page-layout.util'; +import { destroyOnePageLayout } from 'test/integration/metadata/suites/page-layout/utils/destroy-one-page-layout.util'; import { updateOnePageLayoutWithTabsAndWidgets } from 'test/integration/metadata/suites/page-layout/utils/update-one-page-layout-with-tabs-and-widgets.util'; +import { AggregateOperations, FieldMetadataType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; +import { v4 } from 'uuid'; import { WidgetConfigurationType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-configuration-type.type'; import { WidgetType } from 'src/engine/metadata-modules/page-layout-widget/enums/widget-type.enum'; +import { type AllPageLayoutWidgetConfiguration } from 'src/engine/metadata-modules/page-layout-widget/types/all-page-layout-widget-configuration.type'; import { PageLayoutType } from 'src/engine/metadata-modules/page-layout/enums/page-layout-type.enum'; describe('Page layout with tabs update should fail', () => { @@ -42,4 +57,201 @@ describe('Page layout with tabs update should fail', () => { expectOneNotInternalServerErrorSnapshot({ errors }); }); + + describe('chart filter validation failures', () => { + let testFieldMetadataIds: TestFieldMetadataIds; + let testPageLayoutId: string | undefined; + let testPageLayoutTabId: string | undefined; + let testFilterFieldMetadataId: string | undefined; + let testFilterFieldDeleted = false; + + beforeAll(async () => { + testFieldMetadataIds = await fetchTestFieldMetadataIds(); + }); + + beforeEach(async () => { + const filterFieldName = `deletedFilterField${Date.now()}`; + + const { data: createdFieldData } = await createOneFieldMetadata({ + expectToFail: false, + input: { + objectMetadataId: testFieldMetadataIds.objectMetadataId, + type: FieldMetadataType.TEXT, + label: 'Deleted Filter Field', + name: filterFieldName, + isLabelSyncedWithName: false, + }, + }); + + testFilterFieldMetadataId = createdFieldData.createOneField.id; + + const { data: layoutData } = await createOnePageLayout({ + expectToFail: false, + input: { + name: 'Deleted Filter Field Layout', + type: PageLayoutType.RECORD_PAGE, + }, + }); + + testPageLayoutId = layoutData.createPageLayout.id; + + const { data: tabData } = await createOnePageLayoutTab({ + expectToFail: false, + input: { + title: 'Deleted Filter Field Tab', + pageLayoutId: testPageLayoutId, + }, + }); + + testPageLayoutTabId = tabData.createPageLayoutTab.id; + testFilterFieldDeleted = false; + }); + + afterEach(async () => { + if (isDefined(testFilterFieldMetadataId) && !testFilterFieldDeleted) { + await updateOneFieldMetadata({ + expectToFail: false, + input: { + idToUpdate: testFilterFieldMetadataId, + updatePayload: { + isActive: false, + }, + }, + }); + await deleteOneFieldMetadata({ + expectToFail: false, + input: { + idToDelete: testFilterFieldMetadataId, + }, + }); + } + + if (isDefined(testPageLayoutTabId)) { + await destroyOnePageLayoutTab({ + expectToFail: false, + input: { id: testPageLayoutTabId }, + }); + } + + if (isDefined(testPageLayoutId)) { + await destroyOnePageLayout({ + expectToFail: false, + input: { id: testPageLayoutId }, + }); + } + + testFilterFieldMetadataId = undefined; + testPageLayoutTabId = undefined; + testPageLayoutId = undefined; + testFilterFieldDeleted = false; + }); + + it('when saving layout tabs and widgets with a deleted chart filter field', async () => { + if ( + !isDefined(testPageLayoutId) || + !isDefined(testPageLayoutTabId) || + !isDefined(testFilterFieldMetadataId) + ) { + throw new Error('Test setup incomplete'); + } + + const chartWidgetId = v4(); + const chartTitle = 'Opportunities by Name'; + const chartConfiguration = { + configurationType: WidgetConfigurationType.PIE_CHART, + aggregateFieldMetadataId: testFieldMetadataIds.fieldMetadataId1, + aggregateOperation: AggregateOperations.COUNT, + groupByFieldMetadataId: testFieldMetadataIds.fieldMetadataId2, + filter: { + recordFilters: [ + { + fieldMetadataId: testFilterFieldMetadataId, + operand: 'contains', + value: 'acme', + }, + ], + recordFilterGroups: [], + }, + } satisfies AllPageLayoutWidgetConfiguration; + + const tabs = [ + { + id: testPageLayoutTabId, + title: 'Deleted Filter Field Tab', + position: 0, + widgets: [ + { + id: chartWidgetId, + pageLayoutTabId: testPageLayoutTabId, + title: chartTitle, + type: WidgetType.GRAPH, + objectMetadataId: testFieldMetadataIds.objectMetadataId, + gridPosition: { + row: 0, + column: 0, + rowSpan: 1, + columnSpan: 1, + }, + configuration: chartConfiguration, + }, + ], + }, + ]; + + await updateOnePageLayoutWithTabsAndWidgets({ + expectToFail: false, + input: { + id: testPageLayoutId, + name: 'Deleted Filter Field Layout', + type: PageLayoutType.RECORD_PAGE, + objectMetadataId: null, + tabs, + }, + }); + + await updateOneFieldMetadata({ + expectToFail: false, + input: { + idToUpdate: testFilterFieldMetadataId, + updatePayload: { + isActive: false, + }, + }, + }); + + await deleteOneFieldMetadata({ + expectToFail: false, + input: { + idToDelete: testFilterFieldMetadataId, + }, + }); + + testFilterFieldDeleted = true; + + const { errors } = await updateOnePageLayoutWithTabsAndWidgets({ + expectToFail: true, + input: { + id: testPageLayoutId, + name: 'Deleted Filter Field Layout', + type: PageLayoutType.RECORD_PAGE, + objectMetadataId: null, + tabs, + }, + }); + + expect(errors).toBeDefined(); + expect(errors).toHaveLength(1); + + const [firstError] = errors!; + + expect(firstError.extensions.code).toBe('BAD_USER_INPUT'); + expect(firstError.message).toContain(`Chart "${chartTitle}":`); + expect(firstError.message).toContain( + 'Please remove or replace this filter rule.', + ); + expect( + String(firstError.extensions.userFriendlyMessage), + ).toContain(`Chart "${chartTitle}":`); + }); + }); });