feat(editor): Supress validation errors for freshly added nodes (#5149)

* feat(editor): Supress validation errors when node is added from node creator

* Supress initial errors also for resource locator inputs

* Use nodeMetadata prop to store node's `pristine` state

* Revert `setNodeParameters` check for `nodeMetadata`

* Rename getIsNodePristine to isNodePristine
This commit is contained in:
OlegIvaniv
2023-01-16 14:55:58 +01:00
committed by GitHub
parent 96d773f82d
commit 582865c7e9
11 changed files with 179 additions and 27 deletions

View File

@@ -53,4 +53,41 @@ describe('NDV', () => {
ndv.getters.dataContainer().should('contain', 'start');
});
});
it('should show correct validation state for resource locator params', () => {
workflowPage.actions.addNodeToCanvas('Typeform', true);
ndv.getters.container().should('be.visible');
cy.get('.has-issues').should('have.length', 0);
cy.get('[class*=hasIssues]').should('have.length', 0);
ndv.getters.backToCanvas().click();
// Both credentials and resource locator errors should be visible
workflowPage.actions.openNodeNdv('Typeform');
cy.get('.has-issues').should('have.length', 1);
cy.get('[class*=hasIssues]').should('have.length', 1);
});
it('should show validation errors only after blur or re-opening of NDV', () => {
workflowPage.actions.addNodeToCanvas('Manual Trigger');
workflowPage.actions.addNodeToCanvas('Airtable', true);
ndv.getters.container().should('be.visible');
cy.get('.has-issues').should('have.length', 0);
workflowPage.getters.ndvParameterInput('table').find('input').eq(1).focus().blur()
workflowPage.getters.ndvParameterInput('application').find('input').eq(1).focus().blur()
cy.get('.has-issues').should('have.length', 2);
ndv.getters.backToCanvas().click();
workflowPage.actions.openNodeNdv('Airtable');
cy.get('.has-issues').should('have.length', 3);
cy.get('[class*=hasIssues]').should('have.length', 1);
});
it('should show all validation errors when opening pasted node', () => {
cy.fixture('Test_workflow_ndv_errors.json').then((data) => {
cy.get('body').paste(JSON.stringify(data));
workflowPage.getters.canvasNodes().should('have.have.length', 1);
workflowPage.actions.openNodeNdv('Airtable');
cy.get('.has-issues').should('have.length', 3);
cy.get('[class*=hasIssues]').should('have.length', 1);
});
});
});

View File

@@ -0,0 +1,32 @@
{
"meta": {
"instanceId": "3204fc455f5cbeb4e71fdbd3b1dfaf0b088088dea3e639de49e61462b80ffc1d"
},
"nodes": [
{
"parameters": {
"application": {
"__rl": true,
"mode": "url",
"value": "",
"__regex": "https://airtable.com/([a-zA-Z0-9]{2,})"
},
"table": {
"__rl": true,
"mode": "url",
"value": "",
"__regex": "https://airtable.com/[a-zA-Z0-9]{2,}/([a-zA-Z0-9]{2,})"
}
},
"id": "e0c0cf7e-aa98-4b72-9645-6e64e2902bd1",
"name": "Airtable",
"type": "n8n-nodes-base.airtable",
"typeVersion": 1,
"position": [
380,
180
]
}
],
"connections": {}
}

View File

