Tidy up validation for configuration (#14939)

closes https://github.com/twentyhq/core-team-issues/issues/1605

TODO:  
~~- add stories~~
~~- add base graph on companies when creating a new graph widget~~
This commit is contained in:
nitin
2025-10-10 02:04:51 +05:30
committed by GitHub
parent 5adc9fe9b2
commit 44f97058ac
44 changed files with 1015 additions and 475 deletions
@@ -28,7 +28,7 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
@Controller('rest/metadata/page-layout-tabs')
@Controller('rest/metadata/pageLayoutTabs')
@UseGuards(WorkspaceAuthGuard)
@UseFilters(PageLayoutTabRestApiExceptionFilter)
export class PageLayoutTabController {
@@ -28,7 +28,7 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
@Controller('rest/metadata/page-layout-widgets')
@Controller('rest/metadata/pageLayoutWidgets')
@UseGuards(WorkspaceAuthGuard)
@UseFilters(PageLayoutWidgetRestApiExceptionFilter)
export class PageLayoutWidgetController {
@@ -23,7 +23,7 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
@Controller('rest/metadata/page-layouts')
@Controller('rest/metadata/pageLayouts')
@UseGuards(WorkspaceAuthGuard)
@UseFilters(PageLayoutRestApiExceptionFilter)
export class PageLayoutController {
@@ -47,10 +47,10 @@ export class BarChartConfigurationDTO {
@IsOptional()
groupBySubFieldNameX?: string;
@Field(() => GraphOrderBy)
@Field(() => GraphOrderBy, { nullable: true })
@IsEnum(GraphOrderBy)
@IsNotEmpty()
orderByX: GraphOrderBy;
@IsOptional()
orderByX?: GraphOrderBy;
@Field(() => UUIDScalarType, { nullable: true })
@IsUUID()
@@ -72,15 +72,18 @@ export class BarChartConfigurationDTO {
@IsOptional()
omitNullValues?: boolean;
@Field(() => AxisNameDisplay)
@Field(() => AxisNameDisplay, {
nullable: true,
defaultValue: AxisNameDisplay.BOTH,
})
@IsEnum(AxisNameDisplay)
@IsNotEmpty()
axisNameDisplay: AxisNameDisplay;
@IsOptional()
axisNameDisplay?: AxisNameDisplay;
@Field(() => Boolean)
@Field(() => Boolean, { nullable: true, defaultValue: false })
@IsBoolean()
@IsNotEmpty()
displayDataLabel: boolean;
@IsOptional()
displayDataLabel?: boolean;
@Field(() => Number, { nullable: true })
@IsNumber()
@@ -34,9 +34,10 @@ export class GaugeChartConfigurationDTO {
@IsNotEmpty()
aggregateOperation: ExtendedAggregateOperations;
@Field(() => Boolean)
@Field(() => Boolean, { nullable: true, defaultValue: false })
@IsBoolean()
displayDataLabel: boolean;
@IsOptional()
displayDataLabel?: boolean;
@Field(() => String, { nullable: true })
@IsString()
@@ -47,10 +47,13 @@ export class LineChartConfigurationDTO {
@IsOptional()
groupBySubFieldNameX?: string;
@Field(() => GraphOrderBy)
@Field(() => GraphOrderBy, {
nullable: true,
defaultValue: GraphOrderBy.FIELD_ASC,
})
@IsEnum(GraphOrderBy)
@IsNotEmpty()
orderByX: GraphOrderBy;
@IsOptional()
orderByX?: GraphOrderBy;
@Field(() => UUIDScalarType, { nullable: true })
@IsUUID()
@@ -72,15 +75,18 @@ export class LineChartConfigurationDTO {
@IsOptional()
omitNullValues?: boolean;
@Field(() => AxisNameDisplay)
@Field(() => AxisNameDisplay, {
nullable: true,
defaultValue: AxisNameDisplay.BOTH,
})
@IsEnum(AxisNameDisplay)
@IsNotEmpty()
axisNameDisplay: AxisNameDisplay;
@IsOptional()
axisNameDisplay?: AxisNameDisplay;
@Field(() => Boolean)
@Field(() => Boolean, { nullable: true, defaultValue: false })
@IsBoolean()
@IsNotEmpty()
displayDataLabel: boolean;
@IsOptional()
displayDataLabel?: boolean;
@Field(() => Number, { nullable: true })
@IsNumber()
@@ -39,10 +39,10 @@ export class NumberChartConfigurationDTO {
@IsOptional()
label?: string;
@Field(() => Boolean)
@Field(() => Boolean, { nullable: true, defaultValue: false })
@IsBoolean()
@IsNotEmpty()
displayDataLabel: boolean;
@IsOptional()
displayDataLabel?: boolean;
@Field(() => String, { nullable: true })
@IsString()
@@ -54,11 +54,6 @@ export class NumberChartConfigurationDTO {
@IsOptional()
description?: string;
@Field(() => String, { nullable: true })
@IsString()
@IsOptional()
color?: string;
@Field(() => GraphQLJSON, { nullable: true })
@IsObject()
@IsOptional()
@@ -45,15 +45,18 @@ export class PieChartConfigurationDTO {
@IsOptional()
groupBySubFieldName?: string;
@Field(() => GraphOrderBy)
@Field(() => GraphOrderBy, {
nullable: true,
defaultValue: GraphOrderBy.VALUE_DESC,
})
@IsEnum(GraphOrderBy)
@IsNotEmpty()
orderBy: GraphOrderBy;
@IsOptional()
orderBy?: GraphOrderBy;
@Field(() => Boolean)
@Field(() => Boolean, { nullable: true, defaultValue: false })
@IsBoolean()
@IsNotEmpty()
displayDataLabel: boolean;
@IsOptional()
displayDataLabel?: boolean;
@Field(() => String, { nullable: true })
@IsString()
@@ -6,6 +6,7 @@ import { IframeConfigurationDTO } from 'src/engine/core-modules/page-layout/dtos
import { LineChartConfigurationDTO } from 'src/engine/core-modules/page-layout/dtos/line-chart-configuration.dto';
import { NumberChartConfigurationDTO } from 'src/engine/core-modules/page-layout/dtos/number-chart-configuration.dto';
import { PieChartConfigurationDTO } from 'src/engine/core-modules/page-layout/dtos/pie-chart-configuration.dto';
import { GraphType } from 'src/engine/core-modules/page-layout/enums/graph-type.enum';
import { WidgetConfigurationType } from 'src/engine/core-modules/page-layout/enums/widget-configuration-type.enum';
export const WidgetConfiguration = createUnionType({
@@ -25,24 +26,34 @@ export const WidgetConfiguration = createUnionType({
);
}
switch (configuration.configurationType) {
case WidgetConfigurationType.IFRAME_CONFIG:
return IframeConfigurationDTO;
case WidgetConfigurationType.BAR_CHART_CONFIG:
return BarChartConfigurationDTO;
case WidgetConfigurationType.LINE_CHART_CONFIG:
return LineChartConfigurationDTO;
case WidgetConfigurationType.PIE_CHART_CONFIG:
return PieChartConfigurationDTO;
case WidgetConfigurationType.NUMBER_CHART_CONFIG:
return NumberChartConfigurationDTO;
case WidgetConfigurationType.GAUGE_CHART_CONFIG:
return GaugeChartConfigurationDTO;
default:
throw new Error(
`Unknown widget configuration type: ${configuration.configurationType}`,
);
if (
configuration.configurationType === WidgetConfigurationType.CHART_CONFIG
) {
switch (configuration.graphType) {
case GraphType.BAR:
return BarChartConfigurationDTO;
case GraphType.LINE:
return LineChartConfigurationDTO;
case GraphType.PIE:
return PieChartConfigurationDTO;
case GraphType.NUMBER:
return NumberChartConfigurationDTO;
case GraphType.GAUGE:
return GaugeChartConfigurationDTO;
default:
throw new Error(`Unknown graph type: ${configuration.graphType}`);
}
}
if (
configuration.configurationType === WidgetConfigurationType.IFRAME_CONFIG
) {
return IframeConfigurationDTO;
}
throw new Error(
`Unknown widget configuration type: ${configuration.configurationType}`,
);
},
});
@@ -1,8 +1,4 @@
export enum WidgetConfigurationType {
BAR_CHART_CONFIG = 'BAR_CHART_CONFIG',
LINE_CHART_CONFIG = 'LINE_CHART_CONFIG',
PIE_CHART_CONFIG = 'PIE_CHART_CONFIG',
NUMBER_CHART_CONFIG = 'NUMBER_CHART_CONFIG',
GAUGE_CHART_CONFIG = 'GAUGE_CHART_CONFIG',
CHART_CONFIG = 'CHART_CONFIG',
IFRAME_CONFIG = 'IFRAME_CONFIG',
}
@@ -1,11 +1,20 @@
import { UseFilters, UseGuards, UsePipes } from '@nestjs/common';
import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
import {
Args,
Mutation,
Parent,
Query,
ResolveField,
Resolver,
} from '@nestjs/graphql';
import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe';
import { CreatePageLayoutWidgetInput } from 'src/engine/core-modules/page-layout/dtos/inputs/create-page-layout-widget.input';
import { UpdatePageLayoutWidgetInput } from 'src/engine/core-modules/page-layout/dtos/inputs/update-page-layout-widget.input';
import { PageLayoutWidgetDTO } from 'src/engine/core-modules/page-layout/dtos/page-layout-widget.dto';
import { WidgetConfiguration } from 'src/engine/core-modules/page-layout/dtos/widget-configuration.interface';
import { PageLayoutWidgetService } from 'src/engine/core-modules/page-layout/services/page-layout-widget.service';
import { injectWidgetConfigurationDiscriminator } from 'src/engine/core-modules/page-layout/utils/inject-widget-configuration-discriminator.util';
import { PageLayoutGraphqlApiExceptionFilter } from 'src/engine/core-modules/page-layout/utils/page-layout-graphql-api-exception.filter';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
@@ -79,4 +88,12 @@ export class PageLayoutWidgetResolver {
): Promise<PageLayoutWidgetDTO> {
return this.pageLayoutWidgetService.restore(id, workspace.id);
}
@ResolveField(() => WidgetConfiguration, { nullable: true })
configuration(@Parent() widget: PageLayoutWidgetDTO) {
return injectWidgetConfigurationDiscriminator(
widget.type,
widget.configuration,
);
}
}
@@ -18,7 +18,6 @@ import {
TEST_PIE_CHART_CONFIG_MINIMAL,
} from 'test/integration/constants/widget-configuration-test-data.constants';
import { WidgetConfigurationType } from 'src/engine/core-modules/page-layout/enums/widget-configuration-type.enum';
import { WidgetType } from 'src/engine/core-modules/page-layout/enums/widget-type.enum';
import { validateAndTransformWidgetConfiguration } from 'src/engine/core-modules/page-layout/utils/validate-and-transform-widget-configuration.util';
@@ -31,9 +30,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_IFRAME_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.IFRAME_CONFIG,
);
});
it('should throw error for invalid URL', () => {
@@ -73,9 +69,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_NUMBER_CHART_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.NUMBER_CHART_CONFIG,
);
});
it('should validate minimal number graph configuration', () => {
@@ -85,18 +78,15 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_NUMBER_CHART_CONFIG_MINIMAL);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.NUMBER_CHART_CONFIG,
);
});
it('should throw error for missing required fields', () => {
it('should throw error for partial number graph configuration with missing required fields', () => {
expect(() =>
validateAndTransformWidgetConfiguration(
WidgetType.GRAPH,
INVALID_NUMBER_CHART_CONFIG_MISSING_FIELDS,
),
).toThrow(/aggregateFieldMetadataId should not be empty/);
).toThrow(/aggregateFieldMetadataId.*aggregateOperation/);
});
it('should throw error for invalid UUID', () => {
@@ -117,9 +107,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_BAR_CHART_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.BAR_CHART_CONFIG,
);
});
it('should validate minimal bar graph configuration', () => {
@@ -129,18 +116,15 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_BAR_CHART_CONFIG_MINIMAL);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.BAR_CHART_CONFIG,
);
});
it('should throw error for missing group by field', () => {
it('should throw error for partial bar graph configuration with missing required fields', () => {
expect(() =>
validateAndTransformWidgetConfiguration(
WidgetType.GRAPH,
INVALID_BAR_CHART_CONFIG_MISSING_GROUP_BY,
),
).toThrow(/groupByFieldMetadataIdX should not be empty/);
).toThrow(/groupByFieldMetadataIdX/);
});
});
@@ -152,9 +136,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_LINE_CHART_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.LINE_CHART_CONFIG,
);
});
it('should validate minimal line graph configuration', () => {
@@ -164,9 +145,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_LINE_CHART_CONFIG_MINIMAL);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.LINE_CHART_CONFIG,
);
});
});
@@ -178,9 +156,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_PIE_CHART_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.PIE_CHART_CONFIG,
);
});
it('should validate minimal pie graph configuration', () => {
@@ -190,9 +165,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_PIE_CHART_CONFIG_MINIMAL);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.PIE_CHART_CONFIG,
);
});
});
@@ -204,9 +176,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_GAUGE_CHART_CONFIG);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.GAUGE_CHART_CONFIG,
);
});
it('should validate minimal gauge graph configuration', () => {
@@ -216,9 +185,6 @@ describe('validateAndTransformWidgetConfiguration', () => {
);
expect(result).toMatchObject(TEST_GAUGE_CHART_CONFIG_MINIMAL);
expect((result as any)?.configurationType).toBe(
WidgetConfigurationType.GAUGE_CHART_CONFIG,
);
});
});
@@ -0,0 +1,32 @@
import { type WidgetConfigurationInterface } from 'src/engine/core-modules/page-layout/dtos/widget-configuration.interface';
import { WidgetConfigurationType } from 'src/engine/core-modules/page-layout/enums/widget-configuration-type.enum';
import { WidgetType } from 'src/engine/core-modules/page-layout/enums/widget-type.enum';
type ConfigurationWithDiscriminator = WidgetConfigurationInterface & {
configurationType: WidgetConfigurationType;
};
export const injectWidgetConfigurationDiscriminator = (
widgetType: WidgetType,
configuration: WidgetConfigurationInterface | null,
): ConfigurationWithDiscriminator | null => {
if (!configuration) {
return null;
}
if (widgetType === WidgetType.IFRAME) {
return {
...configuration,
configurationType: WidgetConfigurationType.IFRAME_CONFIG,
} as ConfigurationWithDiscriminator;
}
if (widgetType === WidgetType.GRAPH && 'graphType' in configuration) {
return {
...configuration,
configurationType: WidgetConfigurationType.CHART_CONFIG,
} as ConfigurationWithDiscriminator;
}
return configuration as ConfigurationWithDiscriminator;
};
@@ -9,7 +9,6 @@ import { NumberChartConfigurationDTO } from 'src/engine/core-modules/page-layout
import { PieChartConfigurationDTO } from 'src/engine/core-modules/page-layout/dtos/pie-chart-configuration.dto';
import { type WidgetConfigurationInterface } from 'src/engine/core-modules/page-layout/dtos/widget-configuration.interface';
import { GraphType } from 'src/engine/core-modules/page-layout/enums/graph-type.enum';
import { WidgetConfigurationType } from 'src/engine/core-modules/page-layout/enums/widget-configuration-type.enum';
import { WidgetType } from 'src/engine/core-modules/page-layout/enums/widget-type.enum';
const formatValidationErrors = (errors: ValidationError[]): string => {
@@ -37,16 +36,16 @@ const validateGraphConfiguration = (
case GraphType.BAR: {
const instance = plainToInstance(BarChartConfigurationDTO, configuration);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.BAR_CHART_CONFIG,
} as WidgetConfigurationInterface;
return instance;
}
case GraphType.LINE: {
const instance = plainToInstance(
@@ -54,30 +53,30 @@ const validateGraphConfiguration = (
configuration,
);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.LINE_CHART_CONFIG,
} as WidgetConfigurationInterface;
return instance;
}
case GraphType.PIE: {
const instance = plainToInstance(PieChartConfigurationDTO, configuration);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.PIE_CHART_CONFIG,
} as WidgetConfigurationInterface;
return instance;
}
case GraphType.NUMBER: {
const instance = plainToInstance(
@@ -85,16 +84,16 @@ const validateGraphConfiguration = (
configuration,
);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.NUMBER_CHART_CONFIG,
} as WidgetConfigurationInterface;
return instance;
}
case GraphType.GAUGE: {
const instance = plainToInstance(
@@ -102,16 +101,16 @@ const validateGraphConfiguration = (
configuration,
);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.GAUGE_CHART_CONFIG,
} as WidgetConfigurationInterface;
return instance;
}
default:
return null;
@@ -123,16 +122,16 @@ const validateIframeConfiguration = (
): WidgetConfigurationInterface | null => {
const instance = plainToInstance(IframeConfigurationDTO, configuration);
const errors = validateSync(instance);
const errors = validateSync(instance, {
whitelist: true,
forbidUnknownValues: true,
});
if (errors.length > 0) {
throw errors;
}
return {
...instance,
configurationType: WidgetConfigurationType.IFRAME_CONFIG,
} as WidgetConfigurationInterface;
return instance;
};
export const validateAndTransformWidgetConfiguration = (