feat(Summarize Node): Turns error when field not found in items into warning (#11889)

Co-authored-by: Dana Lee <dana@n8n.io>
Co-authored-by: Elias Meire <elias@meire.dev>

packages/nodes-base/nodes/Transform/Summarize/Summarize.node.ts

@@ -5,6 +5,8 @@ import {
 	type INodeExecutionData,
 	type INodeType,
 	type INodeTypeDescription,
+	NodeExecutionOutput,
+	type NodeExecutionHint,
 } from 'n8n-workflow';
 
 import {
@@ -16,7 +18,6 @@ import {
 	fieldValueGetter,
 	splitData,
 } from './utils';
-import { generatePairedItemData } from '../../../utils/utilities';
 
 export class Summarize implements INodeType {
 	description: INodeTypeDescription = {
@@ -25,7 +26,7 @@ export class Summarize implements INodeType {
 		icon: 'file:summarize.svg',
 		group: ['transform'],
 		subtitle: '',
-		version: 1,
+		version: [1, 1.1],
 		description: 'Sum, count, max, etc. across items',
 		defaults: {
 			name: 'Summarize',
@@ -248,7 +249,12 @@ export class Summarize implements INodeType {
 						type: 'boolean',
 						default: false,
 						description:
-							"Whether to continue if field to summarize can't be found in any items and return single empty item, owerwise an error would be thrown",
+							"Whether to continue if field to summarize can't be found in any items and return single empty item, otherwise an error would be thrown",
+						displayOptions: {
+							hide: {
+								'@version': [{ _cnd: { gte: 1.1 } }],
+							},
+						},
 					},
 					{
 						displayName: 'Disable Dot Notation',
@@ -314,20 +320,6 @@ export class Summarize implements INodeType {
 
 		const nodeVersion = this.getNode().typeVersion;
 
-		if (nodeVersion < 2.1) {
-			try {
-				checkIfFieldExists.call(this, newItems, fieldsToSummarize, getValue);
-			} catch (error) {
-				if (options.continueIfFieldNotFound) {
-					const itemData = generatePairedItemData(items.length);
-
-					return [[{ json: {}, pairedItem: itemData }]];
-				} else {
-					throw error;
-				}
-			}
-		}
-
 		const aggregationResult = splitData(
 			fieldsToSplitBy,
 			newItems,
@@ -336,6 +328,21 @@ export class Summarize implements INodeType {
 			getValue,
 		);
 
+		const fieldsNotFound: NodeExecutionHint[] = [];
+		try {
+			checkIfFieldExists.call(this, newItems, fieldsToSummarize, getValue);
+		} catch (error) {
+			if (nodeVersion > 1 || options.continueIfFieldNotFound) {
+				const fieldNotFoundHint: NodeExecutionHint = {
+					message: error instanceof Error ? error.message : String(error),
+					location: 'outputPane',
+				};
+				fieldsNotFound.push(fieldNotFoundHint);
+			} else {
+				throw error;
+			}
+		}
+
 		if (options.outputFormat === 'singleItem') {
 			const executionData: INodeExecutionData = {
 				json: aggregationResult,
@@ -343,7 +350,7 @@ export class Summarize implements INodeType {
 					item: index,
 				})),
 			};
-			return [[executionData]];
+			return new NodeExecutionOutput([[executionData]], fieldsNotFound);
 		} else {
 			if (!fieldsToSplitBy.length) {
 				const { pairedItems, ...json } = aggregationResult;
@@ -353,7 +360,7 @@ export class Summarize implements INodeType {
 						item: index,
 					})),
 				};
-				return [[executionData]];
+				return new NodeExecutionOutput([[executionData]], fieldsNotFound);
 			}
 			const returnData = aggregationToArray(aggregationResult, fieldsToSplitBy);
 			const executionData = returnData.map((item) => {
@@ -365,7 +372,7 @@ export class Summarize implements INodeType {
 					})),
 				};
 			});
-			return [executionData];
+			return new NodeExecutionOutput([executionData], fieldsNotFound);
 		}
 	}
 }

packages/nodes-base/nodes/Transform/Summarize/test/unitTests/checkIfFieldExists.test.ts