@@ -93,11 +93,12 @@ export class WorkflowPage extends BasePage {
this.getters.nodeCreatorSearchBar().type('{enter}');
cy.get('body').type('{esc}');
},
addNodeToCanvas: (nodeDisplayName: string) => {
addNodeToCanvas: (nodeDisplayName: string, preventNdvClose?: boolean) => {
this.getters.nodeCreatorPlusButton().click();
this.getters.nodeCreatorSearchBar().type(nodeDisplayName);
this.getters.nodeCreatorSearchBar().type('{enter}');
cy.get('body').type('{esc}');
if (!preventNdvClose) cy.get('body').type('{esc}');
},
openNodeNdv: (nodeTypeName: string) => {
this.getters.canvasNodeByName(nodeTypeName).dblclick();

View File

@@ -962,6 +962,7 @@ export interface ITemplatesNode extends IVersionNode {
export interface INodeMetadata {
parametersLastUpdatedAt?: number;
pristine: boolean;
}
export interface IUsedCredential {

View File

@@ -27,10 +27,11 @@
size="small"
/>
</div>
<div v-else :class="issues.length ? $style.hasIssues : $style.input">
<div v-else :class="issues.length && !hideIssues ? $style.hasIssues : $style.input">
<n8n-select
:value="getSelectedId(credentialTypeDescription.name)"
@change="(value) => onCredentialSelected(credentialTypeDescription.name, value)"
@blur="$emit('blur', 'credentials')"
:placeholder="getSelectPlaceholder(credentialTypeDescription.name, issues)"
size="small"
>
@@ -49,7 +50,7 @@
</n8n-option>
</n8n-select>
<div :class="$style.warning" v-if="issues.length">
<div :class="$style.warning" v-if="issues.length && !hideIssues">
<n8n-tooltip placement="top">
<template #content>
<titled-list
@@ -82,6 +83,7 @@
</template>
<script lang="ts">
import { PropType } from 'vue';
import { restApi } from '@/mixins/restApi';
import {
ICredentialsResponse,
@@ -108,11 +110,23 @@ import { useCredentialsStore } from '@/stores/credentials';
export default mixins(genericHelpers, nodeHelpers, restApi, showMessage).extend({
name: 'NodeCredentials',
props: [
'readonly',
'node', // INodeUi
'overrideCredType', // cred type
],
props: {
readonly: {
type: Boolean,
default: false,
},
node: {
type: Object as PropType<INodeUi>,
required: true,
},
overrideCredType: {
type: String,
},
hideIssues: {
type: Boolean,
default: false,
},
},
components: {
TitledList,
},

View File

@@ -93,14 +93,18 @@
:hideDelete="true"
:nodeValues="nodeValues"
:isReadOnly="isReadOnly"
:hiddenIssuesInputs="hiddenIssuesInputs"
path="parameters"
@valueChanged="valueChanged"
@activate="onWorkflowActivate"
@parameterBlur="onParameterBlur"
>
<node-credentials
:node="node"
:readonly="isReadOnly"
@credentialSelected="credentialSelected"
@blur="onParameterBlur"
:hide-issues="hiddenIssuesInputs.includes('credentials')"
/>
</parameter-input-list>
<div v-if="parametersNoneSetting.length === 0" class="no-parameters">
@@ -124,16 +128,20 @@
:parameters="parametersSetting"
:nodeValues="nodeValues"
:isReadOnly="isReadOnly"
:hiddenIssuesInputs="hiddenIssuesInputs"
path="parameters"
@valueChanged="valueChanged"
@parameterBlur="onParameterBlur"
/>
<parameter-input-list
:parameters="nodeSettings"
:hideDelete="true"
:nodeValues="nodeValues"
:isReadOnly="isReadOnly"
:hiddenIssuesInputs="hiddenIssuesInputs"
path=""
@valueChanged="valueChanged"
@parameterBlur="onParameterBlur"
/>
</div>
</div>
@@ -414,6 +422,7 @@ export default mixins(externalHooks, nodeHelpers).extend({
COMMUNITY_NODES_INSTALLATION_DOCS_URL,
CUSTOM_NODES_DOCS_URL,
MAIN_NODE_PANEL_WIDTH,
hiddenIssuesInputs: [] as string[],
};
},
watch: {
@@ -445,10 +454,25 @@ export default mixins(externalHooks, nodeHelpers).extend({
},
},
methods: {
populateHiddenIssuesSet() {
if (!this.node || !this.workflowsStore.isNodePristine(this.node.name)) return;
this.hiddenIssuesInputs.push('credentials');
this.parametersNoneSetting.forEach((parameter) => {
this.hiddenIssuesInputs.push(parameter.name);
});
this.workflowsStore.setNodePristine(this.node.name, false);
},
onParameterBlur(parameterName: string) {
this.hiddenIssuesInputs = this.hiddenIssuesInputs.filter((name) => name !== parameterName);
},
onWorkflowActivate() {
this.hiddenIssuesInputs = [];
this.$emit('activate');
},
onNodeExecute() {
this.hiddenIssuesInputs = [];
this.$emit('execute');
},
setValue(name: string, value: NodeParameterValue) {
@@ -457,7 +481,7 @@ export default mixins(externalHooks, nodeHelpers).extend({
let isArray = false;
if (lastNamePart !== undefined && lastNamePart.includes('[')) {
// It incldues an index so we have to extract it
// It includes an index so we have to extract it
const lastNameParts = lastNamePart.match(/(.*)\[(\d+)\]$/);
if (lastNameParts) {
nameParts.push(lastNameParts[1]);
@@ -835,6 +859,7 @@ export default mixins(externalHooks, nodeHelpers).extend({
},
},
mounted() {
this.populateHiddenIssuesSet();
this.setNodeValues();
if (this.eventBus) {
(this.eventBus as Vue).$on('openSettings', () => {

View File

@@ -53,6 +53,7 @@
:activeDrop="activeDrop"
:forceShowExpression="forceShowExpression"
:hint="hint"
:hide-issues="hideIssues"
@valueChanged="valueChanged"
@textInput="onTextInput"
@focus="onFocus"
@@ -119,6 +120,10 @@ export default mixins(showMessage).extend({
type: Boolean,
default: false,
},
hideIssues: {
type: Boolean,
default: false,
},
parameter: {
type: Object as PropType<INodeProperties>,
},
@@ -197,6 +202,7 @@ export default mixins(showMessage).extend({
if (!this.parameter.noDataExpression) {
this.ndvStore.setMappableNDVInputFocus('');
}
this.$emit('blur');
},
onMenuExpanded(expanded: boolean) {
this.menuExpanded = expanded;

View File

@@ -93,11 +93,13 @@
<parameter-input-full
:parameter="parameter"
:hide-issues="hiddenIssuesInputs.includes(parameter.name)"
:value="getParameterValue(nodeValues, parameter.name, path)"
:displayOptions="true"
:path="getPath(parameter.name)"
:isReadOnly="isReadOnly"
@valueChanged="valueChanged"
@blur="onParameterBlur(parameter.name)"
/>
</div>
</div>
@@ -108,13 +110,7 @@
</template>
<script lang="ts">
import {
deepCopy,
INodeParameters,
INodeProperties,
INodeTypeDescription,
NodeParameterValue,
} from 'n8n-workflow';
import { deepCopy, INodeParameters, INodeProperties, NodeParameterValue } from 'n8n-workflow';
import { INodeUi, IUpdateInformation } from '@/Interface';
@@ -126,8 +122,8 @@ import ImportParameter from '@/components/ImportParameter.vue';
import { get, set } from 'lodash';
import mixins from 'vue-typed-mixins';
import { Component } from 'vue';
import { mapState, mapStores } from 'pinia';
import { Component, PropType } from 'vue';
import { mapStores } from 'pinia';
import { useNDVStore } from '@/stores/ndv';
import { useNodeTypesStore } from '@/stores/nodeTypes';
@@ -140,14 +136,36 @@ export default mixins(workflowHelpers).extend({
CollectionParameter: () => import('./CollectionParameter.vue') as Promise<Component>,
ImportParameter,
},
props: [
'nodeValues', // INodeParameters
'parameters', // INodeProperties
'path', // string
'hideDelete', // boolean
'indent',
'isReadOnly',
],
props: {
nodeValues: {
type: Object as PropType<INodeParameters>,
required: true,
},
parameters: {
type: Array as PropType<INodeProperties[]>,
required: true,
},
path: {
type: String,
default: '',
},
hideDelete: {
type: Boolean,
default: false,
},
indent: {
type: Boolean,
default: false,
},
isReadOnly: {
type: Boolean,
default: false,
},
hiddenIssuesInputs: {
type: Array as PropType<string[]>,
default: () => [],
},
},
computed: {
...mapStores(useNodeTypesStore, useNDVStore),
nodeTypeVersion(): number | null {
@@ -187,6 +205,9 @@ export default mixins(workflowHelpers).extend({
},
},
methods: {
onParameterBlur(parameterName: string) {
this.$emit('parameterBlur', parameterName);
},
getCredentialsDependencies() {
const dependencies = new Set();
const nodeType = this.nodeTypesStore.getNodeType(

View File

@@ -684,6 +684,7 @@ export default mixins(debounceHelper, workflowHelpers, nodeHelpers).extend({
if (!this.isSearchable || this.currentQueryError) {
this.showResourceDropdown = false;
}
this.$emit('blur');
},
},
});

View File

@@ -227,6 +227,10 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
this.nodeMetadata[nodeName] && this.nodeMetadata[nodeName].parametersLastUpdatedAt;
},
isNodePristine(): (name: string) => boolean {
return (nodeName: string) =>
this.nodeMetadata[nodeName] === undefined || this.nodeMetadata[nodeName].pristine === true;
},
// Executions getters
getExecutionDataById(): (id: string) => IExecutionsSummary | undefined {
return (id: string): IExecutionsSummary | undefined =>
@@ -731,7 +735,12 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
// TODO: Check if there is an error or whatever that is supposed to be returned
return;
}
this.workflow.nodes.push(nodeData);
// Init node metadata
if (!this.nodeMetadata[nodeData.name]) {
Vue.set(this.nodeMetadata, nodeData.name, {});
}
},
removeNode(node: INodeUi): void {
@@ -821,6 +830,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
if (!this.nodeMetadata[node.name]) {
Vue.set(this.nodeMetadata, node.name, {});
}
Vue.set(this.nodeMetadata[node.name], 'parametersLastUpdatedAt', Date.now());
},
@@ -960,5 +970,8 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
}
});
},
setNodePristine(nodeName: string, isPristine: boolean): void {
Vue.set(this.nodeMetadata[nodeName], 'pristine', isPristine);
},
},
});

View File

@@ -1907,6 +1907,7 @@ export default mixins(
}
await this.addNodes([newNodeData], undefined, trackHistory);
this.workflowsStore.setNodePristine(newNodeData.name, true);
this.uiStore.stateIsDirty = true;