fix: Fix mapping paths when appending to empty expression (#5591)

* fix: Fix mapping when appending to empty expression

* fix: refactor logic out

* test: add tests

* test: add tests

* fix: fix bug where value does not get updated when mapping

* test: add test for bug

* test: add test for bug

cypress/e2e/14-mapping.cy.ts

@@ -262,4 +262,34 @@ describe('Data mapping', () => {
 			.find('input')
 			.should('have.value', 'input[0]["hello.world"]["my count"]');
 	});
+
+	it('maps expressions to updated fields correctly', () => {
+		cy.fixture('Test_workflow_3.json').then((data) => {
+			cy.get('body').paste(JSON.stringify(data));
+		});
+
+		workflowPage.actions.openNode('Set');
+
+		ndv.actions.typeIntoParameterInput('value', 'delete me');
+		ndv.actions.dismissMappingTooltip();
+
+		ndv.actions.typeIntoParameterInput('name', 'test');
+
+		ndv.actions.typeIntoParameterInput('value', 'fun');
+		ndv.actions.clearParameterInput('value'); // keep focus on param
+
+		ndv.getters.inputDataContainer().should('exist').find('span').contains('count').realMouseDown();
+
+		ndv.actions.mapToParameter('value');
+		ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}');
+		ndv.getters.parameterExpressionPreview('value').should('include.text', '0');
+
+		ndv.getters.inputDataContainer().find('span').contains('input').realMouseDown();
+
+		ndv.actions.mapToParameter('value');
+		ndv.getters
+			.inlineExpressionEditorInput()
+			.should('have.text', '{{ $json.input[0].count }} {{ $json.input }}');
+		ndv.getters.parameterExpressionPreview('value').should('include.text', '0 [object Object]');
+	});
 });

cypress/pages/ndv.ts