@@ -0,0 +1,53 @@
+import { NodeOperationError, type IExecuteFunctions, type IDataObject } from 'n8n-workflow';
+
+import { checkIfFieldExists, type Aggregations } from '../../utils';
+
+describe('Test Summarize Node, checkIfFieldExists', () => {
+	let mockExecuteFunctions: IExecuteFunctions;
+
+	beforeEach(() => {
+		mockExecuteFunctions = {
+			getNode: jest.fn().mockReturnValue({ name: 'test-node' }),
+		} as unknown as IExecuteFunctions;
+	});
+
+	const items = [{ a: 1 }, { b: 2 }, { c: 3 }];
+
+	it('should not throw error if all fields exist', () => {
+		const aggregations: Aggregations = [
+			{ aggregation: 'sum', field: 'a' },
+			{ aggregation: 'count', field: 'c' },
+		];
+		const getValue = (item: IDataObject, field: string) => item[field];
+		expect(() => {
+			checkIfFieldExists.call(mockExecuteFunctions, items, aggregations, getValue);
+		}).not.toThrow();
+	});
+
+	it('should throw NodeOperationError if any field does not exist', () => {
+		const aggregations: Aggregations = [
+			{ aggregation: 'sum', field: 'b' },
+			{ aggregation: 'count', field: 'd' },
+		];
+		const getValue = (item: IDataObject, field: string) => item[field];
+		expect(() => {
+			checkIfFieldExists.call(mockExecuteFunctions, items, aggregations, getValue);
+		}).toThrow(NodeOperationError);
+	});
+
+	it("should throw NodeOperationError with error message containing the field name that doesn't exist", () => {
+		const aggregations: Aggregations = [{ aggregation: 'count', field: 'D' }];
+		const getValue = (item: IDataObject, field: string) => item[field];
+		expect(() => {
+			checkIfFieldExists.call(mockExecuteFunctions, items, aggregations, getValue);
+		}).toThrow("The field 'D' does not exist in any items");
+	});
+
+	it('should not throw error if field is empty string', () => {
+		const aggregations: Aggregations = [{ aggregation: 'count', field: '' }];
+		const getValue = (item: IDataObject, field: string) => item[field];
+		expect(() => {
+			checkIfFieldExists.call(mockExecuteFunctions, items, aggregations, getValue);
+		}).not.toThrow();
+	});
+});

packages/nodes-base/nodes/Transform/Summarize/test/unitTests/execute.test.ts

@@ -0,0 +1,80 @@
+import type { MockProxy } from 'jest-mock-extended';
+import { mock } from 'jest-mock-extended';
+import type { IExecuteFunctions } from 'n8n-workflow';
+import { NodeExecutionOutput, NodeOperationError } from 'n8n-workflow';
+
+import { Summarize } from '../../Summarize.node';
+import type { Aggregations } from '../../utils';
+
+let summarizeNode: Summarize;
+let mockExecuteFunctions: MockProxy<IExecuteFunctions>;
+
+describe('Test Summarize Node, execute', () => {
+	beforeEach(() => {
+		summarizeNode = new Summarize();
+		mockExecuteFunctions = mock<IExecuteFunctions>({
+			getNode: jest.fn().mockReturnValue({ name: 'test-node' }),
+			getNodeParameter: jest.fn(),
+			getInputData: jest.fn(),
+			helpers: {
+				constructExecutionMetaData: jest.fn().mockReturnValue([]),
+			},
+		});
+	});
+
+	afterEach(() => {
+		jest.clearAllMocks();
+	});
+
+	it('should handle field not found with hints if version > 1', async () => {
+		mockExecuteFunctions.getInputData.mockReturnValue([{ json: { someField: 1 } }]);
+		mockExecuteFunctions.getNode.mockReturnValue({
+			id: '1',
+			name: 'test-node',
+			type: 'test-type',
+			position: [0, 0],
+			parameters: {},
+			typeVersion: 1.1,
+		});
+		mockExecuteFunctions.getNodeParameter
+			.mockReturnValueOnce({}) // options
+			.mockReturnValueOnce('') // fieldsToSplitBy
+			.mockReturnValueOnce([{ field: 'nonexistentField', aggregation: 'sum' }]); // fieldsToSummarize
+
+		const result = await summarizeNode.execute.call(mockExecuteFunctions);
+
+		expect(result).toBeInstanceOf(NodeExecutionOutput);
+		expect(result).toEqual([[{ json: { sum_nonexistentField: 0 }, pairedItem: [{ item: 0 }] }]]);
+		expect((result as NodeExecutionOutput).getHints()).toEqual([
+			{
+				location: 'outputPane',
+				message: "The field 'nonexistentField' does not exist in any items",
+			},
+		]);
+	});
+
+	it('should throw error if node version is < 1.1 and fields not found', async () => {
+		const items = [{ json: { a: 1, b: 2, c: 3 } }];
+		const aggregations: Aggregations = [
+			{ aggregation: 'sum', field: 'b' },
+			{ aggregation: 'count', field: 'd' },
+		];
+
+		mockExecuteFunctions.getNode.mockReturnValue({
+			id: '1',
+			name: 'test-node',
+			type: 'test-type',
+			position: [0, 0],
+			parameters: {},
+			typeVersion: 1,
+		});
+		mockExecuteFunctions.getInputData.mockReturnValue(items);
+		mockExecuteFunctions.getNodeParameter
+			.mockReturnValueOnce({}) // options
+			.mockReturnValueOnce('') // fieldsToSplitBy
+			.mockReturnValueOnce(aggregations); // fieldsToSummarize
+		await expect(async () => {
+			await summarizeNode.execute.bind(mockExecuteFunctions)();
+		}).rejects.toThrow(NodeOperationError);
+	});
+});

