fix(editor): Fix Remove all fields not removing values in resource mapper (#6940)

Github issue / Community forum post (link here to close automatically):
This commit is contained in:
Milorad FIlipović 2023-08-17 14:22:28 +02:00 committed by GitHub
parent b312f2ee54
commit e6cff3fce4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 149 additions and 40 deletions

View file

@ -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">

View file

@ -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>

View file

@ -1,5 +1,5 @@
<template>
<div>
<div data-test-id="parameter-input">
<parameter-input
ref="param"
:inputSize="inputSize"

View file

@ -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>

View file

@ -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"

View file

@ -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" />

View file

@ -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');
}
});
});

View file

@ -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;
}>;
}