Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Drop outgoing connections on order changed event for nodes with dynamic outputs #9055

20 changes: 20 additions & 0 deletions cypress/e2e/5-ndv.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,26 @@ describe('NDV', () => {
cy.shouldNotHaveConsoleErrors();
});

it('should disconect Switch outputs if rules order was changed', () => {
cy.createFixtureWorkflow('NDV-test-switch_reorder.json', `NDV test switch reorder`);
workflowPage.actions.zoomToFit();

workflowPage.actions.executeWorkflow();
workflowPage.actions.openNode('Merge');
ndv.getters.outputPanel().contains('2 items').should('exist');
cy.contains('span', 'first').should('exist');
ndv.getters.backToCanvas().click();

workflowPage.actions.openNode('Switch');
cy.get('.cm-line').realMouseMove(100, 100);
cy.get('.fa-angle-down').click();
ndv.getters.backToCanvas().click();
workflowPage.actions.executeWorkflow();
workflowPage.actions.openNode('Merge');
ndv.getters.outputPanel().contains('1 item').should('exist');
cy.contains('span', 'zero').should('exist');
});

it('should show correct validation state for resource locator params', () => {
workflowPage.actions.addNodeToCanvas('Typeform', true, true);
ndv.getters.container().should('be.visible');
Expand Down
235 changes: 235 additions & 0 deletions cypress/fixtures/NDV-test-switch_reorder.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
{
"name": "switch reorder",
"nodes": [
{
"parameters": {},
"id": "b3f0815d-b733-413f-ab3f-74e48277bd3a",
"name": "When clicking \"Test workflow\"",
"type": "n8n-nodes-base.manualTrigger",
"typeVersion": 1,
"position": [
-20,
620
]
},
{
"parameters": {},
"id": "fbc5b12a-6165-4cab-80a1-9fd6e4fbe39f",
"name": "One",
"type": "n8n-nodes-base.noOp",
"typeVersion": 1,
"position": [
620,
720
]
},
{
"parameters": {
"duplicateItem": true,
"duplicateCount": 1,
"assignments": {
"assignments": [
{
"id": "ec6c1d1d-a17a-4537-8135-d474df7fded1",
"name": "entry",
"value": "first",
"type": "string"
}
]
},
"options": {}
},
"id": "8c5a72a5-17ef-40e0-8477-764f24770174",
"name": "Edit Fields",
"type": "n8n-nodes-base.set",
"typeVersion": 3.3,
"position": [
160,
740
]
},
{
"parameters": {
"assignments": {
"assignments": [
{
"id": "d8ec7c46-d02f-4bf5-931e-5ec2fb8bea22",
"name": "entry",
"value": "zero",
"type": "string"
}
]
},
"options": {}
},
"id": "bc3fb81d-2ddf-4b28-a93d-762a48e8fd6b",
"name": "Edit Fields1",
"type": "n8n-nodes-base.set",
"typeVersion": 3.3,
"position": [
160,
500
]
},
{
"parameters": {
"rules": {
"values": [
{
"conditions": {
"options": {
"caseSensitive": true,
"leftValue": "",
"typeValidation": "strict"
},
"conditions": [
{
"leftValue": "={{ $json.entry }}",
"rightValue": "first",
"operator": {
"type": "string",
"operation": "equals"
}
}
],
"combinator": "and"
},
"renameOutput": true,
"outputKey": "1"
},
{
"conditions": {
"options": {
"caseSensitive": true,
"leftValue": "",
"typeValidation": "strict"
},
"conditions": [
{
"id": "ffa570ef-fc16-49ec-87be-56159f14a44b",
"leftValue": "={{ $json.entry }}",
"rightValue": "=second",
"operator": {
"type": "string",
"operation": "equals"
}
}
],
"combinator": "and"
},
"renameOutput": true,
"outputKey": "2"
}
]
},
"options": {}
},
"id": "296ba553-c6c5-4c84-89fb-9056b24bab30",
"name": "Switch",
"type": "n8n-nodes-base.switch",
"typeVersion": 3,
"position": [
360,
740
]
},
{
"parameters": {},
"id": "da787dd6-8e85-4dd5-8326-198705b4ae4b",
"name": "Merge",
"type": "n8n-nodes-base.merge",
"typeVersion": 2.1,
"position": [
880,
520
]
}
],
"pinData": {
"Edit Fields": [
{
"json": {
"entry": "first"
}
},
{
"json": {
"entry": "second"
}
}
]
},
"connections": {
"When clicking \"Test workflow\"": {
"main": [
[
{
"node": "Edit Fields",
"type": "main",
"index": 0
},
{
"node": "Edit Fields1",
"type": "main",
"index": 0
}
]
]
},
"Edit Fields": {
"main": [
[
{
"node": "Switch",
"type": "main",
"index": 0
}
]
]
},
"One": {
"main": [
[
{
"node": "Merge",
"type": "main",
"index": 1
}
]
]
},
"Edit Fields1": {
"main": [
[
{
"node": "Merge",
"type": "main",
"index": 0
}
]
]
},
"Switch": {
"main": [
[
{
"node": "One",
"type": "main",
"index": 0
}
]
]
}
},
"active": false,
"settings": {
"executionOrder": "v1"
},
"versionId": "ce5db792-5e38-4d54-895b-88d85f2545d0",
"meta": {
"templateCredsSetupCompleted": true,
"instanceId": "be251a83c052a9862eeac953816fbb1464f89dfbf79d7ac490a8e336a8cc8bfd"
},
"id": "uMpL0bN7t1NYZDJS",
"tags": []
}
1 change: 1 addition & 0 deletions packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export interface IUpdateInformation {
| INodeParameters; // with null makes problems in NodeSettings.vue
node?: string;
oldValue?: string | number;
type?: 'optionsOrderChanged';
}

export interface INodeUpdatePropertiesInformation {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ export default defineComponent({
const parameterData = {
name: this.getPropertyPath(optionName),
value: this.mutableValues[optionName],
type: 'optionsOrderChanged',
};

this.$emit('valueChanged', parameterData);
Expand All @@ -270,6 +271,7 @@ export default defineComponent({
const parameterData = {
name: this.getPropertyPath(optionName),
value: this.mutableValues[optionName],
type: 'optionsOrderChanged',
};

this.$emit('valueChanged', parameterData);
Expand Down
18 changes: 18 additions & 0 deletions packages/editor-ui/src/components/NodeSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ import {
CUSTOM_NODES_DOCS_URL,
MAIN_NODE_PANEL_WIDTH,
IMPORT_CURL_MODAL_KEY,
SHOULD_CLEAR_NODE_OUTPUTS,
} from '@/constants';

import NodeTitle from '@/components/NodeTitle.vue';
Expand All @@ -223,6 +224,7 @@ import { useCredentialsStore } from '@/stores/credentials.store';
import type { EventBus } from 'n8n-design-system';
import { useExternalHooks } from '@/composables/useExternalHooks';
import { useNodeHelpers } from '@/composables/useNodeHelpers';
import { useToast } from '@/composables/useToast';

export default defineComponent({
name: 'NodeSettings',
Expand All @@ -238,10 +240,12 @@ export default defineComponent({
setup() {
const nodeHelpers = useNodeHelpers();
const externalHooks = useExternalHooks();
const { showMessage } = useToast();

return {
externalHooks,
nodeHelpers,
showMessage,
};
},
computed: {
Expand Down Expand Up @@ -853,6 +857,20 @@ export default defineComponent({
return;
}

if (
parameterData.type &&
this.workflowsStore.nodeHasOutputConnection(node.name) &&
SHOULD_CLEAR_NODE_OUTPUTS[nodeType.name]?.eventTypes.includes(parameterData.type) &&
SHOULD_CLEAR_NODE_OUTPUTS[nodeType.name]?.parameterPaths.includes(parameterData.name)
) {
this.workflowsStore.clearNodeOutgoingConnections(node.name);
this.showMessage({
OlegIvaniv marked this conversation as resolved.
Show resolved Hide resolved
type: 'warning',
title: this.$locale.baseText('nodeSettings.outputCleared.title'),
message: this.$locale.baseText('nodeSettings.outputCleared.message'),
});
}

// Get only the parameters which are different to the defaults
let nodeParameters = NodeHelpers.getNodeParameters(
nodeType.properties,
Expand Down
14 changes: 14 additions & 0 deletions packages/editor-ui/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,20 @@ export const MFA_AUTHENTICATION_RECOVERY_CODE_INPUT_MAX_LENGTH = 36;

export const NODE_TYPES_EXCLUDED_FROM_OUTPUT_NAME_APPEND = [FILTER_NODE_TYPE, SWITCH_NODE_TYPE];

type ClearOutgoingConnectonsEvents = {
[nodeName: string]: {
parameterPaths: string[];
eventTypes: string[];
};
};

export const SHOULD_CLEAR_NODE_OUTPUTS: ClearOutgoingConnectonsEvents = {
[SWITCH_NODE_TYPE]: {
parameterPaths: ['parameters.rules.values'],
eventTypes: ['optionsOrderChanged'],
},
};

export const ALLOWED_HTML_ATTRIBUTES = ['href', 'name', 'target', 'title', 'class', 'id', 'style'];

export const ALLOWED_HTML_TAGS = [
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/plugins/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,8 @@
"nodeSettings.latest": "Latest",
"nodeSettings.deprecated": "Deprecated",
"nodeSettings.latestVersion": "Latest version: {version}",
"nodeSettings.outputCleared.title": "Parameters changed",
"nodeSettings.outputCleared.message": "Order of parameters changed, outgoing connections were cleared",
"nodeSettings.nodeVersion": "{node} node version {version}",
"nodeView.addNode": "Add node",
"nodeView.openNodesPanel": "Open nodes panel",
Expand Down
13 changes: 13 additions & 0 deletions packages/editor-ui/src/stores/workflows.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
return {};
};
},
nodeHasOutputConnection() {
return (nodeName: string): boolean => {
if (this.workflow.connections.hasOwnProperty(nodeName)) return true;
return false;
};
},
isNodeInOutgoingNodeConnections() {
return (firstNode: string, secondNode: string): boolean => {
const firstNodeConnections = this.outgoingConnectionsByNodeName(firstNode);
Expand Down Expand Up @@ -285,6 +291,13 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, {
},
},
actions: {
clearNodeOutgoingConnections(nodeName: string) {
OlegIvaniv marked this conversation as resolved.
Show resolved Hide resolved
if (this.workflow.connections.hasOwnProperty(nodeName)) {
delete this.workflow.connections[nodeName];
useUIStore().stateIsDirty = true;
}
return;
},
getPinDataSize(pinData: Record<string, string | INodeExecutionData[]> = {}): number {
return Object.values(pinData).reduce<number>((acc, value) => {
return acc + stringSizeInBytes(value);
Expand Down