fix(Switch Node): Maintain output connections (#11162)

This commit is contained in:
Michael Kret 2024-11-13 13:30:56 +02:00 committed by GitHub
parent f0492bd3bb
commit 9bd79fceeb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 311 additions and 23 deletions

View file

@ -81,7 +81,7 @@ describe('NDV', () => {
ndv.getters.backToCanvas().click();
workflowPage.actions.executeWorkflow();
workflowPage.actions.openNode('Merge');
ndv.getters.outputPanel().contains('1 item').should('exist');
ndv.getters.outputPanel().contains('2 items').should('exist');
cy.contains('span', 'zero').should('exist');
});

View file

@ -22,11 +22,7 @@ import type {
IUpdateInformation,
} from '@/Interface';
import {
COMMUNITY_NODES_INSTALLATION_DOCS_URL,
CUSTOM_NODES_DOCS_URL,
SHOULD_CLEAR_NODE_OUTPUTS,
} from '@/constants';
import { COMMUNITY_NODES_INSTALLATION_DOCS_URL, CUSTOM_NODES_DOCS_URL } from '@/constants';
import NodeTitle from '@/components/NodeTitle.vue';
import ParameterInputList from '@/components/ParameterInputList.vue';
@ -47,11 +43,11 @@ 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';
import { useI18n } from '@/composables/useI18n';
import { useTelemetry } from '@/composables/useTelemetry';
import { importCurlEventBus, ndvEventBus } from '@/event-bus';
import { ProjectTypes } from '@/types/projects.types';
import { updateDynamicConnections } from '@/utils/nodeSettingsUtils';
const props = withDefaults(
defineProps<{
@ -94,7 +90,6 @@ const telemetry = useTelemetry();
const nodeHelpers = useNodeHelpers();
const externalHooks = useExternalHooks();
const i18n = useI18n();
const { showMessage } = useToast();
const nodeValid = ref(true);
const openPanel = ref<'params' | 'settings'>('params');
@ -483,20 +478,6 @@ const valueChanged = (parameterData: IUpdateInformation) => {
return;
}
if (
parameterData.type &&
workflowsStore.nodeHasOutputConnection(_node.name) &&
SHOULD_CLEAR_NODE_OUTPUTS[nodeType.name]?.eventTypes.includes(parameterData.type) &&
SHOULD_CLEAR_NODE_OUTPUTS[nodeType.name]?.parameterPaths.includes(parameterData.name)
) {
workflowsStore.removeAllNodeConnection(_node, { preserveInputConnections: true });
showMessage({
type: 'warning',
title: i18n.baseText('nodeSettings.outputCleared.title'),
message: i18n.baseText('nodeSettings.outputCleared.message'),
});
}
// Get only the parameters which are different to the defaults
let nodeParameters = NodeHelpers.getNodeParameters(
nodeType.properties,
@ -566,6 +547,14 @@ const valueChanged = (parameterData: IUpdateInformation) => {
value: nodeParameters,
};
const connections = workflowsStore.allConnections;
const updatedConnections = updateDynamicConnections(_node, connections, parameterData);
if (updatedConnections) {
workflowsStore.setConnections(updatedConnections, true);
}
workflowsStore.setNodeParameters(updateInformation);
void externalHooks.run('nodeSettings.valueChanged', {

View file

@ -1054,8 +1054,12 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => {
}, {});
}
function setConnections(connections: IConnections): void {
function setConnections(connections: IConnections, updateWorkflow = false): void {
workflow.value.connections = connections;
if (updateWorkflow) {
updateCachedWorkflow();
}
}
function resetAllNodesIssues(): boolean {

View file

@ -0,0 +1,162 @@
import { describe, it, expect, afterAll } from 'vitest';
import { mock } from 'vitest-mock-extended';
import type { IConnections, NodeParameterValueType } from 'n8n-workflow';
import { updateDynamicConnections } from './nodeSettingsUtils';
import { SWITCH_NODE_TYPE } from '@/constants';
import type { INodeUi, IUpdateInformation } from '@/Interface';
describe('updateDynamicConnections', () => {
afterAll(() => {
vi.clearAllMocks();
});
it('should remove extra outputs when the number of outputs decreases', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: SWITCH_NODE_TYPE,
parameters: { numberOutputs: 3 },
});
const connections = mock<IConnections>({
TestNode: {
main: [[{ node: 'Node1' }], [{ node: 'Node2' }], [{ node: 'Node3' }]],
},
});
const parameterData = mock<IUpdateInformation<number>>({
name: 'parameters.numberOutputs',
value: 2,
});
const updatedConnections = updateDynamicConnections(node, connections, parameterData);
expect(updatedConnections?.TestNode.main).toHaveLength(2);
});
it('should splice connections when a rule is removed', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: SWITCH_NODE_TYPE,
parameters: {
rules: { values: [{}, {}, {}] },
options: {},
},
});
const connections = mock<IConnections>({
TestNode: {
main: [[{ node: 'Node1' }], [{ node: 'Node2' }], [{ node: 'Node3' }]],
},
});
const parameterData = mock<IUpdateInformation<number>>({
name: 'parameters.rules.values[1]',
value: undefined,
});
const updatedConnections = updateDynamicConnections(node, connections, parameterData);
expect(updatedConnections?.TestNode.main).toHaveLength(2);
expect(updatedConnections?.TestNode.main[1][0].node).toEqual('Node3');
});
it('should handle fallbackOutput === "extra" and all rules removed', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: SWITCH_NODE_TYPE,
parameters: {
options: { fallbackOutput: 'extra' },
},
});
const connections = mock<IConnections>({
TestNode: {
main: [[{ node: 'Node1' }], [{ node: 'Node2' }], [{ node: 'Node3' }]],
},
});
const parameterData = mock<IUpdateInformation<number>>({
name: 'parameters.rules.values',
value: undefined,
});
const updatedConnections = updateDynamicConnections(node, connections, parameterData);
expect(updatedConnections?.TestNode.main).toHaveLength(1);
expect(updatedConnections?.TestNode.main[0][0].node).toEqual('Node3');
});
it('should add a new connection when a rule is added', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: SWITCH_NODE_TYPE,
parameters: {
rules: { values: [{}, {}] },
options: { fallbackOutput: 'none' },
},
});
const connections = mock<IConnections>({
TestNode: {
main: [[{ node: 'Node1' }], [{ node: 'Node2' }]],
},
});
const parameterData = mock<IUpdateInformation<NodeParameterValueType>>({
name: 'parameters.rules.values',
value: [{}, {}, {}],
});
const updatedConnections = updateDynamicConnections(node, connections, parameterData);
expect(updatedConnections?.TestNode.main).toHaveLength(3);
expect(updatedConnections?.TestNode.main[2]).toEqual([]);
});
it('should handle extra output when rule is added and fallbackOutput is extra', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: SWITCH_NODE_TYPE,
parameters: {
rules: { values: [{}, {}] },
options: { fallbackOutput: 'extra' },
},
});
const connections = mock<IConnections>({
TestNode: {
main: [[{ node: 'Node1' }], [{ node: 'Node2' }], [{ node: 'Node3' }]],
},
});
const parameterData = mock<IUpdateInformation<NodeParameterValueType>>({
name: 'parameters.rules.values',
value: [{}, {}, {}],
});
const updatedConnections = updateDynamicConnections(node, connections, parameterData);
expect(updatedConnections?.TestNode.main).toHaveLength(4);
expect(updatedConnections?.TestNode.main[2]).toEqual([]);
expect(updatedConnections?.TestNode.main[3][0].node).toEqual('Node3');
});
it('should return null if no conditions are met', () => {
const node = mock<INodeUi>({
name: 'TestNode',
type: 'otherNodeType',
});
const connections = mock<IConnections>({
TestNode: { main: [] },
});
const parameterData = mock<IUpdateInformation<number>>({
name: 'parameters.otherParameter',
value: 3,
});
const result = updateDynamicConnections(node, connections, parameterData);
expect(result).toBeNull();
});
});

