fix(editor): Improve dragndrop of input pills with spaces (#9656)

packages/editor-ui/src/components/AssignmentCollection/AssignmentCollection.vue

@@ -13,7 +13,8 @@ import { computed, reactive, watch } from 'vue';
 import DropArea from '../DropArea/DropArea.vue';
 import ParameterOptions from '../ParameterOptions.vue';
 import Assignment from './Assignment.vue';
-import { inputDataToAssignments, nameFromExpression, typeFromExpression } from './utils';
+import { inputDataToAssignments, typeFromExpression } from './utils';
+import { propertyNameFromExpression } from '@/utils/mappingUtils';
 
 interface Props {
 	parameter: INodeProperties;
@@ -49,7 +50,7 @@ const issues = computed(() => {
 });
 
 const empty = computed(() => state.paramValue.assignments.length === 0);
-const activeDragField = computed(() => nameFromExpression(ndvStore.draggableData));
+const activeDragField = computed(() => propertyNameFromExpression(ndvStore.draggableData));
 const inputData = computed(() => ndvStore.ndvInputData?.[0]?.json);
 const actions = computed(() => {
 	return [
@@ -82,7 +83,7 @@ function addAssignment(): void {
 function dropAssignment(expression: string): void {
 	state.paramValue.assignments.push({
 		id: uuid(),
-		name: nameFromExpression(expression),
+		name: propertyNameFromExpression(expression),
 		value: `=${expression}`,
 		type: typeFromExpression(expression),
 	});

packages/editor-ui/src/components/AssignmentCollection/__tests__/utils.test.ts

@@ -1,15 +0,0 @@
-import { nameFromExpression } from '../utils';
-
-describe('AssignmentCollection > utils', () => {
-	describe('nameFromExpression', () => {
-		test('should extract assignment name from previous node', () => {
-			expect(nameFromExpression('{{ $json.foo.bar }}')).toBe('foo.bar');
-		});
-
-		test('should extract assignment name from another node', () => {
-			expect(nameFromExpression("{{ $('Node's \"Name\" (copy)').item.json.foo.bar }}")).toBe(
-				'foo.bar',
-			);
-		});
-	});
-});

packages/editor-ui/src/components/AssignmentCollection/utils.ts

@@ -3,13 +3,6 @@ import type { AssignmentValue, IDataObject } from 'n8n-workflow';
 import { resolveParameter } from '@/composables/useWorkflowHelpers';
 import { v4 as uuid } from 'uuid';
 
-export function nameFromExpression(expression: string): string {
-	return expression
-		.replace(/^{{\s*|\s*}}$/g, '')
-		.replace('$json.', '')
-		.replace(/^\$\(.*\)(\.item\.json)?\.(.*)/, '$2');
-}
-
 export function inferAssignmentType(value: unknown): string {
 	if (typeof value === 'boolean') return 'boolean';
 	if (typeof value === 'number') return 'number';

packages/editor-ui/src/utils/__tests__/mappingUtils.test.ts

@@ -1,5 +1,10 @@
 import type { INodeProperties } from 'n8n-workflow';
-import { getMappedResult, getMappedExpression, escapeMappingString } from '../mappingUtils';
+import {
+	getMappedResult,
+	getMappedExpression,
+	escapeMappingString,
+	propertyNameFromExpression,
+} from '../mappingUtils';
 
 const RLC_PARAM: INodeProperties = {
 	displayName: 'Base',
@@ -146,7 +151,7 @@ describe('Mapping Utils', () => {
 		it('sets data path, replacing if expecting single path', () => {
 			expect(
 				getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json["Readable date"] }}', '={{$json.test}}'),
-			).toEqual('["Readable date"]');
+			).toEqual('Readable date');
 
 			expect(
 				getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json.path }}', '={{$json.test}}'),
@@ -159,18 +164,26 @@ describe('Mapping Utils', () => {
 			).toEqual('path, ["Readable date"]');
 		});
 
-		it('replaces existing dadata path if multiple and is empty expression', () => {
+		it('replaces existing data path if multiple and is empty expression', () => {
 			expect(getMappedResult(MULTIPLE_DATA_PATH_PARAM, '{{ $json.test }}', '=')).toEqual('test');
 		});
 
-		it('handles data when dragging from grand-parent nodes', () => {
+		it('handles data when dragging from grand-parent nodes, replacing if expecting single path', () => {
 			expect(
 				getMappedResult(
 					MULTIPLE_DATA_PATH_PARAM,
 					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
 					'',
 				),
-			).toEqual('={{ $node["Schedule Trigger"].json["Day of week"] }}');
+			).toEqual('["Day of week"]');
+
+			expect(
+				getMappedResult(
+					MULTIPLE_DATA_PATH_PARAM,
+					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
+					'=data',
+				),
+			).toEqual('=data, ["Day of week"]');
 
 			expect(
 				getMappedResult(
@@ -178,7 +191,7 @@ describe('Mapping Utils', () => {
 					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
 					'=data',
 				),
-			).toEqual('=data {{ $node["Schedule Trigger"].json["Day of week"] }}');
+			).toEqual('Day of week');
 
 			expect(
 				getMappedResult(
@@ -186,7 +199,7 @@ describe('Mapping Utils', () => {
 					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
 					'=   ',
 				),
-			).toEqual('=    {{ $node["Schedule Trigger"].json["Day of week"] }}');
+			).toEqual('Day of week');
 		});
 
 		it('handles RLC values', () => {
@@ -195,6 +208,7 @@ describe('Mapping Utils', () => {
 			expect(getMappedResult(RLC_PARAM, '{{ test }}', '=test')).toEqual('=test {{ test }}');
 		});
 	});
+
 	describe('getMappedExpression', () => {
 		it('should generate a mapped expression with simple array path', () => {
 			const input = {
@@ -274,6 +288,68 @@ describe('Mapping Utils', () => {
 			);
 		});
 	});
+
+	describe('propertyNameFromExpression', () => {
+		describe('dot access', () => {
+			test('should extract property name from previous node', () => {
+				expect(propertyNameFromExpression('{{ $json.foo.bar }}')).toBe('foo.bar');
+			});
+
+			test('should extract property name from another node', () => {
+				expect(
+					propertyNameFromExpression("{{ $('Node's \"Name\" (copy)').item.json.foo.bar }}"),
+				).toBe('foo.bar');
+			});
+		});
+
+		describe('bracket access', () => {
+			test('should extract property name from previous node (root)', () => {
+				expect(propertyNameFromExpression("{{ $json['with spaces\\' here'] }}")).toBe(
+					"with spaces' here",
+				);
+			});
+
+			test('should extract property name from previous node (nested)', () => {
+				expect(propertyNameFromExpression("{{ $json.foo['with spaces\\' here'] }}")).toBe(
+					"foo['with spaces\\' here']",
+				);
+			});
+
+			test('should extract property name from another node (root)', () => {
+				expect(
+					propertyNameFromExpression(
+						"{{ $('Node's \"Name\" (copy)').item.json['with spaces\\' here'] }}",
+					),
+				).toBe("with spaces' here");
+			});
+
+			test('should extract property name from another node (nested)', () => {
+				expect(
+					propertyNameFromExpression(
+						"{{ $('Node's \"Name\" (copy)').item.json.foo['with spaces\\' here'] }}",
+					),
+				).toBe("foo['with spaces\\' here']");
+			});
+
+			test('should handle nested bracket access', () => {
+				expect(
+					propertyNameFromExpression(
+						"{{ $('Node's \"Name\" (copy)').item.json['First with spaces']['Second with spaces'] }}",
+					),
+				).toBe("['First with spaces']['Second with spaces']");
+			});
+
+			test('should handle forceBracketAccess=true', () => {
+				expect(
+					propertyNameFromExpression(
+						"{{ $('Node's \"Name\" (copy)').item.json['First with spaces'] }}",
+						true,
+					),
+				).toBe("['First with spaces']");
+			});
+		});
+	});
+
 	describe('escapeMappingString', () => {
 		test.each([
 			{ input: 'Normal node name (here)', output: 'Normal node name (here)' },

packages/editor-ui/src/utils/mappingUtils.ts

@@ -1,5 +1,6 @@
 import type { INodeProperties, NodeParameterValueType } from 'n8n-workflow';
 import { isResourceLocatorValue } from 'n8n-workflow';
+import { isExpression } from './expressions';
 
 const validJsIdNameRegex = /^[a-zA-Z_$][a-zA-Z0-9_$]*$/;
 
@@ -46,34 +47,54 @@ export function getMappedExpression({
 	return `{{ ${generatePath(root, path)} }}`;
 }
 
+const unquote = (str: string) => {
+	if (str.startsWith('"') && str.endsWith('"')) {
+		return str.slice(1, -1).replace(/\\"/g, '"');
+	}
+
+	if (str.startsWith("'") && str.endsWith("'")) {
+		return str.slice(1, -1).replace(/\\'/g, "'");
+	}
+
+	return str;
+};
+
+export function propertyNameFromExpression(expression: string, forceBracketAccess = false): string {
+	const propPath = expression
+		.replace(/^{{\s*|\s*}}$/g, '')
+		.replace(/^(\$\(.*\)\.item\.json|\$json|\$node\[.*\]\.json)\.?(.*)/, '$2');
+
+	const isSingleBracketAccess = propPath.startsWith('[') && !propPath.slice(1).includes('[');
+	if (isSingleBracketAccess && !forceBracketAccess) {
+		// "['Key with spaces']" -> "Key with spaces"
+		return unquote(propPath.slice(1, -1));
+	}
+
+	return propPath;
+}
+
 export function getMappedResult(
 	parameter: INodeProperties,
 	newParamValue: string,
 	prevParamValue: NodeParameterValueType,
 ): string {
-	const useDataPath = !!parameter.requiresDataPath && newParamValue.startsWith('{{ $json'); // ignore when mapping from grand-parent-node
 	const prevValue =
 		parameter.type === 'resourceLocator' && isResourceLocatorValue(prevParamValue)
 			? prevParamValue.value
 			: prevParamValue;
 
-	if (useDataPath) {
-		const newValue = newParamValue
-			.replace('{{ $json', '')
-			.replace(new RegExp('^\\.'), '')
-			.replace(new RegExp('}}$'), '')
-			.trim();
-
-		if (prevValue && parameter.requiresDataPath === 'multiple') {
-			if (typeof prevValue === 'string' && prevValue.trim() === '=') {
-				return newValue;
-			} else {
-				return `${prevValue}, ${newValue}`;
+	if (parameter.requiresDataPath) {
+		if (parameter.requiresDataPath === 'multiple') {
+			const propertyName = propertyNameFromExpression(newParamValue, true);
+			if (typeof prevValue === 'string' && (prevValue.trim() === '=' || prevValue.trim() === '')) {
+				return propertyName;
 			}
-		} else {
-			return newValue;
+
+			return `${prevValue}, ${propertyName}`;
 		}
-	} else if (typeof prevValue === 'string' && prevValue.startsWith('=') && prevValue.length > 1) {
+
+		return propertyNameFromExpression(newParamValue);
+	} else if (typeof prevValue === 'string' && isExpression(prevValue) && prevValue.length > 1) {
 		return `${prevValue} ${newParamValue}`;
 	} else if (prevValue && ['string', 'json'].includes(parameter.type)) {
 		return prevValue === '=' ? `=${newParamValue}` : `=${prevValue} ${newParamValue}`;