From cae2b012886a64e5467fa50920b6d0a64dbba9c9 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Thu, 28 Aug 2025 09:29:19 +0200 Subject: [PATCH] feat: Revamp Data Table column name restrictions (no-changelog) (#18851) --- .../src/schemas/data-store.schema.ts | 7 +- .../data-store-aggregate.controller.test.ts | 4 +- .../__tests__/data-store.controller.test.ts | 128 +++++++++--------- .../dataGrid/AddColumnButton.test.ts | 2 +- .../src/features/dataStore/constants.ts | 4 +- 5 files changed, 74 insertions(+), 71 deletions(-) diff --git a/packages/@n8n/api-types/src/schemas/data-store.schema.ts b/packages/@n8n/api-types/src/schemas/data-store.schema.ts index 56e4924c96..3e3588307c 100644 --- a/packages/@n8n/api-types/src/schemas/data-store.schema.ts +++ b/packages/@n8n/api-types/src/schemas/data-store.schema.ts @@ -5,16 +5,17 @@ import type { ListDataStoreQueryDto } from '../dto'; export const dataStoreNameSchema = z.string().trim().min(1).max(128); export const dataStoreIdSchema = z.string().max(36); -export const DATA_STORE_COLUMN_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9-]*$/; +// Postgres does not allow leading numbers or - +export const DATA_STORE_COLUMN_REGEX = /^[a-zA-Z][a-zA-Z0-9_]*$/; export const dataStoreColumnNameSchema = z .string() .trim() .min(1) - .max(128) + .max(63) // Postgres has a maximum of 63 characters .regex( DATA_STORE_COLUMN_REGEX, - 'Only alphanumeric characters and non-leading dashes are allowed for column names', + 'Only alphabetical characters and non-leading numbers and underscores are allowed for column names', ); export const dataStoreColumnTypeSchema = z.enum(['string', 'number', 'boolean', 'date']); diff --git a/packages/cli/src/modules/data-table/__tests__/data-store-aggregate.controller.test.ts b/packages/cli/src/modules/data-table/__tests__/data-store-aggregate.controller.test.ts index 378dbe1dfd..b10e62090f 100644 --- a/packages/cli/src/modules/data-table/__tests__/data-store-aggregate.controller.test.ts +++ b/packages/cli/src/modules/data-table/__tests__/data-store-aggregate.controller.test.ts @@ -337,11 +337,11 @@ describe('GET /data-stores-global', () => { name: 'Test Data Store', columns: [ { - name: 'test-column-1', + name: 'test_column_1', type: 'string', }, { - name: 'test-column-2', + name: 'test_column_2', type: 'boolean', }, ], diff --git a/packages/cli/src/modules/data-table/__tests__/data-store.controller.test.ts b/packages/cli/src/modules/data-table/__tests__/data-store.controller.test.ts index c3c27bb86b..a7bf81ba25 100644 --- a/packages/cli/src/modules/data-table/__tests__/data-store.controller.test.ts +++ b/packages/cli/src/modules/data-table/__tests__/data-store.controller.test.ts @@ -74,7 +74,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -91,7 +91,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: '', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -108,7 +108,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -125,7 +125,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -145,7 +145,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -165,7 +165,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -184,7 +184,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -202,7 +202,7 @@ describe('POST /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-ccolumn', + name: 'test_ccolumn', type: 'string', }, ], @@ -493,11 +493,11 @@ describe('GET /projects/:projectId/data-stores', () => { name: 'Test Data Store', columns: [ { - name: 'test-column-1', + name: 'test_column_1', type: 'string', }, { - name: 'test-column-2', + name: 'test_column_2', type: 'boolean', }, ], @@ -823,11 +823,11 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'boolean', }, ], @@ -838,8 +838,8 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { .expect(200); expect(response.body.data).toHaveLength(2); - expect(response.body.data[0].name).toBe('test-column'); - expect(response.body.data[1].name).toBe('another-column'); + expect(response.body.data[0].name).toBe('test_column'); + expect(response.body.data[1].name).toBe('another_column'); }); test('should list columns if user has project:editor role in team project', async () => { @@ -848,7 +848,7 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -859,7 +859,7 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { .expect(200); expect(response.body.data).toHaveLength(1); - expect(response.body.data[0].name).toBe('test-column'); + expect(response.body.data[0].name).toBe('test_column'); }); test('should list columns from personal project data store', async () => { @@ -867,7 +867,7 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { name: 'Personal Data Store 1', columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -878,7 +878,7 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/columns', () => { .expect(200); expect(response.body.data).toHaveLength(1); - expect(response.body.data[0].name).toBe('test-column'); + expect(response.body.data[0].name).toBe('test_column'); }); }); @@ -899,7 +899,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const project = await createTeamProject('test project', owner); const payload = { - name: 'test-column', + name: 'test_column', type: 'string', index: 0, }; @@ -950,7 +950,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(ownerProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -959,14 +959,14 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { await authMemberAgent .post(`/projects/${ownerProject.id}/data-stores/${dataStore.id}/columns`) .send({ - name: 'new-column', + name: 'new_column', type: 'string', }) .expect(403); const columnsInDb = await dataStoreColumnRepository.findBy({ dataTableId: dataStore.id }); expect(columnsInDb).toHaveLength(1); - expect(columnsInDb[0].name).toBe('test-column'); + expect(columnsInDb[0].name).toBe('test_column'); }); test('should not create column if user has project:viewer role in team project', async () => { @@ -975,7 +975,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project); const payload = { - name: 'test-column', + name: 'test_column', type: 'string', }; @@ -994,14 +994,14 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], }); const payload = { - name: 'new-column', + name: 'new_column', type: 'string', index: 0, }; @@ -1013,7 +1013,7 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const columnsInDb = await dataStoreColumnRepository.findBy({ dataTableId: dataStore.id }); expect(columnsInDb).toHaveLength(2); - expect(columnsInDb[0].name).toBe('new-column'); + expect(columnsInDb[0].name).toBe('new_column'); expect(columnsInDb[0].type).toBe('string'); }); @@ -1023,14 +1023,14 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], }); const payload = { - name: 'new-column', + name: 'new_column', type: 'boolean', index: 0, }; @@ -1042,9 +1042,9 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const columnsInDb = await dataStoreColumnRepository.findBy({ dataTableId: dataStore.id }); expect(columnsInDb).toHaveLength(2); - expect(columnsInDb[0].name).toBe('new-column'); + expect(columnsInDb[0].name).toBe('new_column'); expect(columnsInDb[0].type).toBe('boolean'); - expect(columnsInDb[1].name).toBe('test-column'); + expect(columnsInDb[1].name).toBe('test_column'); expect(columnsInDb[1].type).toBe('string'); }); @@ -1053,14 +1053,14 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], }); const payload = { - name: 'new-column', + name: 'new_column', type: 'boolean', index: 0, }; @@ -1072,9 +1072,9 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const columnsInDb = await dataStoreColumnRepository.findBy({ dataTableId: dataStore.id }); expect(columnsInDb).toHaveLength(2); - expect(columnsInDb[0].name).toBe('new-column'); + expect(columnsInDb[0].name).toBe('new_column'); expect(columnsInDb[0].type).toBe('boolean'); - expect(columnsInDb[1].name).toBe('test-column'); + expect(columnsInDb[1].name).toBe('test_column'); expect(columnsInDb[1].type).toBe('string'); }); @@ -1083,18 +1083,18 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column-1', + name: 'test_column_1', type: 'string', }, { - name: 'test-column-2', + name: 'test_column_2', type: 'string', }, ], }); const payload: DataStoreCreateColumnSchema = { - name: 'new-column', + name: 'new_column', type: 'boolean', index: 1, }; @@ -1107,9 +1107,9 @@ describe('POST /projects/:projectId/data-stores/:dataStoreId/columns', () => { const columns = await dataStoreColumnRepository.getColumns(dataStore.id); expect(columns).toHaveLength(3); - expect(columns[0].name).toBe('test-column-1'); - expect(columns[1].name).toBe('new-column'); - expect(columns[2].name).toBe('test-column-2'); + expect(columns[0].name).toBe('test_column_1'); + expect(columns[1].name).toBe('new_column'); + expect(columns[2].name).toBe('test_column_2'); }); }); @@ -1135,7 +1135,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1151,14 +1151,14 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(ownerProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], }); await authMemberAgent - .delete(`/projects/${ownerProject.id}/data-stores/${dataStore.id}/columns/test-column`) + .delete(`/projects/${ownerProject.id}/data-stores/${dataStore.id}/columns/test_column`) .send() .expect(403); @@ -1174,7 +1174,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1182,7 +1182,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId await linkUserToProject(member, project, 'project:viewer'); await authMemberAgent - .delete(`/projects/${project.id}/data-stores/${dataStore.id}/columns/test-column`) + .delete(`/projects/${project.id}/data-stores/${dataStore.id}/columns/test_column`) .send() .expect(403); @@ -1200,7 +1200,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1226,7 +1226,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1251,7 +1251,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1275,7 +1275,7 @@ describe('DELETE /projects/:projectId/data-stores/:dataStoreId/columns/:columnId const dataStore = await createDataStore(memberProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1327,7 +1327,7 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, ], @@ -1346,11 +1346,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(ownerProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1377,11 +1377,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1408,11 +1408,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1439,11 +1439,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1470,11 +1470,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(project, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1499,11 +1499,11 @@ describe('PATCH /projects/:projectId/data-stores/:dataStoreId/columns/:columnId/ const dataStore = await createDataStore(memberProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], @@ -1543,11 +1543,11 @@ describe('GET /projects/:projectId/data-stores/:dataStoreId/rows', () => { const dataStore = await createDataStore(ownerProject, { columns: [ { - name: 'test-column', + name: 'test_column', type: 'string', }, { - name: 'another-column', + name: 'another_column', type: 'string', }, ], diff --git a/packages/frontend/editor-ui/src/features/dataStore/components/dataGrid/AddColumnButton.test.ts b/packages/frontend/editor-ui/src/features/dataStore/components/dataGrid/AddColumnButton.test.ts index 0921cc3a5a..59fcedeecd 100644 --- a/packages/frontend/editor-ui/src/features/dataStore/components/dataGrid/AddColumnButton.test.ts +++ b/packages/frontend/editor-ui/src/features/dataStore/components/dataGrid/AddColumnButton.test.ts @@ -132,7 +132,7 @@ describe('AddColumnButton', () => { const nameInput = getByPlaceholderText('Enter column name'); // Test valid names - const validNames = ['column1', 'my-column', 'Column123', 'a1b2c3']; + const validNames = ['column1', 'my_column', 'Column123', 'a1b2c3']; for (const name of validNames) { await fireEvent.update(nameInput, name); diff --git a/packages/frontend/editor-ui/src/features/dataStore/constants.ts b/packages/frontend/editor-ui/src/features/dataStore/constants.ts index 5460244d78..e3a9598415 100644 --- a/packages/frontend/editor-ui/src/features/dataStore/constants.ts +++ b/packages/frontend/editor-ui/src/features/dataStore/constants.ts @@ -1,3 +1,5 @@ +import { DATA_STORE_COLUMN_REGEX } from '@n8n/api-types'; + // Route and view identifiers export const DATA_STORE_VIEW = 'data-stores'; export const PROJECT_DATA_STORES = 'project-data-stores'; @@ -25,7 +27,7 @@ export const DEFAULT_ID_COLUMN_NAME = 'id'; export const MAX_COLUMN_NAME_LENGTH = 128; -export const COLUMN_NAME_REGEX = /^[a-zA-Z0-9][a-zA-Z0-9-]*$/; +export const COLUMN_NAME_REGEX = DATA_STORE_COLUMN_REGEX; export const MIN_LOADING_TIME = 500; // ms