@@ -86,6 +86,9 @@ export class NDV extends BasePage {
 		selectOptionInParameterDropdown: (parameterName: string, content: string) => {
 			this.getters.parameterInput(parameterName).find('.option-headline').contains(content).click();
 		},
+		dismissMappingTooltip: () => {
+			cy.getByTestId('dismiss-mapping-tooltip').click();
+		},
 		rename: (newName: string) => {
 			this.getters.nodeNameContainer().click();
 			this.getters.nodeRenameInput().should('be.visible').type('{selectall}').type(newName);

packages/design-system/src/types/button.ts

@@ -12,6 +12,8 @@ export type IN8nButton = {
 		active?: boolean;
 		float?: 'left' | 'right';
 		square?: boolean;
+		// eslint-disable-next-line @typescript-eslint/naming-convention
+		'data-test-id'?: string;
 	};
 	listeners?: Record<string, (event: Event) => void>;
 };

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

@@ -103,6 +103,12 @@ export default Vue.extend({
 
 			window.addEventListener('mousemove', this.onDrag);
 			window.addEventListener('mouseup', this.onDragEnd);
+
+			// blur so that any focused inputs update value
+			const activeElement = document.activeElement as HTMLElement;
+			if (activeElement) {
+				activeElement.blur();
+			}
 		},
 		onDrag(e: MouseEvent) {
 			e.preventDefault();

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

@@ -90,6 +90,7 @@ import { mapStores } from 'pinia';
 import { useNDVStore } from '@/stores/ndv';
 import { useSegment } from '@/stores/segment';
 import { externalHooks } from '@/mixins/externalHooks';
+import { getMappedResult } from '../utils/mappingUtils';
 
 export default mixins(showMessage, externalHooks).extend({
 	name: 'parameter-input-full',
@@ -146,6 +147,7 @@ export default mixins(showMessage, externalHooks).extend({
 			{
 				attrs: {
 					label: this.$locale.baseText('_reusableBaseText.dismiss' as BaseTextKey),
+					'data-test-id': 'dismiss-mapping-tooltip',
 				},
 				listeners: {
 					click: mappingTooltipDismissHandler,
@@ -228,39 +230,15 @@ export default mixins(showMessage, externalHooks).extend({
 				param?.$emit('optionSelected', 'addExpression');
 			}
 		},
-		onDrop(data: string) {
-			const useDataPath = !!this.parameter.requiresDataPath && data.startsWith('{{ $json');
-			if (!useDataPath) {
+		onDrop(newParamValue: string) {
+			const updatedValue = getMappedResult(this.parameter, newParamValue, this.value);
+			const prevValue = this.isResourceLocator ? this.value.value : this.value;
+
+			if (updatedValue.startsWith('=')) {
 				this.forceShowExpression = true;
 			}
 			setTimeout(() => {
 				if (this.node) {
-					const prevValue = this.isResourceLocator ? this.value.value : this.value;
-					let updatedValue: string;
-					if (useDataPath) {
-						const newValue = data
-							.replace('{{ $json', '')
-							.replace(new RegExp('^\\.'), '')
-							.replace(new RegExp('}}$'), '')
-							.trim();
-
-						if (prevValue && this.parameter.requiresDataPath === 'multiple') {
-							updatedValue = `${prevValue}, ${newValue}`;
-						} else {
-							updatedValue = newValue;
-						}
-					} else if (
-						typeof prevValue === 'string' &&
-						prevValue.startsWith('=') &&
-						prevValue.length > 1
-					) {
-						updatedValue = `${prevValue} ${data}`;
-					} else if (prevValue && ['string', 'json'].includes(this.parameter.type)) {
-						updatedValue = prevValue === '=' ? `=${data}` : `=${prevValue} ${data}`;
-					} else {
-						updatedValue = `=${data}`;
-					}
-
 					let parameterData;
 					if (this.isResourceLocator) {
 						if (!isResourceLocatorValue(this.value)) {
@@ -334,7 +312,7 @@ export default mixins(showMessage, externalHooks).extend({
 			}, 200);
 		},
 		onMappingTooltipDismissed() {
-			this.localStorageMappingFlag = true;
+			this.ndvStore.disableMappingHint(false);
 		},
 	},
 	watch: {

packages/editor-ui/src/stores/ndv.ts

@@ -185,9 +185,11 @@ export const useNDVStore = defineStore(STORES.NDV, {
 		setNDVPanelDataIsEmpty(payload: { panel: 'input' | 'output'; isEmpty: boolean }): void {
 			Vue.set(this[payload.panel].data, 'isEmpty', payload.isEmpty);
 		},
-		disableMappingHint() {
+		disableMappingHint(store = true) {
 			this.isMappingOnboarded = true;
-			window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
+			if (store) {
+				window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
+			}
 		},
 	},
 });

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

@@ -0,0 +1,202 @@
+import { INodeProperties } from 'n8n-workflow';
+import { getMappedResult } from '../mappingUtils';
+
+const RLC_PARAM: INodeProperties = {
+	displayName: 'Base',
+	name: 'application',
+	type: 'resourceLocator',
+	default: {
+		mode: 'url',
+		value: '',
+	},
+	required: true,
+	description: 'The Airtable Base in which to operate on',
+	modes: [
+		{
+			displayName: 'ID',
+			name: 'id',
+			type: 'string',
+			validation: [
+				{
+					type: 'regex',
+					properties: {
+						regex: '[a-zA-Z0-9]{2,}',
+						errorMessage: 'Not a valid Airtable Base ID',
+					},
+				},
+			],
+			placeholder: 'appD3dfaeidke',
+			url: '=https://airtable.com/{{$value}}',
+		},
+	],
+};
+
+const STRING_PARAM: INodeProperties = {
+	displayName: 'Value',
+	name: 'value',
+	type: 'string',
+	default: '',
+	description: 'The string value to write in the property',
+};
+
+const SINGLE_DATA_PATH_PARAM: INodeProperties = {
+	displayName: 'Name',
+	name: 'name',
+	type: 'string',
+	default: 'propertyName',
+	requiresDataPath: 'single',
+	description:
+		'Name of the property to write data to. Supports dot-notation. Example: "data.person[0].name"',
+};
+
+const MULTIPLE_DATA_PATH_PARAM: INodeProperties = {
+	displayName: 'For Everything Except',
+	name: 'exceptWhenMix',
+	type: 'string',
+	default: '',
+	placeholder: 'e.g. id, country',
+	hint: 'Enter the names of the input fields as text, separated by commas',
+	displayOptions: {
+		show: {
+			resolve: ['mix'],
+		},
+	},
+	requiresDataPath: 'multiple',
+};
+
+const JSON_PARAM: INodeProperties = {
+	displayName: 'JSON Payload',
+	name: 'payloadJson',
+	type: 'json',
+	typeOptions: {
+		alwaysOpenEditWindow: true,
+		editor: 'code',
+	},
+	default: '',
+};
+
+const BOOLEAN_PARAM: INodeProperties = {
+	displayName: 'Value',
+	name: 'value',
+	type: 'boolean',
+	default: false,
+	description: 'The boolean value to write in the property',
+};
+
+const NUMBER_PARAM: INodeProperties = {
+	displayName: 'Value',
+	name: 'value',
+	type: 'number',
+	default: 0,
+	description: 'The number value to write in the property',
+};
+
+describe('Mapping Utils', () => {
+	describe('getMappedResult', () => {
+		it('turns empty string into expression', () => {
+			expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '')).toEqual(
+				'={{ $json["Readable date"] }}',
+			);
+		});
+
+		it('keeps spaces when mapping data to fixed value', () => {
+			expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '  ')).toEqual(
+				'=   {{ $json["Readable date"] }}',
+			);
+		});
+
+		it('sets expression when mapping to an empty expression', () => {
+			expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '=')).toEqual(
+				'={{ $json["Readable date"] }}',
+			);
+		});
+
+		it('keeps spaces when mapping data to expression value', () => {
+			expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '=  ')).toEqual(
+				'=   {{ $json["Readable date"] }}',
+			);
+		});
+
+		it('appends to string fixed value and turns into expression', () => {
+			expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', 'test')).toEqual(
+				'=test {{ $json["Readable date"] }}',
+			);
+		});
+
+		it('appends to json fixed value', () => {
+			expect(getMappedResult(JSON_PARAM, '{{ $json["Readable date"] }}', 'test')).toEqual(
+				'=test {{ $json["Readable date"] }}',
+			);
+		});
+
+		it('replaces number value with expression', () => {
+			expect(getMappedResult(NUMBER_PARAM, '{{ $json["Readable date"] }}', 0)).toEqual(
+				'={{ $json["Readable date"] }}',
+			);
+		});
+
+		it('replaces boolean value with expression', () => {
+			expect(getMappedResult(BOOLEAN_PARAM, '{{ $json["Readable date"] }}', false)).toEqual(
+				'={{ $json["Readable date"] }}',
+			);
+		});
+
+		it('appends existing expression value', () => {
+			expect(
+				getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '={{$json.test}}'),
+			).toEqual('={{$json.test}} {{ $json["Readable date"] }}');
+		});
+
+		it('sets data path, replacing if expecting single path', () => {
+			expect(
+				getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json["Readable date"] }}', '={{$json.test}}'),
+			).toEqual('["Readable date"]');
+
+			expect(
+				getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json.path }}', '={{$json.test}}'),
+			).toEqual('path');
+		});
+
+		it('appends to existing data path, if multiple', () => {
+			expect(
+				getMappedResult(MULTIPLE_DATA_PATH_PARAM, '{{ $json["Readable date"] }}', 'path'),
+			).toEqual('path, ["Readable date"]');
+		});
+
+		it('replaces existing dadata 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', () => {
+			expect(
+				getMappedResult(
+					MULTIPLE_DATA_PATH_PARAM,
+					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
+					'',
+				),
+			).toEqual('={{ $node["Schedule Trigger"].json["Day of week"] }}');
+
+			expect(
+				getMappedResult(
+					SINGLE_DATA_PATH_PARAM,
+					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
+					'=data',
+				),
+			).toEqual('=data {{ $node["Schedule Trigger"].json["Day of week"] }}');
+
+			expect(
+				getMappedResult(
+					SINGLE_DATA_PATH_PARAM,
+					'{{ $node["Schedule Trigger"].json["Day of week"] }}',
+					'=   ',
+				),
+			).toEqual('=    {{ $node["Schedule Trigger"].json["Day of week"] }}');
+		});
+
+		it('handles RLC values', () => {
+			expect(getMappedResult(RLC_PARAM, '{{ test }}', '')).toEqual('={{ test }}');
+			expect(getMappedResult(RLC_PARAM, '{{ test }}', '=')).toEqual('={{ test }}');
+			expect(getMappedResult(RLC_PARAM, '{{ test }}', '=test')).toEqual('=test {{ test }}');
+		});
+	});
+});

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

@@ -2,7 +2,7 @@ import jp from 'jsonpath';
 import { isEmpty, intersection, mergeDeep, getSchema, isValidDate } from '@/utils';
 import { Schema } from '@/Interface';
 
-describe('Utils', () => {
+describe('Types Utils', () => {
 	describe('isEmpty', () => {
 		test.each([
 			[undefined, true],

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

@@ -1,3 +1,5 @@
+import { INodeProperties, isResourceLocatorValue, NodeParameterValueType } from 'n8n-workflow';
+
 export function generatePath(root: string, path: Array<string | number>): string {
 	return path.reduce((accu: string, part: string | number) => {
 		if (typeof part === 'number') {
@@ -29,3 +31,39 @@ export function getMappedExpression({
 
 	return `{{ ${generatePath(root, path)} }}`;
 }
+
+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}`;
+			}
+		} else {
+			return newValue;
+		}
+	} else if (typeof prevValue === 'string' && prevValue.startsWith('=') && prevValue.length > 1) {
+		return `${prevValue} ${newParamValue}`;
+	} else if (prevValue && ['string', 'json'].includes(parameter.type)) {
+		return prevValue === '=' ? `=${newParamValue}` : `=${prevValue} ${newParamValue}`;
+	}
+
+	return `=${newParamValue}`;
+}