fix(Summarize Node): Fix type casting of strings and numbers (#14259)

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

@@ -1,23 +1,22 @@
 import {
-	NodeOperationError,
-	NodeConnectionTypes,
 	type IExecuteFunctions,
 	type INodeExecutionData,
 	type INodeType,
 	type INodeTypeDescription,
+	NodeConnectionTypes,
 	type NodeExecutionHint,
-	type IDataObject,
+	NodeOperationError,
 } from 'n8n-workflow';
 
 import {
 	type Aggregations,
 	NUMERICAL_AGGREGATIONS,
 	type SummarizeOptions,
-	aggregationToArray,
-	aggregationToArrayWithOriginalTypes,
+	aggregateAndSplitData,
 	checkIfFieldExists,
 	fieldValueGetter,
-	splitData,
+	flattenAggregationResultToArray,
+	flattenAggregationResultToObject,
 } from './utils';
 
 export class Summarize implements INodeType {
@@ -321,13 +320,14 @@ export class Summarize implements INodeType {
 
 		const nodeVersion = this.getNode().typeVersion;
 
-		const aggregationResult = splitData(
-			fieldsToSplitBy,
-			newItems,
+		const aggregationResult = aggregateAndSplitData({
+			splitKeys: fieldsToSplitBy,
+			inputItems: newItems,
 			fieldsToSummarize,
 			options,
 			getValue,
-		);
+			convertKeysToString: nodeVersion === 1,
+		});
 
 		const fieldsNotFound: NodeExecutionHint[] = [];
 		try {
@@ -350,36 +350,27 @@ export class Summarize implements INodeType {
 
 		if (options.outputFormat === 'singleItem') {
 			const executionData: INodeExecutionData = {
-				json: aggregationResult,
+				json: flattenAggregationResultToObject(aggregationResult),
 				pairedItem: newItems.map((_v, index) => ({
 					item: index,
 				})),
 			};
 			return [[executionData]];
 		} else {
-			if (!fieldsToSplitBy.length) {
-				const { pairedItems, ...json } = aggregationResult;
+			if (!fieldsToSplitBy.length && 'pairedItems' in aggregationResult) {
+				const { pairedItems, returnData } = aggregationResult;
 				const executionData: INodeExecutionData = {
-					json,
-					pairedItem: ((pairedItems as number[]) || []).map((index: number) => ({
-						item: index,
-					})),
+					json: returnData,
+					pairedItem: (pairedItems ?? []).map((index) => ({ item: index })),
 				};
 				return [[executionData]];
 			}
-			let returnData: IDataObject[] = [];
-			if (nodeVersion > 1) {
-				returnData = aggregationToArrayWithOriginalTypes(aggregationResult, fieldsToSplitBy);
-			} else {
-				returnData = aggregationToArray(aggregationResult, fieldsToSplitBy);
-			}
-			const executionData = returnData.map((item) => {
-				const { pairedItems, ...json } = item;
+			const flatAggregationResults = flattenAggregationResultToArray(aggregationResult);
+			const executionData = flatAggregationResults.map((item) => {
+				const { pairedItems, returnData } = item;
 				return {
-					json,
-					pairedItem: ((pairedItems as number[]) || []).map((index: number) => ({
-						item: index,
-					})),
+					json: returnData,
+					pairedItem: (pairedItems ?? []).map((index) => ({ item: index })),
 				};
 			});
 			return [executionData];

packages/nodes-base/nodes/Transform/Summarize/test/unitTests/__snapshots__/splitData.test.ts.snap

@@ -0,0 +1,228 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Test Summarize Node, aggregateAndSplitData should not convert numbers to strings: array 1`] = `
+[
+  {
+    "pairedItems": [
+      0,
+    ],
+    "returnData": {
+      "Qty": 1,
+      "Sku": 12345,
+      "appended_Warehouse": [
+        "BER_0G",
+      ],
+    },
+  },
+  {
+    "pairedItems": [
+      1,
+    ],
+    "returnData": {
+      "Qty": 2,
+      "Sku": 12345,
+      "appended_Warehouse": [
+        "BER_0L",
+      ],
+    },
+  },
+  {
+    "pairedItems": [
+      2,
+    ],
+    "returnData": {
+      "Qty": 1,
+      "Sku": 6534563534,
+      "appended_Warehouse": [
+        "BER_0L",
+      ],
+    },
+  },
+]
+`;
+
+exports[`Test Summarize Node, aggregateAndSplitData should not convert numbers to strings: result 1`] = `
+{
+  "fieldName": "Sku",
+  "splits": Map {
+    12345 => {
+      "fieldName": "Qty",
+      "splits": Map {
+        1 => {
+          "pairedItems": [
+            0,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0G",
+            ],
+          },
+        },
+        2 => {
+          "pairedItems": [
+            1,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0L",
+            ],
+          },
+        },
+      },
+    },
+    6534563534 => {
+      "fieldName": "Qty",
+      "splits": Map {
+        1 => {
+          "pairedItems": [
+            2,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0L",
+            ],
+          },
+        },
+      },
+    },
+  },
+}
+`;
+
+exports[`Test Summarize Node, aggregateAndSplitData should not convert strings to numbers: array 1`] = `
+[
+  {
+    "pairedItems": [
+      0,
+    ],
+    "returnData": {
+      "Qty": "1",
+      "Sku": "012345",
+      "appended_Warehouse": [
+        "BER_0G",
+      ],
+    },
+  },
+  {
+    "pairedItems": [
+      1,
+    ],
+    "returnData": {
+      "Qty": "2",
+      "Sku": "012345",
+      "appended_Warehouse": [
+        "BER_0L",
+      ],
+    },
+  },
+  {
+    "pairedItems": [
+      2,
+    ],
+    "returnData": {
+      "Qty": "1",
+      "Sku": "06534563534",
+      "appended_Warehouse": [
+        "BER_0L",
+      ],
+    },
+  },
+]
+`;
+
+exports[`Test Summarize Node, aggregateAndSplitData should not convert strings to numbers: result 1`] = `
+{
+  "fieldName": "Sku",
+  "splits": Map {
+    "012345" => {
+      "fieldName": "Qty",
+      "splits": Map {
+        "1" => {
+          "pairedItems": [
+            0,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0G",
+            ],
+          },
+        },
+        "2" => {
+          "pairedItems": [
+            1,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0L",
+            ],
+          },
+        },
+      },
+    },
+    "06534563534" => {
+      "fieldName": "Qty",
+      "splits": Map {
+        "1" => {
+          "pairedItems": [
+            2,
+          ],
+          "returnData": {
+            "appended_Warehouse": [
+              "BER_0L",
+            ],
+          },
+        },
+      },
+    },
+  },
+}
+`;
+
+exports[`Test Summarize Node, aggregateAndSplitData with skipEmptySplitFields=true should skip empty split fields: array 1`] = `
+[
+  {
+    "pairedItems": [
+      0,
+      3,
+    ],
+    "returnData": {
+      "Sku": 12345,
+      "concatenated_Warehouse": "BER_0G//{"name":"BER_0G3"}",
+    },
+  },
+  {
+    "pairedItems": [
+      2,
+    ],
+    "returnData": {
+      "Sku": "{}",
+      "concatenated_Warehouse": "BER_0L",
+    },
+  },
+]
+`;
+
+exports[`Test Summarize Node, aggregateAndSplitData with skipEmptySplitFields=true should skip empty split fields: result 1`] = `
+{
+  "fieldName": "Sku",
+  "splits": Map {
+    12345 => {
+      "pairedItems": [
+        0,
+        3,
+      ],
+      "returnData": {
+        "concatenated_Warehouse": "BER_0G//{"name":"BER_0G3"}",
+      },
+    },
+    "{}" => {
+      "pairedItems": [
+        2,
+      ],
+      "returnData": {
+        "concatenated_Warehouse": "BER_0L",
+      },
+    },
+  },
+}
+`;

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

@@ -0,0 +1,135 @@
+import {
+	fieldValueGetter,
+	aggregateAndSplitData,
+	flattenAggregationResultToArray,
+	type Aggregations,
+} from '../../utils';
+
+describe('Test Summarize Node, aggregateAndSplitData', () => {
+	test('should not convert strings to numbers', () => {
+		const data = [
+			{
+				Sku: '012345',
+				Warehouse: 'BER_0G',
+				Qty: '1',
+				_itemIndex: 0,
+			},
+			{
+				Sku: '012345',
+				Warehouse: 'BER_0L',
+				Qty: '2',
+				_itemIndex: 1,
+			},
+			{
+				Sku: '06534563534',
+				Warehouse: 'BER_0L',
+				Qty: '1',
+				_itemIndex: 2,
+			},
+		];
+
+		const aggregations: Aggregations = [
+			{
+				aggregation: 'append',
+				field: 'Warehouse',
+				includeEmpty: true,
+			},
+		];
+
+		const result = aggregateAndSplitData({
+			splitKeys: ['Sku', 'Qty'],
+			inputItems: data,
+			fieldsToSummarize: aggregations,
+			options: { continueIfFieldNotFound: true },
+			getValue: fieldValueGetter(),
+		});
+		expect(result).toMatchSnapshot('result');
+		expect(flattenAggregationResultToArray(result)).toMatchSnapshot('array');
+	});
+
+	test('should not convert numbers to strings', () => {
+		const data = [
+			{
+				Sku: 12345,
+				Warehouse: 'BER_0G',
+				Qty: 1,
+				_itemIndex: 0,
+			},
+			{
+				Sku: 12345,
+				Warehouse: 'BER_0L',
+				Qty: 2,
+				_itemIndex: 1,
+			},
+			{
+				Sku: 6534563534,
+				Warehouse: 'BER_0L',
+				Qty: 1,
+				_itemIndex: 2,
+			},
+		];
+
+		const aggregations: Aggregations = [
+			{
+				aggregation: 'append',
+				field: 'Warehouse',
+				includeEmpty: true,
+			},
+		];
+
+		const result = aggregateAndSplitData({
+			splitKeys: ['Sku', 'Qty'],
+			inputItems: data,
+			fieldsToSummarize: aggregations,
+			options: { continueIfFieldNotFound: true },
+			getValue: fieldValueGetter(),
+		});
+		expect(result).toMatchSnapshot('result');
+		expect(flattenAggregationResultToArray(result)).toMatchSnapshot('array');
+	});
+
+	describe('with skipEmptySplitFields=true', () => {
+		test('should skip empty split fields', () => {
+			const data = [
+				{
+					Sku: 12345,
+					Warehouse: 'BER_0G',
+					_itemIndex: 0,
+				},
+				{
+					Warehouse: 'BER_0L',
+					_itemIndex: 1,
+				},
+				{
+					Sku: {},
+					Warehouse: 'BER_0L',
+					_itemIndex: 2,
+				},
+				{
+					Sku: 12345,
+					Warehouse: { name: 'BER_0G3' },
+					_itemIndex: 3,
+				},
+			];
+
+			const aggregations: Aggregations = [
+				{
+					aggregation: 'concatenate',
+					field: 'Warehouse',
+					separateBy: 'other',
+					customSeparator: '//',
+				},
+			];
+
+			const result = aggregateAndSplitData({
+				splitKeys: ['Sku'],
+				inputItems: data,
+				fieldsToSummarize: aggregations,
+				options: { continueIfFieldNotFound: true, skipEmptySplitFields: true },
+				getValue: fieldValueGetter(),
+			});
+			expect(result).toMatchSnapshot('result');
+			expect(flattenAggregationResultToArray(result)).toMatchSnapshot('array');
+		});
+	});
+});

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

@@ -1,8 +1,7 @@
-import { isNaN } from 'lodash';
 import get from 'lodash/get';
 import {
-	type IDataObject,
 	type GenericValue,
+	type IDataObject,
 	type IExecuteFunctions,
 	NodeOperationError,
 } from 'n8n-workflow';
@@ -56,42 +55,13 @@ function isEmpty<T>(value: T) {
 	return value === undefined || value === null || value === '';
 }
 
-function parseReturnData(returnData: IDataObject) {
-	const regexBrackets = /[\]\["]/g;
-	const regexSpaces = /[ .]/g;
-	for (const key of Object.keys(returnData)) {
-		if (key.match(regexBrackets)) {
-			const newKey = key.replace(regexBrackets, '');
-			returnData[newKey] = returnData[key];
-			delete returnData[key];
-		}
-	}
-	for (const key of Object.keys(returnData)) {
-		if (key.match(regexSpaces)) {
-			const newKey = key.replace(regexSpaces, '_');
-			returnData[newKey] = returnData[key];
-			delete returnData[key];
-		}
-	}
-}
-
-function parseFieldName(fieldName: string[]) {
-	const regexBrackets = /[\]\["]/g;
-	const regexSpaces = /[ .]/g;
-	fieldName = fieldName.map((field) => {
-		field = field.replace(regexBrackets, '');
-		field = field.replace(regexSpaces, '_');
-		return field;
-	});
-	return fieldName;
+function normalizeFieldName(fieldName: string) {
+	return fieldName.replace(/[\]\["]/g, '').replace(/[ .]/g, '');
 }
 
 export const fieldValueGetter = (disableDotNotation?: boolean) => {
-	if (disableDotNotation) {
-		return (item: IDataObject, field: string) => item[field];
-	} else {
-		return (item: IDataObject, field: string) => get(item, field);
-	}
+	return (item: IDataObject, field: string) =>
+		disableDotNotation ? item[field] : get(item, field);
 };
 
 export function checkIfFieldExists(
@@ -207,126 +177,111 @@ function aggregateData(
 	fieldsToSummarize: Aggregations,
 	options: SummarizeOptions,
 	getValue: ValueGetterFn,
-) {
-	const returnData = fieldsToSummarize.reduce((acc, aggregation) => {
-		acc[`${AggregationDisplayNames[aggregation.aggregation]}${aggregation.field}`] = aggregate(
-			data,
-			aggregation,
-			getValue,
+): { returnData: IDataObject; pairedItems?: number[] } {
+	const returnData = Object.fromEntries(
+		fieldsToSummarize.map((aggregation) => {
+			const key = normalizeFieldName(
+				`${AggregationDisplayNames[aggregation.aggregation]}${aggregation.field}`,
 			);
-		return acc;
-	}, {} as IDataObject);
-	parseReturnData(returnData);
+			const result = aggregate(data, aggregation, getValue);
+			return [key, result];
+		}),
+	);
+
 	if (options.outputFormat === 'singleItem') {
-		parseReturnData(returnData);
-		return returnData;
-	} else {
-		return { ...returnData, pairedItems: data.map((item) => item._itemIndex as number) };
+		return { returnData };
 	}
+
+	return { returnData, pairedItems: data.map((item) => item._itemIndex as number) };
 }
 
-export function splitData(
-	splitKeys: string[],
-	data: IDataObject[],
-	fieldsToSummarize: Aggregations,
-	options: SummarizeOptions,
-	getValue: ValueGetterFn,
-) {
-	if (!splitKeys || splitKeys.length === 0) {
-		return aggregateData(data, fieldsToSummarize, options, getValue);
+type AggregationResult = { returnData: IDataObject; pairedItems?: number[] };
+type NestedAggregationResult =
+	| AggregationResult
+	| { fieldName: string; splits: Map<unknown, NestedAggregationResult> };
+
+// Using Map to preserve types
+// With a plain JS object, keys are converted to string
+export function aggregateAndSplitData({
+	splitKeys,
+	inputItems,
+	fieldsToSummarize,
+	options,
+	getValue,
+	convertKeysToString = false,
+}: {
+	splitKeys: string[] | undefined;
+	inputItems: IDataObject[];
+	fieldsToSummarize: Aggregations;
+	options: SummarizeOptions;
+	getValue: ValueGetterFn;
+	convertKeysToString?: boolean; // Legacy option for v1
+}): NestedAggregationResult {
+	if (!splitKeys?.length) {
+		return aggregateData(inputItems, fieldsToSummarize, options, getValue);
 	}
 
 	const [firstSplitKey, ...restSplitKeys] = splitKeys;
 
-	const groupedData = data.reduce((acc, item) => {
-		let keyValue = getValue(item, firstSplitKey) as string;
+	const groupedItems = new Map<unknown, IDataObject[]>();
+	for (const item of inputItems) {
+		let key = getValue(item, firstSplitKey);
 
-		if (typeof keyValue === 'object') {
-			keyValue = JSON.stringify(keyValue);
+		if (key && typeof key === 'object') {
+			key = JSON.stringify(key);
 		}
 
-		if (options.skipEmptySplitFields && typeof keyValue !== 'number' && !keyValue) {
-			return acc;
+		if (convertKeysToString) {
+			key = normalizeFieldName(String(key));
 		}
 
-		if (acc[keyValue] === undefined) {
-			acc[keyValue] = [item];
-		} else {
-			(acc[keyValue] as IDataObject[]).push(item);
+		if (options.skipEmptySplitFields && typeof key !== 'number' && !key) {
+			continue;
 		}
-		return acc;
-	}, {} as IDataObject);
 
-	return Object.keys(groupedData).reduce((acc, key) => {
-		const value = groupedData[key] as IDataObject[];
-		acc[key] = splitData(restSplitKeys, value, fieldsToSummarize, options, getValue);
-		return acc;
-	}, {} as IDataObject);
+		const group = groupedItems.get(key) ?? [];
+		groupedItems.set(key, group.concat([item]));
+	}
+
+	const splits = new Map(
+		Array.from(groupedItems.entries()).map(([groupKey, items]) => [
+			groupKey,
+			aggregateAndSplitData({
+				splitKeys: restSplitKeys,
+				inputItems: items,
+				fieldsToSummarize,
+				options,
+				getValue,
+			}),
+		]),
+	);
+
+	return { fieldName: firstSplitKey, splits };
 }
 
-export function aggregationToArray(
-	aggregationResult: IDataObject,
-	fieldsToSplitBy: string[],
-	previousStage: IDataObject = {},
-) {
-	const returnData: IDataObject[] = [];
-	fieldsToSplitBy = parseFieldName(fieldsToSplitBy);
-	const splitFieldName = fieldsToSplitBy[0];
-	const isNext = fieldsToSplitBy[1];
-
-	if (isNext === undefined) {
-		for (const fieldName of Object.keys(aggregationResult)) {
-			returnData.push({
-				...previousStage,
-				[splitFieldName]: fieldName,
-				...(aggregationResult[fieldName] as IDataObject),
-			});
+export function flattenAggregationResultToObject(result: NestedAggregationResult): IDataObject {
+	if ('splits' in result) {
+		return Object.fromEntries(
+			Array.from(result.splits.entries()).map(([key, value]) => [
+				key,
+				flattenAggregationResultToObject(value),
+			]),
+		);
 	}
-		return returnData;
-	} else {
-		for (const key of Object.keys(aggregationResult)) {
-			returnData.push(
-				...aggregationToArray(aggregationResult[key] as IDataObject, fieldsToSplitBy.slice(1), {
-					...previousStage,
-					[splitFieldName]: key,
+
+	return result.returnData;
+}
+
+export function flattenAggregationResultToArray(
+	result: NestedAggregationResult,
+): AggregationResult[] {
+	if ('splits' in result) {
+		return Array.from(result.splits.entries()).flatMap(([value, innerResult]) =>
+			flattenAggregationResultToArray(innerResult).map((v) => {
+				v.returnData[normalizeFieldName(result.fieldName)] = value as IDataObject;
+				return v;
 			}),
 		);
 	}
-		return returnData;
-	}
-}
-
-const getOriginalFieldValue = (field: string | number) =>
-	field === 'null' ? null : isNaN(Number(field)) ? field : Number(field);
-
-export function aggregationToArrayWithOriginalTypes(
-	aggregationResult: IDataObject,
-	fieldsToSplitBy: string[],
-	previousStage: IDataObject = {},
-) {
-	const returnData: IDataObject[] = [];
-	fieldsToSplitBy = parseFieldName(fieldsToSplitBy);
-	const splitFieldName = fieldsToSplitBy[0];
-	const isNext = fieldsToSplitBy[1];
-
-	if (isNext === undefined) {
-		for (const fieldName of Object.keys(aggregationResult)) {
-			returnData.push({
-				...previousStage,
-				[splitFieldName]: getOriginalFieldValue(fieldName),
-				...(aggregationResult[fieldName] as IDataObject),
-			});
-		}
-		return returnData;
-	} else {
-		for (const key of Object.keys(aggregationResult)) {
-			returnData.push(
-				...aggregationToArray(aggregationResult[key] as IDataObject, fieldsToSplitBy.slice(1), {
-					...previousStage,
-					[splitFieldName]: getOriginalFieldValue(key),
-				}),
-			);
-		}
-		return returnData;
-	}
+	return [result];
 }