fix(editor): Disable test step option in context menu for sub-nodes (#13816)

This commit is contained in:
jeanpaul
2025-03-11 14:56:49 +01:00
committed by GitHub
parent e8334eefa1
commit b6d5092258
4 changed files with 300 additions and 3 deletions

View File

@@ -47,6 +47,7 @@ describe('useContextMenu', () => {
} as never);
vi.spyOn(NodeHelpers, 'getNodeInputs').mockReturnValue([]);
vi.spyOn(NodeHelpers, 'isExecutable').mockReturnValue(true);
});
afterEach(() => {
@@ -106,6 +107,18 @@ describe('useContextMenu', () => {
expect(targetNodeIds.value).toEqual([basicChain.id]);
});
it('should disable test step option for sub-nodes (AI tool nodes)', () => {
const { open, isOpen, actions, targetNodeIds } = useContextMenu();
const subNode = nodeFactory({ type: 'n8n-nodes-base.hackerNewsTool' });
vi.spyOn(workflowsStore, 'getNodeById').mockReturnValue(subNode);
vi.spyOn(NodeHelpers, 'isExecutable').mockReturnValueOnce(false);
open(mockEvent, { source: 'node-right-click', nodeId: subNode.id });
expect(isOpen.value).toBe(true);
expect(actions.value.find((action) => action.id === 'execute')?.disabled).toBe(true);
expect(targetNodeIds.value).toEqual([subNode.id]);
});
it('should return the correct actions when right clicking a Node', () => {
const { open, isOpen, actions, targetNodeIds } = useContextMenu();
const node = nodeFactory();
@@ -141,7 +154,6 @@ describe('useContextMenu', () => {
expect(actions.value).toMatchSnapshot();
expect(targetNodeIds.value).toEqual([sticky.id]);
});
it('should return the correct actions when right clicking a Node', () => {
vi.spyOn(uiStore, 'isReadOnlyView', 'get').mockReturnValue(true);
const { open, isOpen, actions, targetNodeIds } = useContextMenu();

View File

@@ -1,10 +1,11 @@
import type { ActionDropdownItem, XYPosition } from '@/Interface';
import type { ActionDropdownItem, XYPosition, INodeUi } from '@/Interface';
import { NOT_DUPLICATABLE_NODE_TYPES, STICKY_NODE_TYPE } from '@/constants';
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
import { useSourceControlStore } from '@/stores/sourceControl.store';
import { useUIStore } from '@/stores/ui.store';
import { useWorkflowsStore } from '@/stores/workflows.store';
import type { INode, INodeTypeDescription } from 'n8n-workflow';
import { NodeHelpers } from 'n8n-workflow';
import { computed, ref, watch } from 'vue';
import { getMousePosition } from '../utils/nodeViewUtils';
import { useI18n } from './useI18n';
@@ -94,6 +95,16 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) =
position.value = [0, 0];
};
const isExecutable = (node: INodeUi) => {
const currentWorkflow = workflowsStore.getCurrentWorkflow();
const workflowNode = currentWorkflow.getNode(node.name) as INode;
const nodeType = nodeTypesStore.getNodeType(
workflowNode.type,
workflowNode.typeVersion,
) as INodeTypeDescription;
return NodeHelpers.isExecutable(currentWorkflow, workflowNode, nodeType);
};
const open = (event: MouseEvent, menuTarget: ContextMenuTarget) => {
event.stopPropagation();
@@ -228,7 +239,7 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) =
{
id: 'execute',
label: i18n.baseText('contextMenu.test'),
disabled: isReadOnly.value,
disabled: isReadOnly.value || !isExecutable(nodes[0]),
},
{
id: 'rename',

View File

@@ -1718,3 +1718,13 @@ export function getVersionedNodeType(
}
return object;
}
export function isTriggerNode(nodeTypeData: INodeTypeDescription) {
return nodeTypeData.group.includes('trigger');
}
export function isExecutable(workflow: Workflow, node: INode, nodeTypeData: INodeTypeDescription) {
const outputs = getNodeOutputs(workflow, node, nodeTypeData);
const outputNames = getConnectionTypes(outputs);
return outputNames.includes(NodeConnectionType.Main) || isTriggerNode(nodeTypeData);
}

View File

@@ -13,6 +13,8 @@ import {
isSubNodeType,
applyDeclarativeNodeOptionParameters,
getParameterIssues,
isTriggerNode,
isExecutable,
} from '@/NodeHelpers';
import type { Workflow } from '@/Workflow';
@@ -4248,4 +4250,266 @@ describe('NodeHelpers', () => {
});
});
});
describe('isTriggerNode', () => {
const tests: Array<{
description: string;
input: INodeTypeDescription;
expected: boolean;
}> = [
{
description: 'Should return true for node with trigger in group',
input: {
name: 'TriggerNode',
displayName: 'Trigger Node',
group: ['trigger'],
description: 'Trigger node description',
version: 1,
defaults: {},
inputs: [],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: true,
},
{
description: 'Should return true for node with multiple groups including trigger',
input: {
name: 'MultiGroupTriggerNode',
displayName: 'Multi-Group Trigger Node',
group: ['trigger', 'input'],
description: 'Multi-group trigger node description',
version: 1,
defaults: {},
inputs: [],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: true,
},
{
description: 'Should return false for node without trigger in group',
input: {
name: 'RegularNode',
displayName: 'Regular Node',
group: ['input'],
description: 'Regular node description',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: false,
},
{
description: 'Should return false for node with empty group array',
input: {
name: 'EmptyGroupNode',
displayName: 'Empty Group Node',
group: [],
description: 'Empty group node description',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: false,
},
{
description:
'Should return false when trigger is called Trigger, but does not have a trigger group',
input: {
name: 'AlmostTriggerNode',
displayName: 'Almost Trigger Node',
group: ['transform'],
description: 'Almost trigger node description',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: false,
},
];
for (const testData of tests) {
test(testData.description, () => {
const result = isTriggerNode(testData.input);
expect(result).toEqual(testData.expected);
});
}
});
describe('isExecutable', () => {
const workflowMock = {
expression: {
getSimpleParameterValue: jest.fn().mockReturnValue([NodeConnectionType.Main]),
},
} as unknown as Workflow;
const tests: Array<{
description: string;
node: INode;
nodeTypeData: INodeTypeDescription;
expected: boolean;
mockReturnValue?: NodeConnectionType[];
}> = [
{
description: 'Should return true for trigger node',
node: {
id: 'triggerNodeId',
name: 'TriggerNode',
position: [0, 0],
type: 'n8n-nodes-base.TriggerNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'TriggerNode',
displayName: 'Trigger Node',
group: ['trigger'],
description: 'Trigger node description',
version: 1,
defaults: {},
inputs: [],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: true,
},
{
description: 'Should return true for node with Main output',
node: {
id: 'mainOutputNodeId',
name: 'MainOutputNode',
position: [0, 0],
type: 'n8n-nodes-base.MainOutputNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'MainOutputNode',
displayName: 'Main Output Node',
group: ['transform'],
description: 'Node with Main output',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
properties: [],
},
expected: true,
},
{
description: 'Should return false for node without Main output and not a trigger',
node: {
id: 'nonExecutableNodeId',
name: 'NonExecutableNode',
position: [0, 0],
type: 'n8n-nodes-base.NonExecutableNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'NonExecutableNode',
displayName: 'Non-Executable Node',
group: ['output'],
description: 'Node without Main output and not a trigger',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.AiAgent],
properties: [],
},
expected: false,
},
{
description: 'Should return true for node with mixed outputs including Main',
node: {
id: 'mixedOutputNodeId',
name: 'MixedOutputNode',
position: [0, 0],
type: 'n8n-nodes-base.MixedOutputNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'MixedOutputNode',
displayName: 'Mixed Output Node',
group: ['transform'],
description: 'Node with multiple output types including Main',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main, NodeConnectionType.AiAgent],
properties: [],
},
expected: true,
},
{
description: 'Should return false for node with only AiTool output and not a trigger',
node: {
id: 'aiToolOutputNodeId',
name: 'AiToolOutputNode',
position: [0, 0],
type: 'n8n-nodes-base.AiToolOutputNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'AiToolOutputNode',
displayName: 'AI Tool Output Node',
group: ['output'],
description: 'Node with only AiTool output and not a trigger',
version: 1,
defaults: {},
inputs: [],
outputs: [NodeConnectionType.AiTool], // Only AiTool output, no Main
properties: [],
},
expected: false,
},
{
description: 'Should return false for node with dynamic outputs set to AiTool only',
node: {
id: 'dynamicAiToolNodeId',
name: 'DynamicAiToolNode',
position: [0, 0],
type: 'n8n-nodes-base.DynamicAiToolNode',
typeVersion: 1,
parameters: {},
},
nodeTypeData: {
name: 'DynamicAiToolNode',
displayName: 'Dynamic AiTool Node',
group: ['output'],
description: 'Node with dynamic outputs that resolve to only AiTool',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: '={{["ai_tool"]}}', // Dynamic expression that resolves to AiTool only
properties: [],
},
expected: false,
mockReturnValue: [NodeConnectionType.AiTool],
},
];
for (const testData of tests) {
test(testData.description, () => {
// If this test has a custom mock return value, configure it
if (testData.mockReturnValue) {
(workflowMock.expression.getSimpleParameterValue as jest.Mock).mockReturnValueOnce(
testData.mockReturnValue,
);
}
const result = isExecutable(workflowMock, testData.node, testData.nodeTypeData);
expect(result).toEqual(testData.expected);
});
}
});
});