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" :key="action.value"
:command="action.value" :command="action.value"
:disabled="action.disabled" :disabled="action.disabled"
:data-test-id="`action-${action.value}`"
> >
{{ action.label }} {{ action.label }}
<div :class="$style.iconContainer"> <div :class="$style.iconContainer">

View file

@ -36,7 +36,7 @@
<div <div
v-if="$slots.options" v-if="$slots.options"
:class="{ [$style.options]: true, [$style.visible]: showOptions }" :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" /> <slot name="options" />
</div> </div>

View file

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

View file

@ -10,8 +10,8 @@ import type {
ResourceMapperValue, ResourceMapperValue,
} from 'n8n-workflow'; } from 'n8n-workflow';
import ParameterInputFull from '@/components/ParameterInputFull.vue'; import ParameterInputFull from '@/components/ParameterInputFull.vue';
import ParameterIssues from '../ParameterIssues.vue'; import ParameterIssues from '@/components//ParameterIssues.vue';
import ParameterOptions from '../ParameterOptions.vue'; import ParameterOptions from '@/components//ParameterOptions.vue';
import { computed } from 'vue'; import { computed } from 'vue';
import { i18n as locale } from '@/plugins/i18n'; import { i18n as locale } from '@/plugins/i18n';
import { useNDVStore } from '@/stores'; import { useNDVStore } from '@/stores';
@ -276,6 +276,7 @@ defineExpose({
:size="labelSize" :size="labelSize"
:showOptions="true" :showOptions="true"
:showExpressionSelector="false" :showExpressionSelector="false"
inputName="columns"
color="text-dark" color="text-dark"
> >
<template #options> <template #options>

View file

@ -1,5 +1,6 @@
<script setup lang="ts"> <script setup lang="ts">
import type { import type {
INodeProperties,
INodePropertyTypeOptions, INodePropertyTypeOptions,
ResourceMapperField, ResourceMapperField,
ResourceMapperFields, ResourceMapperFields,
@ -7,8 +8,10 @@ import type {
import { computed, reactive, watch } from 'vue'; import { computed, reactive, watch } from 'vue';
import { i18n as locale } from '@/plugins/i18n'; import { i18n as locale } from '@/plugins/i18n';
import { useNodeSpecificationValues } from '@/composables'; import { useNodeSpecificationValues } from '@/composables';
import ParameterOptions from '@/components/ParameterOptions.vue';
interface Props { interface Props {
parameter: INodeProperties;
initialValue: string[]; initialValue: string[];
fieldsToMap: ResourceMapperFields['fields']; fieldsToMap: ResourceMapperFields['fields'];
typeOptions: INodePropertyTypeOptions | undefined; typeOptions: INodePropertyTypeOptions | undefined;
@ -17,6 +20,7 @@ interface Props {
loading: boolean; loading: boolean;
serviceName: string; serviceName: string;
teleported?: boolean; teleported?: boolean;
refreshInProgress: boolean;
} }
const props = withDefaults(defineProps<Props>(), { const props = withDefaults(defineProps<Props>(), {
@ -47,6 +51,7 @@ watch(
const emit = defineEmits<{ const emit = defineEmits<{
(event: 'matchingColumnsChanged', value: string[]): void; (event: 'matchingColumnsChanged', value: string[]): void;
(event: 'refreshFieldList'): void;
}>(); }>();
const availableMatchingFields = computed<ResourceMapperField[]>(() => { 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[]) { function onSelectionChange(value: string | string[]) {
if (resourceMapperTypeOptions.value?.multiKeyMatch === true) { if (resourceMapperTypeOptions.value?.multiKeyMatch === true) {
state.selected = value as string[]; 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({ defineExpose({
state, state,
}); });
@ -137,6 +173,15 @@ defineExpose({
:size="labelSize" :size="labelSize"
color="text-dark" color="text-dark"
> >
<template #options>
<parameter-options
:parameter="parameter"
:customActions="parameterActions"
:loading="props.refreshInProgress"
:loadingMessage="fetchingFieldsLabel"
@update:modelValue="onParameterActionSelected"
/>
</template>
<n8n-select <n8n-select
:multiple="resourceMapperTypeOptions?.multiKeyMatch === true" :multiple="resourceMapperTypeOptions?.multiKeyMatch === true"
:modelValue="state.selected" :modelValue="state.selected"

View file

@ -348,18 +348,40 @@ function removeField(name: string): void {
} }
const fieldName = parseResourceMapperFieldName(name); const fieldName = parseResourceMapperFieldName(name);
if (fieldName) { if (fieldName) {
if (state.paramValue.value) { const field = state.paramValue.schema.find((f) => f.id === fieldName);
delete state.paramValue.value[fieldName]; if (field) {
const field = state.paramValue.schema.find((f) => f.id === fieldName); deleteField(field);
if (field) {
field.removed = true;
state.paramValue.schema.splice(state.paramValue.schema.indexOf(field), 1, field);
}
emitValueChanged(); 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 { function addField(name: string): void {
if (name === 'addAllFields') { if (name === 'addAllFields') {
return addAllFields(); return addAllFields();
@ -395,23 +417,6 @@ function addAllFields(): void {
emitValueChanged(); 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 { function emitValueChanged(): void {
pruneParamValues(); pruneParamValues();
emit('valueChanged', { emit('valueChanged', {
@ -457,6 +462,7 @@ defineExpose({
/> />
<matching-columns-select <matching-columns-select
v-if="showMatchingColumnsSelector" v-if="showMatchingColumnsSelector"
:parameter="props.parameter"
:label-size="labelSize" :label-size="labelSize"
:fieldsToMap="state.paramValue.schema" :fieldsToMap="state.paramValue.schema"
:typeOptions="props.parameter.typeOptions" :typeOptions="props.parameter.typeOptions"
@ -465,7 +471,9 @@ defineExpose({
:initialValue="matchingColumns" :initialValue="matchingColumns"
:serviceName="nodeType?.displayName || locale.baseText('generic.service')" :serviceName="nodeType?.displayName || locale.baseText('generic.service')"
:teleported="teleported" :teleported="teleported"
:refreshInProgress="state.refreshInProgress"
@matchingColumnsChanged="onMatchingColumnsChanged" @matchingColumnsChanged="onMatchingColumnsChanged"
@refreshFieldList="initFetching(true)"
/> />
<n8n-text v-if="!showMappingModeSelect && state.loading" size="small"> <n8n-text v-if="!showMappingModeSelect && state.loading" size="small">
<n8n-icon icon="sync-alt" size="xsmall" :spin="true" /> <n8n-icon icon="sync-alt" size="xsmall" :spin="true" />

View file

@ -3,6 +3,7 @@ import {
MAPPING_COLUMNS_RESPONSE, MAPPING_COLUMNS_RESPONSE,
NODE_PARAMETER_VALUES, NODE_PARAMETER_VALUES,
UPDATED_SCHEMA, UPDATED_SCHEMA,
getLatestValueChangeEvent,
} from './utils/ResourceMapper.utils'; } from './utils/ResourceMapper.utils';
import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { waitAllPromises } from '@/__tests__/utils'; import { waitAllPromises } from '@/__tests__/utils';
@ -13,7 +14,8 @@ import { createComponentRenderer } from '@/__tests__/render';
import type { SpyInstance } from 'vitest'; import type { SpyInstance } from 'vitest';
let nodeTypeStore: ReturnType<typeof useNodeTypesStore>; let nodeTypeStore: ReturnType<typeof useNodeTypesStore>;
let fetchFieldsSpy: SpyInstance, resolveParameterSpy: SpyInstance; let fetchFieldsSpy: SpyInstance;
let resolveParameterSpy: SpyInstance;
const renderComponent = createComponentRenderer(ResourceMapper, DEFAULT_SETUP); const renderComponent = createComponentRenderer(ResourceMapper, DEFAULT_SETUP);
@ -289,4 +291,55 @@ describe('ResourceMapper.vue', () => {
await waitAllPromises(); await waitAllPromises();
expect(fetchFieldsSpy).not.toHaveBeenCalled(); 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 { STORES } from '@/constants';
import { createTestingPinia } from '@pinia/testing'; import { createTestingPinia } from '@pinia/testing';
import { merge } from 'lodash-es'; import { merge } from 'lodash-es';
import type { ResourceMapperFields } from 'n8n-workflow'; import type { ResourceMapperFields, ResourceMapperValue } from 'n8n-workflow';
export const NODE_PARAMETER_VALUES = { export const NODE_PARAMETER_VALUES = {
authentication: 'oAuth2', authentication: 'oAuth2',
@ -201,15 +201,16 @@ export const MAPPING_COLUMNS_RESPONSE: ResourceMapperFields = {
type: 'string', type: 'string',
canBeUsedToMatch: true, 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;
}>;
}