feat(Data Table Node): Add Delete operation (no-changelog) (#18785)

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<INodeExecutionData[][]> {
 	const operationResult: INodeExecutionData[] = [];

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,
 ];

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<INodeExecutionData[]> {
+	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;
+}

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<INodeExecutionData[]> {
 	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);
 }

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;

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<Array<{ json: DataStoreRowReturn }>> {
+	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;
+}

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<IDataStoreProjectService>({
+		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',
+				),
+			);
+		});
 	});
 });