feat(editor): Show fixed collection parameter issues in UI (#12899)

This commit is contained in:
Milorad FIlipović 2025-01-29 15:50:15 +01:00 committed by GitHub
parent 152310b7a0
commit 12d686ce52
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 444 additions and 148 deletions

View file

@ -4,21 +4,14 @@ import type { ILocalLoadOptionsFunctions, ResourceMapperFields } from 'n8n-workf
export async function loadSubWorkflowInputs(
this: ILocalLoadOptionsFunctions,
): Promise<ResourceMapperFields> {
const { fields, dataMode, subworkflowInfo } = await loadWorkflowInputMappings.bind(this)();
const { fields, subworkflowInfo } = await loadWorkflowInputMappings.bind(this)();
let emptyFieldsNotice: string | undefined;
if (fields.length === 0) {
const subworkflowLink = subworkflowInfo?.id
? `<a href="/workflow/${subworkflowInfo?.id}" target="_blank">sub-workflows trigger</a>`
: 'sub-workflows trigger';
switch (dataMode) {
case 'passthrough':
emptyFieldsNotice = `This sub-workflow will consume all input data passed to it. Define specific expected input in the ${subworkflowLink}.`;
break;
default:
emptyFieldsNotice = `This sub-workflow will not receive any input when called by your AI node. Define your expected input in the ${subworkflowLink}.`;
break;
}
emptyFieldsNotice = `This sub-workflow will not receive any input when called by your AI node. Define your expected input in the ${subworkflowLink}.`;
}
return { fields, emptyFieldsNotice };
}

View file

@ -35,20 +35,29 @@ exports[`components > N8nCheckbox > should render with both child and label 1`]
class="n8n-input-label inputLabel heading medium"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium regular"
<div
class="title"
>
Checkbox
<!--v-if-->
</span>
<span
class="n8n-text size-medium regular"
>
Checkbox
<!--v-if-->
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>
@ -126,20 +135,29 @@ exports[`components > N8nCheckbox > should render with label 1`] = `
class="n8n-input-label inputLabel heading medium"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium regular"
<div
class="title"
>
Checkbox
<!--v-if-->
</span>
<span
class="n8n-text size-medium regular"
>
Checkbox
<!--v-if-->
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>

View file

@ -38,26 +38,35 @@ exports[`FormBox > should render the component 1`] = `
for="name"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-small bold"
<div
class="title"
>
Name
<span
class="n8n-text primary size-small bold"
class="n8n-text size-small bold"
>
*
Name
<span
class="n8n-text primary size-small bold"
>
*
</span>
</span>
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>
<div
@ -110,26 +119,35 @@ exports[`FormBox > should render the component 1`] = `
for="email"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium bold"
<div
class="title"
>
Email
<span
class="n8n-text primary size-medium bold"
class="n8n-text size-medium bold"
>
*
Email
<span
class="n8n-text primary size-medium bold"
>
*
</span>
</span>
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>
<div
@ -182,26 +200,35 @@ exports[`FormBox > should render the component 1`] = `
for="password"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium bold"
<div
class="title"
>
Password
<span
class="n8n-text primary size-medium bold"
class="n8n-text size-medium bold"
>
*
Password
<span
class="n8n-text primary size-medium bold"
>
*
</span>
</span>
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>
<div

View file

@ -53,41 +53,52 @@ const addTargetBlank = (html: string) =>
[$style.overflow]: !!$slots.options,
}"
>
<div v-if="label" :class="$style.title">
<N8nText
:bold="bold"
:size="size"
:compact="compact"
:color="color"
:class="{
[$style.textEllipses]: showOptions,
}"
<div :class="$style['main-content']">
<div v-if="label" :class="$style.title">
<N8nText
:bold="bold"
:size="size"
:compact="compact"
:color="color"
:class="{
[$style.textEllipses]: showOptions,
}"
>
{{ label }}
<N8nText v-if="required" color="primary" :bold="bold" :size="size">*</N8nText>
</N8nText>
</div>
<span
v-if="tooltipText && label"
:class="[$style.infoIcon, showTooltip ? $style.visible : $style.hidden]"
>
{{ label }}
<N8nText v-if="required" color="primary" :bold="bold" :size="size">*</N8nText>
</N8nText>
<N8nTooltip placement="top" :popper-class="$style.tooltipPopper" :show-after="300">
<N8nIcon icon="question-circle" size="small" />
<template #content>
<div v-n8n-html="addTargetBlank(tooltipText)" />
</template>
</N8nTooltip>
</span>
</div>
<span
v-if="tooltipText && label"
:class="[$style.infoIcon, showTooltip ? $style.visible : $style.hidden]"
>
<N8nTooltip placement="top" :popper-class="$style.tooltipPopper" :show-after="300">
<N8nIcon icon="question-circle" size="small" />
<template #content>
<div v-n8n-html="addTargetBlank(tooltipText)" />
</template>
</N8nTooltip>
</span>
<div
v-if="$slots.options && label"
:class="{ [$style.overlay]: true, [$style.visible]: showOptions }"
/>
<div
v-if="$slots.options"
:class="{ [$style.options]: true, [$style.visible]: showOptions }"
:data-test-id="`${inputName}-parameter-input-options-container`"
>
<slot name="options" />
<div :class="$style['trailing-content']">
<div
v-if="$slots.options && label"
:class="{ [$style.overlay]: true, [$style.visible]: showOptions }"
/>
<div
v-if="$slots.options"
:class="{ [$style.options]: true, [$style.visible]: showOptions }"
:data-test-id="`${inputName}-parameter-input-options-container`"
>
<slot name="options" />
</div>
<div
v-if="$slots.issues"
:class="$style.issues"
:data-test-id="`${inputName}-parameter-input-issues-container`"
>
<slot name="issues" />
</div>
</div>
</label>
<slot />
@ -98,20 +109,40 @@ const addTargetBlank = (html: string) =>
.container {
display: flex;
flex-direction: column;
label {
display: flex;
justify-content: space-between;
}
}
.main-content {
display: flex;
&:hover {
.infoIcon {
opacity: 1;
&:hover {
color: var(--color-text-base);
}
}
}
}
.trailing-content {
display: flex;
gap: var(--spacing-3xs);
* {
align-self: center;
}
}
.inputLabel {
display: block;
}
.container:hover,
.inputLabel:hover {
.infoIcon {
opacity: 1;
&:hover {
color: var(--color-text-base);
}
}
.options {
opacity: 1;
transition: opacity 100ms ease-in; // transition on hover in
@ -150,10 +181,13 @@ const addTargetBlank = (html: string) =>
.options {
opacity: 0;
transition: opacity 250ms cubic-bezier(0.98, -0.06, 0.49, -0.2); // transition on hover out
display: flex;
align-self: center;
}
> * {
float: right;
}
.issues {
display: flex;
align-self: center;
}
.overlay {

View file

@ -10,20 +10,29 @@ exports[`component > Text overflow behavior > displays ellipsis with options 1`]
class="n8n-input-label inputLabel heading medium"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium bold textEllipses textEllipses"
<div
class="title"
>
a label
<!--v-if-->
</span>
<span
class="n8n-text size-medium bold textEllipses textEllipses"
>
a label
<!--v-if-->
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>
@ -41,20 +50,29 @@ exports[`component > Text overflow behavior > displays full text without options
class="n8n-input-label inputLabel heading medium"
>
<div
class="title"
class="main-content"
>
<span
class="n8n-text size-medium bold"
<div
class="title"
>
a label
<!--v-if-->
</span>
<span
class="n8n-text size-medium bold"
>
a label
<!--v-if-->
</span>
</div>
<!--v-if-->
</div>
<div
class="trailing-content"
>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
</label>

View file

@ -0,0 +1,70 @@
import type { INodeUi } from '@/Interface';
import type { INodeParameters, INodeProperties } from 'n8n-workflow';
export const TEST_PARAMETERS: INodeProperties[] = [
{
displayName: 'Test Fixed Collection',
name: 'fixedCollectionTest',
placeholder: 'Test',
type: 'fixedCollection',
description:
'Test fixed collection description. This is a long description that should be wrapped.',
typeOptions: { multipleValues: true, sortable: true, minRequiredFields: 1 },
displayOptions: {
show: { '@version': [{ _cnd: { gte: 1.1 } }] },
},
default: {},
options: [
{
name: 'values',
displayName: 'Values',
values: [
{
displayName: 'Name',
name: 'name',
type: 'string',
default: '',
placeholder: 'e.g. fieldName',
description: 'A name of the field in the collection',
required: true,
noDataExpression: true,
},
],
},
],
},
];
export const FIXED_COLLECTION_PARAMETERS: INodeProperties[] = TEST_PARAMETERS.filter(
(p) => p.type === 'fixedCollection',
);
export const TEST_NODE_VALUES: INodeParameters = {
color: '#ff0000',
alwaysOutputData: false,
executeOnce: false,
notesInFlow: false,
onError: 'stopWorkflow',
retryOnFail: false,
maxTries: 3,
waitBetweenTries: 1000,
notes: '',
parameters: { fixedCollectionTest: {} },
};
export const TEST_NODE_NO_ISSUES: INodeUi = {
id: 'test-123',
parameters: { fixedCollectionTest: { values: [{ name: 'firstName' }] } },
typeVersion: 1.1,
name: 'Test Node',
type: 'n8n-nodes-base.executeWorkflowTrigger',
position: [260, 340],
};
export const TEST_ISSUE = 'At least 1 field is required.';
export const TEST_NODE_WITH_ISSUES: INodeUi = {
...TEST_NODE_NO_ISSUES,
parameters: { fixedCollectionTest: {} },
issues: { parameters: { fixedCollectionTest: [TEST_ISSUE] } },
};

View file

@ -0,0 +1,101 @@
import { createComponentRenderer } from '@/__tests__/render';
import ParameterInputList from './ParameterInputList.vue';
import { createTestingPinia } from '@pinia/testing';
import { mockedStore } from '@/__tests__/utils';
import { useNDVStore } from '@/stores/ndv.store';
import {
TEST_NODE_NO_ISSUES,
TEST_PARAMETERS,
TEST_NODE_VALUES,
TEST_NODE_WITH_ISSUES,
FIXED_COLLECTION_PARAMETERS,
TEST_ISSUE,
} from './ParameterInputList.test.constants';
vi.mock('vue-router', async () => {
const actual = await vi.importActual('vue-router');
const params = {};
const location = {};
return {
...actual,
useRouter: () => ({
push: vi.fn(),
}),
useRoute: () => ({
params,
location,
}),
};
});
let ndvStore: ReturnType<typeof mockedStore<typeof useNDVStore>>;
const renderComponent = createComponentRenderer(ParameterInputList, {
props: {
hideDelete: true,
indent: true,
isReadOnly: false,
},
global: {
stubs: {
ParameterInputFull: { template: '<div data-test-id="parameter-input"></div>' },
Suspense: { template: '<div data-test-id="suspense-stub"><slot></slot></div>' },
},
},
});
describe('ParameterInputList', () => {
beforeEach(() => {
createTestingPinia();
ndvStore = mockedStore(useNDVStore);
});
it('renders', () => {
ndvStore.activeNode = TEST_NODE_NO_ISSUES;
expect(() =>
renderComponent({
props: {
parameters: TEST_PARAMETERS,
nodeValues: TEST_NODE_VALUES,
},
}),
).not.toThrow();
});
it('renders fixed collection inputs correctly', () => {
ndvStore.activeNode = TEST_NODE_NO_ISSUES;
const { getAllByTestId, getByText } = renderComponent({
props: {
parameters: TEST_PARAMETERS,
nodeValues: TEST_NODE_VALUES,
},
});
// Should render labels for all parameters
TEST_PARAMETERS.forEach((parameter) => {
expect(getByText(parameter.displayName)).toBeInTheDocument();
});
// Should render input placeholders for all fixed collection parameters
expect(getAllByTestId('suspense-stub')).toHaveLength(FIXED_COLLECTION_PARAMETERS.length);
});
it('renders fixed collection inputs correctly with issues', () => {
ndvStore.activeNode = TEST_NODE_WITH_ISSUES;
const { getByText, getByTestId } = renderComponent({
props: {
parameters: TEST_PARAMETERS,
nodeValues: TEST_NODE_VALUES,
},
});
// Should render labels for all parameters
TEST_PARAMETERS.forEach((parameter) => {
expect(getByText(parameter.displayName)).toBeInTheDocument();
});
// Should render error message for fixed collection parameter
expect(
getByTestId(`${FIXED_COLLECTION_PARAMETERS[0].name}-parameter-input-issues-container`),
).toBeInTheDocument();
expect(getByText(TEST_ISSUE)).toBeInTheDocument();
});
});

View file

@ -5,7 +5,7 @@ import type {
NodeParameterValue,
NodeParameterValueType,
} from 'n8n-workflow';
import { deepCopy, ADD_FORM_NOTICE } from 'n8n-workflow';
import { deepCopy, ADD_FORM_NOTICE, NodeHelpers } from 'n8n-workflow';
import { computed, defineAsyncComponent, onErrorCaptured, ref, watch } from 'vue';
import type { IUpdateInformation } from '@/Interface';
@ -45,6 +45,9 @@ const LazyCollectionParameter = defineAsyncComponent(
async () => await import('./CollectionParameter.vue'),
);
// Parameter issues are displayed within the inputs themselves, but some parameters need to show them in the label UI
const showIssuesInLabelFor = ['fixedCollection'];
type Props = {
nodeValues: INodeParameters;
parameters: INodeProperties[];
@ -432,6 +435,15 @@ function onNoticeAction(action: string) {
}
}
function getParameterIssues(parameter: INodeProperties): string[] {
if (!node.value || !showIssuesInLabelFor.includes(parameter.type)) {
return [];
}
const issues = NodeHelpers.getParameterIssues(parameter, node.value.parameters, '', node.value);
return issues.parameters?.[parameter.name] ?? [];
}
/**
* Handles default node button parameter type actions
* @param parameter
@ -536,8 +548,26 @@ function getParameterValue<T extends NodeParameterValueType = NodeParameterValue
:tooltip-text="i18n.nodeText().inputLabelDescription(parameter, path)"
size="small"
:underline="true"
:input-name="parameter.name"
color="text-dark"
/>
>
<template
v-if="
showIssuesInLabelFor.includes(parameter.type) &&
getParameterIssues(parameter).length > 0
"
#issues
>
<N8nTooltip>
<template #content>
<span v-for="(issue, i) in getParameterIssues(parameter)" :key="i">{{
issue
}}</span>
</template>
<N8nIcon icon="exclamation-triangle" size="small" color="danger" />
</N8nTooltip>
</template>
</N8nInputLabel>
<Suspense v-if="!asyncLoadingError">
<template #default>
<LazyCollectionParameter

View file

@ -310,15 +310,6 @@ defineExpose({
color="text-dark"
>
<template #options>
<ParameterOptions
:parameter="parameter"
:custom-actions="parameterActions"
:loading="props.refreshInProgress"
:loading-message="fetchingFieldsLabel"
:is-read-only="isReadOnly"
:value="props.paramValue"
@update:model-value="onParameterActionSelected"
/>
<div v-if="props.isDataStale && !props.refreshInProgress" :class="$style.staleDataWarning">
<N8nTooltip>
<template #content>
@ -340,6 +331,15 @@ defineExpose({
@click="onParameterActionSelected('refreshFieldList')"
/>
</div>
<ParameterOptions
:parameter="parameter"
:custom-actions="parameterActions"
:loading="props.refreshInProgress"
:loading-message="fetchingFieldsLabel"
:is-read-only="isReadOnly"
:value="props.paramValue"
@update:model-value="onParameterActionSelected"
/>
</template>
</N8nInputLabel>
<div v-if="orderedFields.length === 0" class="mt-3xs mb-xs">

View file

@ -3,10 +3,15 @@
exports[`MultipleParameter > should render correctly 1`] = `
"<div data-v-a47e4507="" class="duplicate-parameter">
<div data-v-a47e4507="" class="container" data-test-id="input-label"><label class="n8n-input-label inputLabel heading underline small">
<div class="title"><span class="n8n-text text-dark size-small bold">Additional Fields <!--v-if--></span></div>
<!--v-if-->
<!--v-if-->
<!--v-if-->
<div class="main-content">
<div class="title"><span class="n8n-text text-dark size-small bold">Additional Fields <!--v-if--></span></div>
<!--v-if-->
</div>
<div class="trailing-content">
<!--v-if-->
<!--v-if-->
<!--v-if-->
</div>
</label></div>
<div data-v-a47e4507="" class="add-item-wrapper">
<div data-v-a47e4507="" class="no-items-exist"><span data-v-a47e4507="" class="n8n-text size-small regular">Currently no items exist</span></div><button data-v-a47e4507="" class="button button tertiary medium block" aria-live="polite">