From 4dcb22048dcba28947594e11fbd7d34f3da18c6f Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Wed, 27 Aug 2025 10:28:03 +0200 Subject: [PATCH] feat(Data Table Node): Add Delete operation (no-changelog) (#18785) --- .../nodes/DataTable/actions/router.ts | 2 +- .../DataTable/actions/row/Row.resource.ts | 19 +++--- .../DataTable/actions/row/delete.operation.ts | 61 +++++++++++++++++++ .../DataTable/actions/row/get.operation.ts | 25 +------- .../nodes/DataTable/common/fields.ts | 13 +++- .../nodes/DataTable/common/selectMany.ts | 50 +++++++++++++-- .../selectMany.test.ts} | 58 ++++++++++++------ 7 files changed, 171 insertions(+), 57 deletions(-) create mode 100644 packages/nodes-base/nodes/DataTable/actions/row/delete.operation.ts rename packages/nodes-base/nodes/DataTable/test/{actions/rows/get.operation.test.ts => common/selectMany.test.ts} (53%) diff --git a/packages/nodes-base/nodes/DataTable/actions/router.ts b/packages/nodes-base/nodes/DataTable/actions/router.ts index a2f9d57238..4affd9f4d8 100644 --- a/packages/nodes-base/nodes/DataTable/actions/router.ts +++ b/packages/nodes-base/nodes/DataTable/actions/router.ts @@ -3,7 +3,7 @@ import { NodeOperationError } from 'n8n-workflow'; import * as row from './row/Row.resource'; -type DataTableNodeType = AllEntities<{ row: 'insert' | 'get' }>; +type DataTableNodeType = AllEntities<{ row: 'insert' | 'get' | 'deleteRows' }>; export async function router(this: IExecuteFunctions): Promise { const operationResult: INodeExecutionData[] = []; diff --git a/packages/nodes-base/nodes/DataTable/actions/row/Row.resource.ts b/packages/nodes-base/nodes/DataTable/actions/row/Row.resource.ts index 055982a851..f1ad1a52a8 100644 --- a/packages/nodes-base/nodes/DataTable/actions/row/Row.resource.ts +++ b/packages/nodes-base/nodes/DataTable/actions/row/Row.resource.ts @@ -1,10 +1,11 @@ import type { INodeProperties } from 'n8n-workflow'; +import * as deleteRows from './delete.operation'; import * as get from './get.operation'; import * as insert from './insert.operation'; import { DATA_TABLE_ID_FIELD } from '../../common/fields'; -export { insert, get }; +export { insert, get, deleteRows }; export const description: INodeProperties[] = [ { @@ -24,12 +25,12 @@ export const description: INodeProperties[] = [ // description: 'Create a new record, or update the current one if it already exists (upsert)', // action: 'Create or update a row', // }, - // { - // name: 'Delete', - // value: 'delete', - // description: 'Delete a row', - // action: 'Delete a row', - // }, + { + name: 'Delete', + value: deleteRows.FIELD, + description: 'Delete row(s)', + action: 'Delete row(s)', + }, { name: 'Get', value: get.FIELD, @@ -52,7 +53,7 @@ export const description: INodeProperties[] = [ default: 'insert', }, { - displayName: 'Data Store', + displayName: 'Data Table', name: DATA_TABLE_ID_FIELD, type: 'resourceLocator', default: { mode: 'list', value: '' }, @@ -75,7 +76,7 @@ export const description: INodeProperties[] = [ ], displayOptions: { show: { resource: ['row'] } }, }, - + ...deleteRows.description, ...insert.description, ...get.description, ]; diff --git a/packages/nodes-base/nodes/DataTable/actions/row/delete.operation.ts b/packages/nodes-base/nodes/DataTable/actions/row/delete.operation.ts new file mode 100644 index 0000000000..9d42224722 --- /dev/null +++ b/packages/nodes-base/nodes/DataTable/actions/row/delete.operation.ts @@ -0,0 +1,61 @@ +import { + NodeOperationError, + type IDisplayOptions, + type IExecuteFunctions, + type INodeExecutionData, + type INodeProperties, +} from 'n8n-workflow'; + +import { DRY_RUN } from '../../common/fields'; +import { executeSelectMany, getSelectFields } from '../../common/selectMany'; +import { getDataTableProxyExecute } from '../../common/utils'; + +// named `deleteRows` since `delete` is a reserved keyword +export const FIELD: string = 'deleteRows'; + +const displayOptions: IDisplayOptions = { + show: { + resource: ['row'], + operation: [FIELD], + }, +}; + +export const description: INodeProperties[] = [ + ...getSelectFields(displayOptions), + { + displayName: 'Options', + name: 'options', + type: 'collection', + default: {}, + placeholder: 'Add option', + options: [DRY_RUN], + displayOptions, + }, +]; + +export async function execute( + this: IExecuteFunctions, + index: number, +): Promise { + const dataStoreProxy = await getDataTableProxyExecute(this, index); + + const dryRun = this.getNodeParameter(`options.${DRY_RUN.name}`, index, false); + + if (typeof dryRun !== 'boolean') { + throw new NodeOperationError( + this.getNode(), + `unexpected input ${JSON.stringify(dryRun)} for boolean dryRun`, + ); + } + + const matches = await executeSelectMany(this, index, dataStoreProxy); + + if (!dryRun) { + const success = await dataStoreProxy.deleteRows(matches.map((x) => x.json.id)); + if (!success) { + throw new NodeOperationError(this.getNode(), `failed to delete rows for index ${index}`); + } + } + + return matches; +} diff --git a/packages/nodes-base/nodes/DataTable/actions/row/get.operation.ts b/packages/nodes-base/nodes/DataTable/actions/row/get.operation.ts index 0213c3fb30..8539c15763 100644 --- a/packages/nodes-base/nodes/DataTable/actions/row/get.operation.ts +++ b/packages/nodes-base/nodes/DataTable/actions/row/get.operation.ts @@ -5,7 +5,7 @@ import type { INodeProperties, } from 'n8n-workflow'; -import { getSelectFields, getSelectFilter } from '../../common/selectMany'; +import { executeSelectMany, getSelectFields } from '../../common/selectMany'; import { getDataTableProxyExecute } from '../../common/utils'; export const FIELD: string = 'get'; @@ -25,26 +25,5 @@ export async function execute( ): Promise { const dataStoreProxy = await getDataTableProxyExecute(this, index); - let take = 1000; - const result: INodeExecutionData[] = []; - - const filter = getSelectFilter(this, index); - - do { - const response = await dataStoreProxy.getManyRowsAndCount({ - skip: result.length, - take, - filter, - }); - const data = response.data.map((json) => ({ json })); - - // Optimize common path of <1000 results - if (response.count === response.data.length) { - return data; - } - - result.push.apply(result, data); - take = Math.min(take, response.count - result.length); - } while (take > 0); - return result; + return await executeSelectMany(this, index, dataStoreProxy); } diff --git a/packages/nodes-base/nodes/DataTable/common/fields.ts b/packages/nodes-base/nodes/DataTable/common/fields.ts index faccc51f83..60a9231316 100644 --- a/packages/nodes-base/nodes/DataTable/common/fields.ts +++ b/packages/nodes-base/nodes/DataTable/common/fields.ts @@ -2,7 +2,7 @@ import type { INodeProperties } from 'n8n-workflow'; export const DATA_TABLE_ID_FIELD = 'dataTableId'; -export const COLUMNS: INodeProperties = { +export const COLUMNS = { displayName: 'Columns', name: 'columns', type: 'resourceMapper', @@ -25,4 +25,13 @@ export const COLUMNS: INodeProperties = { multiKeyMatch: true, }, }, -}; +} satisfies INodeProperties; + +export const DRY_RUN = { + displayName: 'Dry Run', + name: 'dryRun', + type: 'boolean', + default: false, + description: + 'Whether the delete operation should only be simulated, returning the rows that would have been deleted', +} satisfies INodeProperties; diff --git a/packages/nodes-base/nodes/DataTable/common/selectMany.ts b/packages/nodes-base/nodes/DataTable/common/selectMany.ts index ae78e419d2..11ec68be09 100644 --- a/packages/nodes-base/nodes/DataTable/common/selectMany.ts +++ b/packages/nodes-base/nodes/DataTable/common/selectMany.ts @@ -1,8 +1,10 @@ -import { - NodeOperationError, - type IDisplayOptions, - type IExecuteFunctions, - type INodeProperties, +import { NodeOperationError } from 'n8n-workflow'; +import type { + DataStoreRowReturn, + IDataStoreProjectService, + IDisplayOptions, + IExecuteFunctions, + INodeProperties, } from 'n8n-workflow'; import type { FilterType } from './constants'; @@ -98,3 +100,41 @@ export function getSelectFilter(ctx: IExecuteFunctions, index: number) { return buildGetManyFilter(fields, matchType); } + +export async function executeSelectMany( + ctx: IExecuteFunctions, + index: number, + dataStoreProxy: IDataStoreProjectService, +): Promise> { + const filter = getSelectFilter(ctx, index); + + let take = 1000; + const result: Array<{ json: DataStoreRowReturn }> = []; + let totalCount = undefined; + do { + const response = await dataStoreProxy.getManyRowsAndCount({ + skip: result.length, + take, + filter, + }); + const data = response.data.map((json) => ({ json })); + + // Optimize common path of <1000 results + if (response.count === response.data.length) { + return data; + } + + if (totalCount !== undefined && response.count !== totalCount) { + throw new NodeOperationError( + ctx.getNode(), + 'synchronization error: result count changed during pagination', + ); + } + totalCount = response.count; + + result.push.apply(result, data); + take = Math.min(take, response.count - result.length); + } while (take > 0); + + return result; +} diff --git a/packages/nodes-base/nodes/DataTable/test/actions/rows/get.operation.test.ts b/packages/nodes-base/nodes/DataTable/test/common/selectMany.test.ts similarity index 53% rename from packages/nodes-base/nodes/DataTable/test/actions/rows/get.operation.test.ts rename to packages/nodes-base/nodes/DataTable/test/common/selectMany.test.ts index 843bb8d42a..3edd37e9f9 100644 --- a/packages/nodes-base/nodes/DataTable/test/actions/rows/get.operation.test.ts +++ b/packages/nodes-base/nodes/DataTable/test/common/selectMany.test.ts @@ -1,15 +1,24 @@ -import type { IExecuteFunctions } from 'n8n-workflow'; +import { + type INode, + NodeOperationError, + type IDataStoreProjectService, + type IExecuteFunctions, +} from 'n8n-workflow'; -import { execute } from '../../../actions/row/get.operation'; -import type { FieldEntry } from '../../../common/constants'; -import { ANY_FILTER } from '../../../common/constants'; -import { DATA_TABLE_ID_FIELD } from '../../../common/fields'; +import type { FieldEntry } from '../../common/constants'; +import { ANY_FILTER } from '../../common/constants'; +import { DATA_TABLE_ID_FIELD } from '../../common/fields'; +import { executeSelectMany } from '../../common/selectMany'; -describe('Data Table get Operation', () => { +describe('selectMany utils', () => { let mockExecuteFunctions: IExecuteFunctions; const getManyRowsAndCount = jest.fn(); + const dataStoreProxy = jest.mocked({ + getManyRowsAndCount, + } as unknown as IDataStoreProjectService); const dataTableId = 2345; let filters: FieldEntry[]; + const node = { id: 1 } as unknown as INode; beforeEach(() => { filters = [ @@ -20,8 +29,7 @@ describe('Data Table get Operation', () => { }, ]; mockExecuteFunctions = { - getNode: jest.fn().mockReturnValue({}), - getInputData: jest.fn().mockReturnValue([{}]), + getNode: jest.fn().mockReturnValue(node), getNodeParameter: jest.fn().mockImplementation((field) => { switch (field) { case DATA_TABLE_ID_FIELD: @@ -32,23 +40,18 @@ describe('Data Table get Operation', () => { return ANY_FILTER; } }), - helpers: { - getDataStoreProxy: jest.fn().mockReturnValue({ - getManyRowsAndCount, - }), - }, } as unknown as IExecuteFunctions; jest.clearAllMocks(); }); - describe('execute', () => { + describe('executeSelectMany', () => { it('should get a few rows', async () => { // ARRANGE getManyRowsAndCount.mockReturnValue({ data: [{ id: 1 }], count: 1 }); // ACT - const result = await execute.call(mockExecuteFunctions, 0); + const result = await executeSelectMany(mockExecuteFunctions, 0, dataStoreProxy); // ASSERT expect(result).toEqual([{ json: { id: 1 } }]); @@ -72,7 +75,7 @@ describe('Data Table get Operation', () => { filters = []; // ACT - const result = await execute.call(mockExecuteFunctions, 0); + const result = await executeSelectMany(mockExecuteFunctions, 0, dataStoreProxy); // ASSERT expect(result.length).toBe(2345); @@ -84,10 +87,31 @@ describe('Data Table get Operation', () => { getManyRowsAndCount.mockReturnValue({ data: [{ id: 1, colA: null }], count: 1 }); // ACT - const result = await execute.call(mockExecuteFunctions, 0); + const result = await executeSelectMany(mockExecuteFunctions, 0, dataStoreProxy); // ASSERT expect(result).toEqual([{ json: { id: 1, colA: null } }]); }); + it('should panic if pagination gets out of sync', async () => { + // ARRANGE + getManyRowsAndCount.mockReturnValueOnce({ + data: Array.from({ length: 1000 }, (_, k) => ({ id: k })), + count: 2345, + }); + getManyRowsAndCount.mockReturnValueOnce({ + data: Array.from({ length: 1000 }, (_, k) => ({ id: k + 1000 })), + count: 2344, + }); + + filters = []; + + // ACT ASSERT + await expect(executeSelectMany(mockExecuteFunctions, 0, dataStoreProxy)).rejects.toEqual( + new NodeOperationError( + node, + 'synchronization error: result count changed during pagination', + ), + ); + }); }); });