packages/nodes-base/nodes/Transform/Summarize/test/workflow.summarize_hint.json

@@ -0,0 +1,70 @@
+{
+	"nodes": [
+		{
+			"parameters": {},
+			"type": "n8n-nodes-base.manualTrigger",
+			"typeVersion": 1,
+			"position": [520, -80],
+			"id": "9ff1d8e0-ebe8-4c52-931a-e482e14ac911",
+			"name": "When clicking ‘Test workflow’"
+		},
+		{
+			"parameters": {
+				"category": "randomData",
+				"randomDataSeed": "ria",
+				"randomDataCount": 5
+			},
+			"type": "n8n-nodes-base.debugHelper",
+			"typeVersion": 1,
+			"position": [700, -80],
+			"id": "50cea846-f8ed-4b5a-a14a-c11bf4ba0f5d",
+			"name": "DebugHelper2"
+		},
+		{
+			"parameters": {
+				"fieldsToSummarize": {
+					"values": [
+						{
+							"field": "passwor"
+						}
+					]
+				},
+				"options": {}
+			},
+			"type": "n8n-nodes-base.summarize",
+			"typeVersion": 1.1,
+			"position": [900, -80],
+			"id": "d39fedc9-62e1-45bc-8c9a-d20f2b63b543",
+			"name": "Summarize1"
+		}
+	],
+	"connections": {
+		"When clicking ‘Test workflow’": {
+			"main": [
+				[
+					{
+						"node": "DebugHelper2",
+						"type": "main",
+						"index": 0
+					}
+				]
+			]
+		},
+		"DebugHelper2": {
+			"main": [
+				[
+					{
+						"node": "Summarize1",
+						"type": "main",
+						"index": 0
+					}
+				]
+			]
+		}
+	},
+	"pinData": {},
+	"meta": {
+		"templateCredsSetupCompleted": true,
+		"instanceId": "ee90fdf8d57662f949e6c691dc07fa0fd2f66e1eee28ed82ef06658223e67255"
+	}
+}

packages/nodes-base/nodes/Transform/Summarize/utils.ts

@@ -231,20 +231,20 @@ export function splitData(
 	const [firstSplitKey, ...restSplitKeys] = splitKeys;
 
 	const groupedData = data.reduce((acc, item) => {
-		let keyValuee = getValue(item, firstSplitKey) as string;
+		let keyValue = getValue(item, firstSplitKey) as string;
 
-		if (typeof keyValuee === 'object') {
-			keyValuee = JSON.stringify(keyValuee);
+		if (typeof keyValue === 'object') {
+			keyValue = JSON.stringify(keyValue);
 		}
 
-		if (options.skipEmptySplitFields && typeof keyValuee !== 'number' && !keyValuee) {
+		if (options.skipEmptySplitFields && typeof keyValue !== 'number' && !keyValue) {
 			return acc;
 		}
 
-		if (acc[keyValuee] === undefined) {
-			acc[keyValuee] = [item];
+		if (acc[keyValue] === undefined) {
+			acc[keyValue] = [item];
 		} else {
-			(acc[keyValuee] as IDataObject[]).push(item);
+			(acc[keyValue] as IDataObject[]).push(item);
 		}
 		return acc;
 	}, {} as IDataObject);