From e6cff3fce4de1644b75b07318fe561f721aee51c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= <milorad@n8n.io>
Date: Thu, 17 Aug 2023 14:22:28 +0200
Subject: [PATCH] fix(editor): Fix Remove all fields not removing values in
 resource mapper (#6940)

Github issue / Community forum post (link here to close automatically):
---
 .../N8nActionToggle/ActionToggle.vue          |  1 +
 .../components/N8nInputLabel/InputLabel.vue   |  2 +-
 .../src/components/ParameterInputWrapper.vue  |  2 +-
 .../ResourceMapper/MappingFields.vue          |  5 +-
 .../ResourceMapper/MatchingColumnsSelect.vue  | 45 +++++++++++++++
 .../ResourceMapper/ResourceMapper.vue         | 56 +++++++++++--------
 .../__tests__/ResourceMapper.test.ts          | 55 +++++++++++++++++-
 .../__tests__/utils/ResourceMapper.utils.ts   | 23 ++++----
 8 files changed, 149 insertions(+), 40 deletions(-)

diff --git a/packages/design-system/src/components/N8nActionToggle/ActionToggle.vue b/packages/design-system/src/components/N8nActionToggle/ActionToggle.vue
index 4b92dd4d74..f555db6eb1 100644
--- a/packages/design-system/src/components/N8nActionToggle/ActionToggle.vue
+++ b/packages/design-system/src/components/N8nActionToggle/ActionToggle.vue
@@ -21,6 +21,7 @@
 						:key="action.value"
 						:command="action.value"
 						:disabled="action.disabled"
+						:data-test-id="`action-${action.value}`"
 					>
 						{{ action.label }}
 						<div :class="$style.iconContainer">
diff --git a/packages/design-system/src/components/N8nInputLabel/InputLabel.vue b/packages/design-system/src/components/N8nInputLabel/InputLabel.vue
index 38c340380f..f029f16722 100644
--- a/packages/design-system/src/components/N8nInputLabel/InputLabel.vue
+++ b/packages/design-system/src/components/N8nInputLabel/InputLabel.vue
@@ -36,7 +36,7 @@
 			<div
 				v-if="$slots.options"
 				:class="{ [$style.options]: true, [$style.visible]: showOptions }"
-				data-test-id="parameter-input-options-container"
+				:data-test-id="`${inputName}-parameter-input-options-container`"
 			>
 				<slot name="options" />
 			</div>
diff --git a/packages/editor-ui/src/components/ParameterInputWrapper.vue b/packages/editor-ui/src/components/ParameterInputWrapper.vue
index a0246a7fc6..3d4e86975f 100644
--- a/packages/editor-ui/src/components/ParameterInputWrapper.vue
+++ b/packages/editor-ui/src/components/ParameterInputWrapper.vue
@@ -1,5 +1,5 @@
 <template>
-	<div>
+	<div data-test-id="parameter-input">
 		<parameter-input
 			ref="param"
 			:inputSize="inputSize"
diff --git a/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue b/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue
index 66b6d99cda..168b7cafed 100644
--- a/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue
+++ b/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue
@@ -10,8 +10,8 @@ import type {
 	ResourceMapperValue,
 } from 'n8n-workflow';
 import ParameterInputFull from '@/components/ParameterInputFull.vue';
-import ParameterIssues from '../ParameterIssues.vue';
-import ParameterOptions from '../ParameterOptions.vue';
+import ParameterIssues from '@/components//ParameterIssues.vue';
+import ParameterOptions from '@/components//ParameterOptions.vue';
 import { computed } from 'vue';
 import { i18n as locale } from '@/plugins/i18n';
 import { useNDVStore } from '@/stores';
@@ -276,6 +276,7 @@ defineExpose({
 			:size="labelSize"
 			:showOptions="true"
 			:showExpressionSelector="false"
+			inputName="columns"
 			color="text-dark"
 		>
 			<template #options>
diff --git a/packages/editor-ui/src/components/ResourceMapper/MatchingColumnsSelect.vue b/packages/editor-ui/src/components/ResourceMapper/MatchingColumnsSelect.vue
index 198524e088..5b84683a36 100644
--- a/packages/editor-ui/src/components/ResourceMapper/MatchingColumnsSelect.vue
+++ b/packages/editor-ui/src/components/ResourceMapper/MatchingColumnsSelect.vue
@@ -1,5 +1,6 @@
 <script setup lang="ts">
 import type {
+	INodeProperties,
 	INodePropertyTypeOptions,
 	ResourceMapperField,
 	ResourceMapperFields,
@@ -7,8 +8,10 @@ import type {
 import { computed, reactive, watch } from 'vue';
 import { i18n as locale } from '@/plugins/i18n';
 import { useNodeSpecificationValues } from '@/composables';
+import ParameterOptions from '@/components/ParameterOptions.vue';
 
 interface Props {
+	parameter: INodeProperties;
 	initialValue: string[];
 	fieldsToMap: ResourceMapperFields['fields'];
 	typeOptions: INodePropertyTypeOptions | undefined;
@@ -17,6 +20,7 @@ interface Props {
 	loading: boolean;
 	serviceName: string;
 	teleported?: boolean;
+	refreshInProgress: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -47,6 +51,7 @@ watch(
 
 const emit = defineEmits<{
 	(event: 'matchingColumnsChanged', value: string[]): void;
+	(event: 'refreshFieldList'): void;
 }>();
 
 const availableMatchingFields = computed<ResourceMapperField[]>(() => {
@@ -103,6 +108,27 @@ const fieldTooltip = computed<string>(() => {
 	});
 });
 
+const parameterActions = computed<Array<{ label: string; value: string; disabled?: boolean }>>(
+	() => {
+		return [
+			{
+				label: locale.baseText('resourceMapper.refreshFieldList', {
+					interpolate: { fieldWord: singularFieldWordCapitalized.value },
+				}),
+				value: 'refreshFieldList',
+			},
+		];
+	},
+);
+
+const fetchingFieldsLabel = computed<string>(() => {
+	return locale.baseText('resourceMapper.fetchingFields.message', {
+		interpolate: {
+			fieldWord: pluralFieldWord.value,
+		},
+	});
+});
+
 function onSelectionChange(value: string | string[]) {
 	if (resourceMapperTypeOptions.value?.multiKeyMatch === true) {
 		state.selected = value as string[];
@@ -121,6 +147,16 @@ function emitValueChanged() {
 	}
 }
 
+function onParameterActionSelected(action: string): void {
+	switch (action) {
+		case 'refreshFieldList':
+			emit('refreshFieldList');
+			break;
+		default:
+			break;
+	}
+}
+
 defineExpose({
 	state,
 });
@@ -137,6 +173,15 @@ defineExpose({
 			:size="labelSize"
 			color="text-dark"
 		>
+			<template #options>
+				<parameter-options
+					:parameter="parameter"
+					:customActions="parameterActions"
+					:loading="props.refreshInProgress"
+					:loadingMessage="fetchingFieldsLabel"
+					@update:modelValue="onParameterActionSelected"
+				/>
+			</template>
 			<n8n-select
 				:multiple="resourceMapperTypeOptions?.multiKeyMatch === true"
 				:modelValue="state.selected"
diff --git a/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue b/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue
index 4ab6ebeae2..b4a7967ca4 100644
--- a/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue
+++ b/packages/editor-ui/src/components/ResourceMapper/ResourceMapper.vue
@@ -348,18 +348,40 @@ function removeField(name: string): void {
 	}
 	const fieldName = parseResourceMapperFieldName(name);
 	if (fieldName) {
-		if (state.paramValue.value) {
-			delete state.paramValue.value[fieldName];
-			const field = state.paramValue.schema.find((f) => f.id === fieldName);
-			if (field) {
-				field.removed = true;
-				state.paramValue.schema.splice(state.paramValue.schema.indexOf(field), 1, field);
-			}
+		const field = state.paramValue.schema.find((f) => f.id === fieldName);
+		if (field) {
+			deleteField(field);
 			emitValueChanged();
 		}
 	}
 }
 
+function removeAllFields(): void {
+	state.paramValue.schema.forEach((field) => {
+		if (
+			!fieldCannotBeDeleted(
+				field,
+				showMatchingColumnsSelector.value,
+				resourceMapperMode.value,
+				matchingColumns.value,
+			)
+		) {
+			deleteField(field);
+		}
+	});
+	emitValueChanged();
+}
+
+// Delete a single field from the mapping (set removed flag to true and delete from value)
+// Used when removing one or all fields
+function deleteField(field: ResourceMapperField): void {
+	if (state.paramValue.value) {
+		delete state.paramValue.value[field.id];
+		field.removed = true;
+		state.paramValue.schema.splice(state.paramValue.schema.indexOf(field), 1, field);
+	}
+}
+
 function addField(name: string): void {
 	if (name === 'addAllFields') {
 		return addAllFields();
@@ -395,23 +417,6 @@ function addAllFields(): void {
 	emitValueChanged();
 }
 
-function removeAllFields(): void {
-	state.paramValue.schema.forEach((field) => {
-		if (
-			!fieldCannotBeDeleted(
-				field,
-				showMatchingColumnsSelector.value,
-				resourceMapperMode.value,
-				matchingColumns.value,
-			)
-		) {
-			field.removed = true;
-			state.paramValue.schema.splice(state.paramValue.schema.indexOf(field), 1, field);
-		}
-	});
-	emitValueChanged();
-}
-
 function emitValueChanged(): void {
 	pruneParamValues();
 	emit('valueChanged', {
@@ -457,6 +462,7 @@ defineExpose({
 		/>
 		<matching-columns-select
 			v-if="showMatchingColumnsSelector"
+			:parameter="props.parameter"
 			:label-size="labelSize"
 			:fieldsToMap="state.paramValue.schema"
 			:typeOptions="props.parameter.typeOptions"
@@ -465,7 +471,9 @@ defineExpose({
 			:initialValue="matchingColumns"
 			:serviceName="nodeType?.displayName || locale.baseText('generic.service')"
 			:teleported="teleported"
+			:refreshInProgress="state.refreshInProgress"
 			@matchingColumnsChanged="onMatchingColumnsChanged"
+			@refreshFieldList="initFetching(true)"
 		/>
 		<n8n-text v-if="!showMappingModeSelect && state.loading" size="small">
 			<n8n-icon icon="sync-alt" size="xsmall" :spin="true" />
diff --git a/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts b/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts
index f1e487e3ef..e3ee648e73 100644
--- a/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts
+++ b/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts
@@ -3,6 +3,7 @@ import {
 	MAPPING_COLUMNS_RESPONSE,
 	NODE_PARAMETER_VALUES,
 	UPDATED_SCHEMA,
+	getLatestValueChangeEvent,
 } from './utils/ResourceMapper.utils';
 import { useNodeTypesStore } from '@/stores/nodeTypes.store';
 import { waitAllPromises } from '@/__tests__/utils';
@@ -13,7 +14,8 @@ import { createComponentRenderer } from '@/__tests__/render';
 import type { SpyInstance } from 'vitest';
 
 let nodeTypeStore: ReturnType<typeof useNodeTypesStore>;
-let fetchFieldsSpy: SpyInstance, resolveParameterSpy: SpyInstance;
+let fetchFieldsSpy: SpyInstance;
+let resolveParameterSpy: SpyInstance;
 
 const renderComponent = createComponentRenderer(ResourceMapper, DEFAULT_SETUP);
 
@@ -289,4 +291,55 @@ describe('ResourceMapper.vue', () => {
 		await waitAllPromises();
 		expect(fetchFieldsSpy).not.toHaveBeenCalled();
 	});
+
+	it('should delete fields from UI and parameter value when they are deleted', async () => {
+		const { getByTestId, emitted } = renderComponent({
+			props: {
+				node: {
+					parameters: {
+						columns: {
+							schema: null,
+						},
+					},
+				},
+			},
+		});
+		await waitAllPromises();
+		// Add some values so we can test if they are gone after deletion
+		const idInput = getByTestId('parameter-input-value["id"]').querySelector('input');
+		const firstNameInput = getByTestId('parameter-input-value["First name"]').querySelector(
+			'input',
+		);
+		const lastNameInput = getByTestId('parameter-input-value["Last name"]').querySelector('input');
+		const usernameInput = getByTestId('parameter-input-value["Username"]').querySelector('input');
+		const addressInput = getByTestId('parameter-input-value["Address"]').querySelector('input');
+		if (idInput && firstNameInput && lastNameInput && usernameInput && addressInput) {
+			await userEvent.type(idInput, '123');
+			await userEvent.type(firstNameInput, 'John');
+			await userEvent.type(lastNameInput, 'Doe');
+			await userEvent.type(usernameInput, 'johndoe');
+			await userEvent.type(addressInput, '123 Main St');
+			// All field values should be in parameter value
+			const valueBeforeRemove = getLatestValueChangeEvent(emitted());
+			expect(valueBeforeRemove[0].value.value).toHaveProperty('id');
+			expect(valueBeforeRemove[0].value.value).toHaveProperty('First name');
+			expect(valueBeforeRemove[0].value.value).toHaveProperty('Last name');
+			expect(valueBeforeRemove[0].value.value).toHaveProperty('Username');
+			expect(valueBeforeRemove[0].value.value).toHaveProperty('Address');
+			// Click on 'Remove all fields' option
+			await userEvent.click(getByTestId('columns-parameter-input-options-container'));
+			await userEvent.click(getByTestId('action-removeAllFields'));
+			// Should delete all non-mandatory fields:
+			// 1. From UI
+			expect(
+				getByTestId('resource-mapper-container').querySelectorAll('.parameter-item').length,
+			).toBe(3);
+			// 2. And their values from parameter value
+			const valueAfterRemove = getLatestValueChangeEvent(emitted());
+			expect(valueAfterRemove[0].value.value).not.toHaveProperty('Username');
+			expect(valueAfterRemove[0].value.value).not.toHaveProperty('Address');
+		} else {
+			throw new Error('Could not find input fields');
+		}
+	});
 });
diff --git a/packages/editor-ui/src/components/__tests__/utils/ResourceMapper.utils.ts b/packages/editor-ui/src/components/__tests__/utils/ResourceMapper.utils.ts
index 0a220f75c6..3e2fe5b33e 100644
--- a/packages/editor-ui/src/components/__tests__/utils/ResourceMapper.utils.ts
+++ b/packages/editor-ui/src/components/__tests__/utils/ResourceMapper.utils.ts
@@ -2,7 +2,7 @@ import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils';
 import { STORES } from '@/constants';
 import { createTestingPinia } from '@pinia/testing';
 import { merge } from 'lodash-es';
-import type { ResourceMapperFields } from 'n8n-workflow';
+import type { ResourceMapperFields, ResourceMapperValue } from 'n8n-workflow';
 
 export const NODE_PARAMETER_VALUES = {
 	authentication: 'oAuth2',
@@ -201,15 +201,16 @@ export const MAPPING_COLUMNS_RESPONSE: ResourceMapperFields = {
 			type: 'string',
 			canBeUsedToMatch: true,
 		},
-		{
-			id: 'Last Name',
-			displayName: 'Last Name',
-			match: false,
-			required: false,
-			defaultMatch: false,
-			display: true,
-			type: 'string',
-			canBeUsedToMatch: true,
-		},
 	],
 };
+
+// Gets the latest `valueChanged` event emitted by the component
+// This will be used to inspect current resource mapper value set in node parameters
+export function getLatestValueChangeEvent(emitted: Record<string, unknown[]>) {
+	const valueChangeEmits = emitted.valueChanged as [];
+	return valueChangeEmits[valueChangeEmits.length - 1] as Array<{
+		name: string;
+		value: ResourceMapperValue;
+		node: string;
+	}>;
+}