View file

@ -0,0 +1,133 @@
import type {
IConnection,
IConnections,
IDataObject,
NodeInputConnections,
NodeParameterValueType,
} from 'n8n-workflow';
import type { INodeUi, IUpdateInformation } from '@/Interface';
import { SWITCH_NODE_TYPE } from '@/constants';
import { isEqual } from 'lodash-es';
import { captureException } from '@sentry/vue';
export function updateDynamicConnections(
node: INodeUi,
workflowConnections: IConnections,
parameterData: IUpdateInformation<NodeParameterValueType>,
) {
const connections = { ...workflowConnections };
try {
if (parameterData.name.includes('conditions') || !connections[node.name]?.main) return null;
if (node.type === SWITCH_NODE_TYPE && parameterData.name === 'parameters.numberOutputs') {
const curentNumberOutputs = node.parameters?.numberOutputs as number;
const newNumberOutputs = parameterData.value as number;
// remove extra outputs
if (newNumberOutputs < curentNumberOutputs) {
connections[node.name].main = connections[node.name].main.slice(0, newNumberOutputs);
return connections;
}
}
if (
node.type === SWITCH_NODE_TYPE &&
parameterData.name === 'parameters.options.fallbackOutput'
) {
const curentFallbackOutput = (node.parameters?.options as { fallbackOutput: string })
?.fallbackOutput as string;
if (curentFallbackOutput === 'extra') {
if (!parameterData.value || parameterData.value !== 'extra') {
connections[node.name].main = connections[node.name].main.slice(0, -1);
return connections;
}
}
}
if (node.type === SWITCH_NODE_TYPE && parameterData.name.includes('parameters.rules.values')) {
const { fallbackOutput } = node.parameters?.options as { fallbackOutput: string };
if (parameterData.value === undefined) {
function extractIndex(path: string): number | null {
const match = path.match(/parameters\.rules\.values\[(\d+)\]$/);
return match ? parseInt(match[1], 10) : null;
}
const index = extractIndex(parameterData.name);
// rule was removed
if (index !== null) {
connections[node.name].main.splice(index, 1);
return connections;
}
// all rules were removed
if (parameterData.name === 'parameters.rules.values') {
if (fallbackOutput === 'extra') {
connections[node.name].main = [
connections[node.name].main[connections[node.name].main.length - 1],
];
} else {
connections[node.name].main = [];
}
return connections;
}
} else if (parameterData.name === 'parameters.rules.values') {
const curentRulesvalues = (node.parameters?.rules as { values: IDataObject[] })?.values;
let lastConnection: IConnection[] | undefined = undefined;
if (
fallbackOutput === 'extra' &&
connections[node.name].main.length === curentRulesvalues.length + 1
) {
lastConnection = connections[node.name].main.pop();
}
// rule was added
const currentRulesLength = (node.parameters?.rules as { values: IDataObject[] })?.values
?.length;
const newRulesLength = (parameterData.value as IDataObject[])?.length;
if (newRulesLength - currentRulesLength === 1) {
connections[node.name].main = [...connections[node.name].main, []];
if (lastConnection) {
connections[node.name].main.push(lastConnection);
}
return connections;
} else {
// order was changed
const newRulesvalues = parameterData.value as IDataObject[];
const updatedConnectionsIndex: number[] = [];
for (const rule of curentRulesvalues) {
const index = newRulesvalues.findIndex((newRule) => isEqual(rule, newRule));
if (index !== -1) {
updatedConnectionsIndex.push(index);
}
}
const reorderedConnections: NodeInputConnections = [];
for (const index of updatedConnectionsIndex) {
reorderedConnections.push(connections[node.name].main[index] ?? []);
}
if (lastConnection) {
reorderedConnections.push(lastConnection);
}
connections[node.name].main = reorderedConnections;
return connections;
}
}
}
} catch (error) {
captureException(error);
}
return null;
}