From 28c6edfa1f4947d92be955580b7bb5c6d114e738 Mon Sep 17 00:00:00 2001 From: nitin <142569587+ehconitin@users.noreply.github.com> Date: Wed, 29 Oct 2025 19:35:26 +0530 Subject: [PATCH] add validation on widgets grid position and sizing (#15397) closes https://github.com/twentyhq/core-team-issues/issues/1606 As discussed in DMS -- overlapping is not a concern since the library handles the collision and handles overlapping widgets (if created through api) using the compact type vertical (ie, move the widget vertically to create space) --- .../widget-grid-max-columns.constant.ts | 1 + .../widget-grid-max-rows.constant.ts | 1 + .../dtos/inputs/grid-position.input.ts | 10 +- .../page-layout-widget.exception.ts | 10 + .../services/page-layout-widget.service.ts | 12 ++ ...validate-widget-grid-position.util.spec.ts | 187 ++++++++++++++++++ .../validate-widget-grid-position.util.ts | 70 +++++++ 7 files changed, 286 insertions(+), 5 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant.ts create mode 100644 packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant.ts create mode 100644 packages/twenty-server/src/engine/core-modules/page-layout/utils/__tests__/validate-widget-grid-position.util.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util.ts diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant.ts b/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant.ts new file mode 100644 index 0000000000..1bac9c4859 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant.ts @@ -0,0 +1 @@ +export const WIDGET_GRID_MAX_COLUMNS = 12; diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant.ts b/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant.ts new file mode 100644 index 0000000000..31782396a8 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant.ts @@ -0,0 +1 @@ +export const WIDGET_GRID_MAX_ROWS = 100; diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/dtos/inputs/grid-position.input.ts b/packages/twenty-server/src/engine/core-modules/page-layout/dtos/inputs/grid-position.input.ts index df647b8a88..adbc4b5b8d 100644 --- a/packages/twenty-server/src/engine/core-modules/page-layout/dtos/inputs/grid-position.input.ts +++ b/packages/twenty-server/src/engine/core-modules/page-layout/dtos/inputs/grid-position.input.ts @@ -1,31 +1,31 @@ import { Field, InputType } from '@nestjs/graphql'; -import { IsNotEmpty, IsNumber, Min } from 'class-validator'; +import { IsInt, IsNotEmpty, Min } from 'class-validator'; import { GridPosition } from 'src/engine/core-modules/page-layout/types/grid-position.type'; @InputType('GridPositionInput') export class GridPositionInput implements GridPosition { @Field() - @IsNumber() + @IsInt() @Min(0) @IsNotEmpty() row: number; @Field() - @IsNumber() + @IsInt() @Min(0) @IsNotEmpty() column: number; @Field() - @IsNumber() + @IsInt() @Min(1) @IsNotEmpty() rowSpan: number; @Field() - @IsNumber() + @IsInt() @Min(1) @IsNotEmpty() columnSpan: number; diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception.ts b/packages/twenty-server/src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception.ts index 771640c480..4d861426ed 100644 --- a/packages/twenty-server/src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception.ts +++ b/packages/twenty-server/src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception.ts @@ -14,6 +14,7 @@ export enum PageLayoutWidgetExceptionMessageKey { PAGE_LAYOUT_TAB_NOT_FOUND = 'PAGE_LAYOUT_TAB_NOT_FOUND', PAGE_LAYOUT_WIDGET_NOT_DELETED = 'PAGE_LAYOUT_WIDGET_NOT_DELETED', GRID_POSITION_REQUIRED = 'GRID_POSITION_REQUIRED', + INVALID_WIDGET_GRID_POSITION = 'INVALID_WIDGET_GRID_POSITION', INVALID_WIDGET_CONFIGURATION = 'INVALID_WIDGET_CONFIGURATION', } @@ -38,6 +39,15 @@ export const generatePageLayoutWidgetExceptionMessage = ( return 'Page layout widget is not deleted and cannot be restored'; case PageLayoutWidgetExceptionMessageKey.GRID_POSITION_REQUIRED: return 'Grid position is required'; + case PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_GRID_POSITION: + if (widgetTitle && detailedError) { + return `Invalid grid position for widget "${widgetTitle}": ${detailedError}`; + } + if (detailedError) { + return `Invalid grid position: ${detailedError}`; + } + + return 'Invalid widget grid position'; case PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_CONFIGURATION: if (widgetTitle && widgetType && detailedError) { return `Invalid configuration for widget "${widgetTitle}" of type ${widgetType}: ${detailedError}`; diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/services/page-layout-widget.service.ts b/packages/twenty-server/src/engine/core-modules/page-layout/services/page-layout-widget.service.ts index 126fb0d120..9c176be778 100644 --- a/packages/twenty-server/src/engine/core-modules/page-layout/services/page-layout-widget.service.ts +++ b/packages/twenty-server/src/engine/core-modules/page-layout/services/page-layout-widget.service.ts @@ -21,6 +21,7 @@ import { } from 'src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception'; import { PageLayoutTabService } from 'src/engine/core-modules/page-layout/services/page-layout-tab.service'; import { validateAndTransformWidgetConfiguration } from 'src/engine/core-modules/page-layout/utils/validate-and-transform-widget-configuration.util'; +import { validateWidgetGridPosition } from 'src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util'; @Injectable() export class PageLayoutWidgetService { @@ -115,6 +116,11 @@ export class PageLayoutWidgetService { ); } + validateWidgetGridPosition( + pageLayoutWidgetData.gridPosition, + pageLayoutWidgetData.title, + ); + try { await this.pageLayoutTabService.findByIdOrThrow( pageLayoutWidgetData.pageLayoutTabId, @@ -209,6 +215,12 @@ export class PageLayoutWidgetService { ); } + if (updateData.gridPosition) { + const titleForValidation = updateData.title ?? existingWidget.title; + + validateWidgetGridPosition(updateData.gridPosition, titleForValidation); + } + let validatedConfig: WidgetConfigurationInterface | null = null; if (updateData.configuration) { diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/utils/__tests__/validate-widget-grid-position.util.spec.ts b/packages/twenty-server/src/engine/core-modules/page-layout/utils/__tests__/validate-widget-grid-position.util.spec.ts new file mode 100644 index 0000000000..45c84c3373 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/page-layout/utils/__tests__/validate-widget-grid-position.util.spec.ts @@ -0,0 +1,187 @@ +import { WIDGET_GRID_MAX_COLUMNS } from 'src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant'; +import { WIDGET_GRID_MAX_ROWS } from 'src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant'; +import { PageLayoutWidgetException } from 'src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception'; +import { validateWidgetGridPosition } from 'src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util'; + +describe('validateWidgetGridPosition', () => { + const validGridPosition = { + row: 0, + column: 0, + rowSpan: 2, + columnSpan: 3, + }; + + describe('Valid grid positions', () => { + it('should not throw for valid grid position', () => { + expect(() => + validateWidgetGridPosition(validGridPosition, 'Test Widget'), + ).not.toThrow(); + }); + + it('should not throw for widget at max column boundary', () => { + expect(() => + validateWidgetGridPosition( + { + row: 0, + column: WIDGET_GRID_MAX_COLUMNS - 1, + rowSpan: 1, + columnSpan: 1, + }, + 'Test Widget', + ), + ).not.toThrow(); + }); + + it('should not throw for widget at max row boundary', () => { + expect(() => + validateWidgetGridPosition( + { + row: WIDGET_GRID_MAX_ROWS - 1, + column: 0, + rowSpan: 1, + columnSpan: 1, + }, + 'Test Widget', + ), + ).not.toThrow(); + }); + + it('should not throw for widget spanning to column grid edge', () => { + expect(() => + validateWidgetGridPosition( + { + row: 0, + column: 8, + rowSpan: 1, + columnSpan: 4, + }, + 'Test Widget', + ), + ).not.toThrow(); + }); + + it('should not throw for widget spanning to row grid edge', () => { + expect(() => + validateWidgetGridPosition( + { + row: WIDGET_GRID_MAX_ROWS - 5, + column: 0, + rowSpan: 5, + columnSpan: 6, + }, + 'Test Widget', + ), + ).not.toThrow(); + }); + }); + + describe('Invalid row positions', () => { + it('should throw for row exceeding max rows', () => { + expect(() => + validateWidgetGridPosition( + { ...validGridPosition, row: WIDGET_GRID_MAX_ROWS }, + 'Test Widget', + ), + ).toThrow(PageLayoutWidgetException); + }); + + it('should throw when widget extends beyond grid height', () => { + expect(() => + validateWidgetGridPosition( + { + row: WIDGET_GRID_MAX_ROWS - 2, + column: 0, + rowSpan: 5, + columnSpan: 6, + }, + 'Test Widget', + ), + ).toThrow(/extends beyond grid height/); + }); + }); + + describe('Invalid column positions', () => { + it('should throw for column exceeding max columns', () => { + expect(() => + validateWidgetGridPosition( + { ...validGridPosition, column: WIDGET_GRID_MAX_COLUMNS }, + 'Test Widget', + ), + ).toThrow(PageLayoutWidgetException); + }); + }); + + describe('Widget extending beyond grid', () => { + it('should throw when widget extends beyond grid width', () => { + expect(() => + validateWidgetGridPosition( + { + row: 0, + column: 10, + rowSpan: 1, + columnSpan: 3, + }, + 'Test Widget', + ), + ).toThrow(/extends beyond grid width/); + }); + }); + + describe('Error messages', () => { + it('should include max columns value in error', () => { + expect(() => + validateWidgetGridPosition( + { + row: 0, + column: 10, + rowSpan: 1, + columnSpan: 5, + }, + 'Test Widget', + ), + ).toThrow(new RegExp(WIDGET_GRID_MAX_COLUMNS.toString())); + }); + + it('should include max rows value in error for row start', () => { + expect(() => + validateWidgetGridPosition( + { + row: WIDGET_GRID_MAX_ROWS + 10, + column: 0, + rowSpan: 1, + columnSpan: 1, + }, + 'Test Widget', + ), + ).toThrow(new RegExp(WIDGET_GRID_MAX_ROWS.toString())); + }); + + it('should include max rows value in error for row extension', () => { + expect(() => + validateWidgetGridPosition( + { + row: 95, + column: 0, + rowSpan: 10, + columnSpan: 6, + }, + 'Test Widget', + ), + ).toThrow(new RegExp(WIDGET_GRID_MAX_ROWS.toString())); + }); + + it('should include widget title in error message', () => { + expect(() => + validateWidgetGridPosition( + { + row: WIDGET_GRID_MAX_ROWS, + column: 0, + rowSpan: 1, + columnSpan: 1, + }, + 'My Custom Widget', + ), + ).toThrow(/My Custom Widget/); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util.ts b/packages/twenty-server/src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util.ts new file mode 100644 index 0000000000..925d8e0f79 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/page-layout/utils/validate-widget-grid-position.util.ts @@ -0,0 +1,70 @@ +import { WIDGET_GRID_MAX_COLUMNS } from 'src/engine/core-modules/page-layout/constants/widget-grid-max-columns.constant'; +import { WIDGET_GRID_MAX_ROWS } from 'src/engine/core-modules/page-layout/constants/widget-grid-max-rows.constant'; +import { + PageLayoutWidgetException, + PageLayoutWidgetExceptionCode, + PageLayoutWidgetExceptionMessageKey, + generatePageLayoutWidgetExceptionMessage, +} from 'src/engine/core-modules/page-layout/exceptions/page-layout-widget.exception'; + +type GridPosition = { + row: number; + column: number; + rowSpan: number; + columnSpan: number; +}; + +export const validateWidgetGridPosition = ( + gridPosition: GridPosition, + widgetTitle: string, +): void => { + const { row, column, rowSpan, columnSpan } = gridPosition; + + if (column >= WIDGET_GRID_MAX_COLUMNS) { + throw new PageLayoutWidgetException( + generatePageLayoutWidgetExceptionMessage( + PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_GRID_POSITION, + widgetTitle, + undefined, + `column ${column} exceeds grid width (max column is ${WIDGET_GRID_MAX_COLUMNS - 1})`, + ), + PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, + ); + } + + if (column + columnSpan > WIDGET_GRID_MAX_COLUMNS) { + throw new PageLayoutWidgetException( + generatePageLayoutWidgetExceptionMessage( + PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_GRID_POSITION, + widgetTitle, + undefined, + `widget extends beyond grid width (column ${column} + columnSpan ${columnSpan} > ${WIDGET_GRID_MAX_COLUMNS})`, + ), + PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, + ); + } + + if (row >= WIDGET_GRID_MAX_ROWS) { + throw new PageLayoutWidgetException( + generatePageLayoutWidgetExceptionMessage( + PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_GRID_POSITION, + widgetTitle, + undefined, + `row ${row} exceeds maximum allowed rows (${WIDGET_GRID_MAX_ROWS})`, + ), + PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, + ); + } + + if (row + rowSpan > WIDGET_GRID_MAX_ROWS) { + throw new PageLayoutWidgetException( + generatePageLayoutWidgetExceptionMessage( + PageLayoutWidgetExceptionMessageKey.INVALID_WIDGET_GRID_POSITION, + widgetTitle, + undefined, + `widget extends beyond grid height (row ${row} + rowSpan ${rowSpan} > ${WIDGET_GRID_MAX_ROWS})`, + ), + PageLayoutWidgetExceptionCode.INVALID_PAGE_LAYOUT_WIDGET_DATA, + ); + } +};