From d17a15ef9a560a0b0d7b327e247e1a7635316d04 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Tue, 10 Jun 2025 11:34:08 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"feat(editor):=20Change=20default=20no?= =?UTF-8?q?de=20names=20depending=20on=20node=20ope=E2=80=A6=20(#16169)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cypress/e2e/11-inline-expression-editor.cy.ts | 6 +- cypress/e2e/17-sharing.cy.ts | 6 +- cypress/e2e/39-projects.cy.ts | 8 +- cypress/e2e/4-node-creator.cy.ts | 8 +- cypress/e2e/41-editors.cy.ts | 16 +- cypress/e2e/45-ai-assistant.cy.ts | 2 +- cypress/e2e/5-ndv.cy.ts | 9 +- cypress/e2e/9-expression-editor-modal.cy.ts | 8 +- cypress/pages/workflow.ts | 2 +- .../manual-execution.service.test.ts | 5 - packages/cli/src/manual-execution.service.ts | 9 +- .../__tests__/is-tool.test.ts | 102 +++++ .../partial-execution-utils/index.ts | 1 + .../partial-execution-utils/is-tool.ts | 22 ++ .../src/execution-engine/workflow-execute.ts | 7 +- .../NodeCreator/composables/useActions.ts | 23 +- .../editor-ui/src/components/NodeSettings.vue | 11 - .../src/composables/useCanvasOperations.ts | 5 +- .../src/composables/useNodeHelpers.ts | 17 - packages/workflow/src/node-helpers.ts | 124 +----- packages/workflow/test/node-helpers.test.ts | 365 ------------------ 21 files changed, 182 insertions(+), 574 deletions(-) create mode 100644 packages/core/src/execution-engine/partial-execution-utils/__tests__/is-tool.test.ts create mode 100644 packages/core/src/execution-engine/partial-execution-utils/is-tool.ts diff --git a/cypress/e2e/11-inline-expression-editor.cy.ts b/cypress/e2e/11-inline-expression-editor.cy.ts index bfbbbcaf16..5afebc4de6 100644 --- a/cypress/e2e/11-inline-expression-editor.cy.ts +++ b/cypress/e2e/11-inline-expression-editor.cy.ts @@ -50,7 +50,7 @@ describe('Inline expression editor', () => { beforeEach(() => { WorkflowPage.actions.addNodeToCanvas('Hacker News'); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openInlineExpressionEditor(); }); @@ -112,7 +112,7 @@ describe('Inline expression editor', () => { WorkflowPage.actions.addNodeToCanvas('No Operation'); WorkflowPage.actions.addNodeToCanvas('Hacker News'); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openInlineExpressionEditor(); }); @@ -150,7 +150,7 @@ describe('Inline expression editor', () => { // Run workflow ndv.actions.close(); WorkflowPage.actions.executeNode('No Operation, do nothing', { anchor: 'topLeft' }); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openInlineExpressionEditor(); // Previous nodes have run, input can be resolved diff --git a/cypress/e2e/17-sharing.cy.ts b/cypress/e2e/17-sharing.cy.ts index 84365153c7..468cc2ed73 100644 --- a/cypress/e2e/17-sharing.cy.ts +++ b/cypress/e2e/17-sharing.cy.ts @@ -96,7 +96,7 @@ describe('Sharing', { disableAutoLogin: true }, () => { ndv.actions.close(); workflowPage.actions.saveWorkflowOnButtonClick(); - workflowPage.actions.openNode('Append a block'); + workflowPage.actions.openNode('Notion'); ndv.getters.credentialInput().should('have.value', 'Credential C1').should('be.disabled'); ndv.actions.close(); }); @@ -112,7 +112,7 @@ describe('Sharing', { disableAutoLogin: true }, () => { ndv.actions.close(); workflowPage.actions.saveWorkflowOnButtonClick(); - workflowPage.actions.openNode('Append a block'); + workflowPage.actions.openNode('Notion'); ndv.getters .credentialInput() .find('input') @@ -136,7 +136,7 @@ describe('Sharing', { disableAutoLogin: true }, () => { cy.visit(workflowsPage.url); workflowsPage.getters.workflowCards().should('have.length', 2); workflowsPage.getters.workflowCardContent('Workflow W1').click(); - workflowPage.actions.openNode('Append a block'); + workflowPage.actions.openNode('Notion'); ndv.getters .credentialInput() .find('input') diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index 5d46f58621..e6e53ae41c 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -157,7 +157,7 @@ describe('Projects', { disableAutoLogin: true }, () => { expect(interception.request.query).not.to.have.property('projectId'); expect(interception.request.query).to.have.property('workflowId'); }); - workflowPage.getters.canvasNodeByName('Append a block').should('be.visible').dblclick(); + workflowPage.getters.canvasNodeByName(NOTION_NODE_NAME).should('be.visible').dblclick(); workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') @@ -182,7 +182,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.actions.saveWorkflowOnButtonClick(); cy.reload(); - workflowPage.getters.canvasNodeByName('Append a block').should('be.visible').dblclick(); + workflowPage.getters.canvasNodeByName(NOTION_NODE_NAME).should('be.visible').dblclick(); workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') @@ -211,7 +211,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowPage.actions.saveWorkflowOnButtonClick(); cy.reload(); - workflowPage.getters.canvasNodeByName('Append a block').should('be.visible').dblclick(); + workflowPage.getters.canvasNodeByName(NOTION_NODE_NAME).should('be.visible').dblclick(); workflowPage.getters.nodeCredentialsSelect().first().click(); getVisibleSelect() .find('li') @@ -280,7 +280,7 @@ describe('Projects', { disableAutoLogin: true }, () => { workflowsPage.getters.workflowCards().first().findChildByTestId('card-content').click(); // Check if the credential can be changed - workflowPage.getters.canvasNodeByName('Append a block').should('be.visible').dblclick(); + workflowPage.getters.canvasNodeByName(NOTION_NODE_NAME).should('be.visible').dblclick(); ndv.getters.credentialInput().find('input').should('be.enabled'); }); diff --git a/cypress/e2e/4-node-creator.cy.ts b/cypress/e2e/4-node-creator.cy.ts index 7a8c75d5f8..7c11f7ee9e 100644 --- a/cypress/e2e/4-node-creator.cy.ts +++ b/cypress/e2e/4-node-creator.cy.ts @@ -316,7 +316,7 @@ describe('Node Creator', () => { nodeCreatorFeature.getters.getCreatorItem('Create a credential').click(); NDVModal.actions.close(); WorkflowPage.actions.deleteNode('When clicking ‘Execute workflow’'); - WorkflowPage.getters.canvasNodePlusEndpointByName('Create a credential').click(); + WorkflowPage.getters.canvasNodePlusEndpointByName('n8n').click(); nodeCreatorFeature.getters.searchBar().find('input').clear().type('n8n'); nodeCreatorFeature.getters.getCreatorItem('n8n').click(); nodeCreatorFeature.getters.getCategoryItem('Actions').click(); @@ -324,11 +324,7 @@ describe('Node Creator', () => { NDVModal.actions.close(); WorkflowPage.getters.canvasNodes().should('have.length', 2); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.addNodeBetweenNodes( - 'Create a credential', - 'Create a credential1', - 'Summarize', - ); + WorkflowPage.actions.addNodeBetweenNodes('n8n', 'n8n1', 'Summarize'); WorkflowPage.getters.canvasNodes().should('have.length', 3); }); }); diff --git a/cypress/e2e/41-editors.cy.ts b/cypress/e2e/41-editors.cy.ts index c59e65129c..d84aa5843f 100644 --- a/cypress/e2e/41-editors.cy.ts +++ b/cypress/e2e/41-editors.cy.ts @@ -24,14 +24,14 @@ describe('Editors', () => { .type('SELECT * FROM `testTable`', { delay: TYPING_DELAY }) .type('{esc}'); ndv.actions.close(); - workflowPage.actions.openNode('Execute a SQL query'); + workflowPage.actions.openNode('Postgres'); ndv.getters .sqlEditorContainer() .find('.cm-content') .type('{end} LIMIT 10', { delay: TYPING_DELAY }) .type('{esc}'); ndv.actions.close(); - workflowPage.actions.openNode('Execute a SQL query'); + workflowPage.actions.openNode('Postgres'); ndv.getters.sqlEditorContainer().should('contain', 'SELECT * FROM `testTable` LIMIT 10'); }); @@ -45,7 +45,7 @@ describe('Editors', () => { ndv.actions.setPinnedData([{ table: 'test_table' }]); ndv.actions.close(); - workflowPage.actions.openNode('Execute a SQL query'); + workflowPage.actions.openNode('MySQL'); ndv.getters .sqlEditorContainer() .find('.cm-content') @@ -86,7 +86,7 @@ describe('Editors', () => { ndv.actions.close(); workflowPage.actions.saveWorkflowOnButtonClick(); workflowPage.getters.isWorkflowSaved(); - workflowPage.actions.openNode('Execute a SQL query'); + workflowPage.actions.openNode('Postgres'); ndv.actions.close(); // Workflow should still be saved workflowPage.getters.isWorkflowSaved(); @@ -100,7 +100,7 @@ describe('Editors', () => { ndv.actions.close(); workflowPage.actions.saveWorkflowOnButtonClick(); workflowPage.getters.isWorkflowSaved(); - workflowPage.actions.openNode('Execute a SQL query'); + workflowPage.actions.openNode('Postgres'); ndv.getters .sqlEditorContainer() .click() @@ -131,14 +131,14 @@ describe('Editors', () => { .paste('SELECT * FROM `secondTable`'); ndv.actions.close(); - workflowPage.actions.openNode('Execute a SQL query'); - ndv.actions.clickFloatingNode('Execute a SQL query1'); + workflowPage.actions.openNode('Postgres'); + ndv.actions.clickFloatingNode('Postgres1'); ndv.getters .sqlEditorContainer() .find('.cm-content') .should('have.text', 'SELECT * FROM `secondTable`'); - ndv.actions.clickFloatingNode('Execute a SQL query'); + ndv.actions.clickFloatingNode('Postgres'); ndv.getters .sqlEditorContainer() .find('.cm-content') diff --git a/cypress/e2e/45-ai-assistant.cy.ts b/cypress/e2e/45-ai-assistant.cy.ts index db3f489506..ab6f4e3f40 100644 --- a/cypress/e2e/45-ai-assistant.cy.ts +++ b/cypress/e2e/45-ai-assistant.cy.ts @@ -364,7 +364,7 @@ describe('AI Assistant Credential Help', () => { }).as('chatRequest'); wf.actions.addNodeToCanvas(SCHEDULE_TRIGGER_NODE_NAME); wf.actions.addNodeToCanvas(GMAIL_NODE_NAME); - wf.actions.openNode('Add label to message'); + wf.actions.openNode('Gmail'); openCredentialSelect(); clickCreateNewCredential(); aiAssistant.getters.credentialEditAssistantButton().find('button').should('be.visible'); diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index e98f6fd88c..6be23cb4be 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -86,11 +86,18 @@ describe('NDV', () => { cy.get('[class*=hasIssues]').should('have.length', 1); }); - it('should show validation errors only after blur or re-opening of NDV of a node with operation and resource', () => { + it('should show validation errors only after blur or re-opening of NDV', () => { workflowPage.actions.addNodeToCanvas('Manual'); workflowPage.actions.addNodeToCanvas('Airtable', true, true, 'Search records'); ndv.getters.container().should('be.visible'); + cy.get('.has-issues').should('have.length', 0); + ndv.getters.parameterInput('table').find('input').eq(1).focus().blur(); + ndv.getters.parameterInput('base').find('input').eq(1).focus().blur(); cy.get('.has-issues').should('have.length', 2); + ndv.getters.backToCanvas().click(); + workflowPage.actions.openNode('Airtable'); + cy.get('.has-issues').should('have.length', 2); + cy.get('[class*=hasIssues]').should('have.length', 1); }); // Correctly failing in V2 - node issues are only shows after execution diff --git a/cypress/e2e/9-expression-editor-modal.cy.ts b/cypress/e2e/9-expression-editor-modal.cy.ts index 5cd9b7e2e2..79185d5ec6 100644 --- a/cypress/e2e/9-expression-editor-modal.cy.ts +++ b/cypress/e2e/9-expression-editor-modal.cy.ts @@ -17,7 +17,7 @@ describe('Expression editor modal', () => { beforeEach(() => { WorkflowPage.actions.addNodeToCanvas('Hacker News'); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openExpressionEditorModal(); }); @@ -34,7 +34,7 @@ describe('Expression editor modal', () => { beforeEach(() => { WorkflowPage.actions.addNodeToCanvas('Hacker News'); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openExpressionEditorModal(); }); @@ -90,7 +90,7 @@ describe('Expression editor modal', () => { WorkflowPage.actions.addNodeToCanvas('No Operation'); WorkflowPage.actions.addNodeToCanvas('Hacker News'); WorkflowPage.actions.zoomToFit(); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openExpressionEditorModal(); }); @@ -125,7 +125,7 @@ describe('Expression editor modal', () => { cy.get('body').type('{esc}'); ndv.actions.close(); WorkflowPage.actions.executeNode('No Operation, do nothing', { anchor: 'topLeft' }); - WorkflowPage.actions.openNode('Get many items'); + WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openExpressionEditorModal(); // Previous nodes have run, input can be resolved diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index 57dc7c145c..91c3555ebf 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -217,7 +217,7 @@ export class WorkflowPage extends BasePage { cy.get('body').then((body) => { if (body.find('[data-test-id=node-creator]').length > 0) { if (action) { - cy.get('[data-keyboard-nav-type="action"]').contains(action).click(); + cy.contains(action).click(); } else { // Select the first action if (body.find('[data-keyboard-nav-type="action"]').length > 0) { diff --git a/packages/cli/src/__tests__/manual-execution.service.test.ts b/packages/cli/src/__tests__/manual-execution.service.test.ts index a0bce205fa..c7bcd76acf 100644 --- a/packages/cli/src/__tests__/manual-execution.service.test.ts +++ b/packages/cli/src/__tests__/manual-execution.service.test.ts @@ -562,11 +562,6 @@ describe('ManualExecutionService', () => { return null; }), getTriggerNodes: jest.fn().mockReturnValue([determinedStartNode]), - nodeTypes: { - getByNameAndVersion: jest - .fn() - .mockReturnValue({ description: { name: '', outputs: [] } }), - }, }); jest diff --git a/packages/cli/src/manual-execution.service.ts b/packages/cli/src/manual-execution.service.ts index df6587a15d..7a3df1ce17 100644 --- a/packages/cli/src/manual-execution.service.ts +++ b/packages/cli/src/manual-execution.service.ts @@ -6,9 +6,10 @@ import { filterDisabledNodes, recreateNodeExecutionStack, WorkflowExecute, + isTool, rewireGraph, } from 'n8n-core'; -import { MANUAL_TRIGGER_NODE_TYPE, NodeHelpers } from 'n8n-workflow'; +import { MANUAL_TRIGGER_NODE_TYPE } from 'n8n-workflow'; import type { IExecuteData, IPinData, @@ -135,12 +136,8 @@ export class ManualExecutionService { `Could not find a node named "${data.destinationNode}" in the workflow.`, ); - const destinationNodeType = workflow.nodeTypes.getByNameAndVersion( - destinationNode.type, - destinationNode.typeVersion, - ); // Rewire graph to be able to execute the destination tool node - if (NodeHelpers.isTool(destinationNodeType.description, destinationNode.parameters)) { + if (isTool(destinationNode, workflow.nodeTypes)) { const graph = rewireGraph( destinationNode, DirectedGraph.fromWorkflow(workflow), diff --git a/packages/core/src/execution-engine/partial-execution-utils/__tests__/is-tool.test.ts b/packages/core/src/execution-engine/partial-execution-utils/__tests__/is-tool.test.ts new file mode 100644 index 0000000000..70321acff6 --- /dev/null +++ b/packages/core/src/execution-engine/partial-execution-utils/__tests__/is-tool.test.ts @@ -0,0 +1,102 @@ +import { mock } from 'jest-mock-extended'; +import { type INode, type INodeTypes, NodeConnectionTypes } from 'n8n-workflow'; + +import { isTool } from '../is-tool'; + +describe('isTool', () => { + const mockNode = mock({ + id: '1', + type: 'n8n-nodes-base.openAi', + typeVersion: 1, + parameters: {}, + }); + const mockNodeTypes = mock(); + + it('should return true for a node with AiTool output', () => { + mockNodeTypes.getByNameAndVersion.mockReturnValue({ + description: { + outputs: [NodeConnectionTypes.AiTool], + version: 0, + defaults: { + name: '', + color: '', + }, + inputs: [NodeConnectionTypes.Main], + properties: [], + displayName: '', + name: '', + group: [], + description: '', + }, + }); + const result = isTool(mockNode, mockNodeTypes); + expect(result).toBe(true); + }); + + it('should return true for a node with AiTool output in NodeOutputConfiguration', () => { + mockNodeTypes.getByNameAndVersion.mockReturnValue({ + description: { + outputs: [{ type: NodeConnectionTypes.AiTool }, { type: NodeConnectionTypes.Main }], + version: 0, + defaults: { + name: '', + color: '', + }, + inputs: [NodeConnectionTypes.Main], + properties: [], + displayName: '', + name: '', + group: [], + description: '', + }, + }); + const result = isTool(mockNode, mockNodeTypes); + expect(result).toBe(true); + }); + + it('returns true for a vectore store node in retrieve-as-tool mode', () => { + mockNode.type = 'n8n-nodes-base.vectorStore'; + mockNode.parameters = { mode: 'retrieve-as-tool' }; + mockNodeTypes.getByNameAndVersion.mockReturnValue({ + description: { + outputs: [NodeConnectionTypes.Main], + version: 0, + defaults: { + name: '', + color: '', + }, + inputs: [NodeConnectionTypes.Main], + properties: [], + displayName: '', + name: '', + group: [], + description: '', + }, + }); + const result = isTool(mockNode, mockNodeTypes); + expect(result).toBe(true); + }); + + it('returns false for node with no AiTool output', () => { + mockNode.type = 'n8n-nodes-base.someTool'; + mockNode.parameters = {}; + mockNodeTypes.getByNameAndVersion.mockReturnValue({ + description: { + outputs: [NodeConnectionTypes.Main], + version: 0, + defaults: { + name: '', + color: '', + }, + inputs: [NodeConnectionTypes.Main], + properties: [], + displayName: '', + name: '', + group: [], + description: '', + }, + }); + const result = isTool(mockNode, mockNodeTypes); + expect(result).toBe(false); + }); +}); diff --git a/packages/core/src/execution-engine/partial-execution-utils/index.ts b/packages/core/src/execution-engine/partial-execution-utils/index.ts index bd01d10fcc..5400ccbf3b 100644 --- a/packages/core/src/execution-engine/partial-execution-utils/index.ts +++ b/packages/core/src/execution-engine/partial-execution-utils/index.ts @@ -6,5 +6,6 @@ export { recreateNodeExecutionStack } from './recreate-node-execution-stack'; export { cleanRunData } from './clean-run-data'; export { handleCycles } from './handle-cycles'; export { filterDisabledNodes } from './filter-disabled-nodes'; +export { isTool } from './is-tool'; export { rewireGraph } from './rewire-graph'; export { getNextExecutionIndex } from './run-data-utils'; diff --git a/packages/core/src/execution-engine/partial-execution-utils/is-tool.ts b/packages/core/src/execution-engine/partial-execution-utils/is-tool.ts new file mode 100644 index 0000000000..0dd06a7c91 --- /dev/null +++ b/packages/core/src/execution-engine/partial-execution-utils/is-tool.ts @@ -0,0 +1,22 @@ +import { type INode, type INodeTypes, NodeConnectionTypes } from 'n8n-workflow'; + +export function isTool(node: INode, nodeTypes: INodeTypes) { + const type = nodeTypes.getByNameAndVersion(node.type, node.typeVersion); + + // Check if node is a vector store in retrieve-as-tool mode + if (node.type.includes('vectorStore')) { + const mode = node.parameters?.mode; + return mode === 'retrieve-as-tool'; + } + + // Check for other tool nodes + for (const output of type.description.outputs) { + if (typeof output === 'string') { + return output === NodeConnectionTypes.AiTool; + } else if (output?.type && output.type === NodeConnectionTypes.AiTool) { + return true; + } + } + + return false; +} diff --git a/packages/core/src/execution-engine/workflow-execute.ts b/packages/core/src/execution-engine/workflow-execute.ts index 2657d6c429..f4af04b536 100644 --- a/packages/core/src/execution-engine/workflow-execute.ts +++ b/packages/core/src/execution-engine/workflow-execute.ts @@ -72,6 +72,7 @@ import { handleCycles, filterDisabledNodes, rewireGraph, + isTool, getNextExecutionIndex, } from './partial-execution-utils'; import { TOOL_EXECUTOR_NODE_NAME } from './partial-execution-utils/rewire-graph'; @@ -367,12 +368,8 @@ export class WorkflowExecute { let graph = DirectedGraph.fromWorkflow(workflow); - const destinationNodeType = workflow.nodeTypes.getByNameAndVersion( - destination.type, - destination.typeVersion, - ); // Partial execution of nodes as tools - if (NodeHelpers.isTool(destinationNodeType.description, destination.parameters)) { + if (isTool(destination, workflow.nodeTypes)) { graph = rewireGraph(destination, graph, agentRequest); workflow = graph.toWorkflow({ ...workflow }); // Rewire destination node to the virtual agent diff --git a/packages/frontend/editor-ui/src/components/Node/NodeCreator/composables/useActions.ts b/packages/frontend/editor-ui/src/components/Node/NodeCreator/composables/useActions.ts index 798921c3a4..5143dac3f4 100644 --- a/packages/frontend/editor-ui/src/components/Node/NodeCreator/composables/useActions.ts +++ b/packages/frontend/editor-ui/src/components/Node/NodeCreator/composables/useActions.ts @@ -2,7 +2,6 @@ import { computed } from 'vue'; import { CHAIN_LLM_LANGCHAIN_NODE_TYPE, NodeConnectionTypes, - NodeHelpers, type IDataObject, type INodeParameters, } from 'n8n-workflow'; @@ -44,14 +43,11 @@ import { useExternalHooks } from '@/composables/useExternalHooks'; import { sortNodeCreateElements, transformNodeType } from '../utils'; import { useI18n } from '@n8n/i18n'; import { useCanvasStore } from '@/stores/canvas.store'; -import { useCanvasOperations } from '@/composables/useCanvasOperations'; -import findLast from 'lodash/findLast'; export const useActions = () => { const nodeCreatorStore = useNodeCreatorStore(); const nodeTypesStore = useNodeTypesStore(); const i18n = useI18n(); - const canvasOperations = useCanvasOperations(); const singleNodeOpenSources = [ NODE_CREATOR_OPEN_SOURCES.PLUS_ENDPOINT, NODE_CREATOR_OPEN_SOURCES.NODE_CONNECTION_ACTION, @@ -309,7 +305,7 @@ export const useActions = () => { return { nodes, connections }; } - // Hook into addNode action to set the last node parameters, adjust default name and track the action selected + // Hook into addNode action to set the last node parameters & track the action selected function setAddedNodeActionParameters( action: IUpdateInformation, telemetry?: Telemetry, @@ -317,25 +313,10 @@ export const useActions = () => { ) { const { $onAction: onWorkflowStoreAction } = useWorkflowsStore(); const storeWatcher = onWorkflowStoreAction( - ({ name, after, store: { setLastNodeParameters, allNodes }, args }) => { + ({ name, after, store: { setLastNodeParameters }, args }) => { if (name !== 'addNode' || args[0].type !== action.key) return; after(() => { - const node = findLast(allNodes, (n) => n.type === action.key); - const nodeType = node && nodeTypesStore.getNodeType(node.type, node.typeVersion); - const wasDefaultName = - nodeType && NodeHelpers.isDefaultNodeName(node.name, nodeType, node.parameters ?? {}); - setLastNodeParameters(action); - - // We update the default name here based on the chosen resource and operation - if (wasDefaultName) { - const newName = NodeHelpers.makeNodeName(node.parameters, nodeType); - // Account for unique-ified nodes with `` - if (!node.name.startsWith(newName)) { - // setTimeout to allow remaining events trigger by node addition to finish - setTimeout(async () => await canvasOperations.renameNode(node.name, newName)); - } - } if (telemetry) trackActionSelected(action, telemetry, rootView); // Unsubscribe from the store watcher storeWatcher(); diff --git a/packages/frontend/editor-ui/src/components/NodeSettings.vue b/packages/frontend/editor-ui/src/components/NodeSettings.vue index da30207dd7..8e1767529b 100644 --- a/packages/frontend/editor-ui/src/components/NodeSettings.vue +++ b/packages/frontend/editor-ui/src/components/NodeSettings.vue @@ -53,7 +53,6 @@ import { importCurlEventBus, ndvEventBus } from '@/event-bus'; import { ProjectTypes } from '@/types/projects.types'; import { updateDynamicConnections } from '@/utils/nodeSettingsUtils'; import FreeAiCreditsCallout from '@/components/FreeAiCreditsCallout.vue'; -import { useCanvasOperations } from '@/composables/useCanvasOperations'; const props = withDefaults( defineProps<{ @@ -97,7 +96,6 @@ const telemetry = useTelemetry(); const nodeHelpers = useNodeHelpers(); const externalHooks = useExternalHooks(); const i18n = useI18n(); -const canvasOperations = useCanvasOperations(); const nodeValid = ref(true); const openPanel = ref<'params' | 'settings'>('params'); @@ -581,15 +579,6 @@ const valueChanged = (parameterData: IUpdateInformation) => { } } - if (NodeHelpers.isDefaultNodeName(_node.name, nodeType, node.value?.parameters ?? {})) { - const newName = NodeHelpers.makeNodeName(nodeParameters ?? {}, nodeType); - // Account for unique-ified nodes with `` - if (!_node.name.startsWith(newName)) { - // We need a timeout here to support events reacting to the valueChange based on node names - setTimeout(async () => await canvasOperations.renameNode(_node.name, newName)); - } - } - for (const key of Object.keys(nodeParameters as object)) { if (nodeParameters && nodeParameters[key] !== null && nodeParameters[key] !== undefined) { setValue(`parameters.${key}`, nodeParameters[key] as string); diff --git a/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts index e8ea5cac83..f6f7f07267 100644 --- a/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/composables/useCanvasOperations.ts @@ -911,10 +911,7 @@ export function useCanvasOperations() { options: { viewport?: ViewportBoundaries; forcePosition?: boolean } = {}, ) { const id = node.id ?? nodeHelpers.assignNodeId(node as INodeUi); - const name = - node.name ?? - nodeHelpers.getDefaultNodeName(node) ?? - (nodeTypeDescription.defaults.name as string); + const name = node.name ?? (nodeTypeDescription.defaults.name as string); const type = nodeTypeDescription.name; const typeVersion = node.typeVersion; const position = diff --git a/packages/frontend/editor-ui/src/composables/useNodeHelpers.ts b/packages/frontend/editor-ui/src/composables/useNodeHelpers.ts index 163e42523f..68790af5e9 100644 --- a/packages/frontend/editor-ui/src/composables/useNodeHelpers.ts +++ b/packages/frontend/editor-ui/src/composables/useNodeHelpers.ts @@ -29,7 +29,6 @@ import type { } from 'n8n-workflow'; import type { - AddedNode, ICredentialsResponse, INodeUi, INodeUpdatePropertiesInformation, @@ -987,21 +986,6 @@ export function useNodeHelpers() { return nodeIssues; } - function getDefaultNodeName(node: AddedNode | INode) { - const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion); - if (nodeType === null) return null; - const parameters = NodeHelpers.getNodeParameters( - nodeType?.properties, - node.parameters ?? {}, - true, - false, - node.typeVersion ? { typeVersion: node.typeVersion } : null, - nodeType, - ); - - return NodeHelpers.makeNodeName(parameters ?? {}, nodeType); - } - return { hasProxyAuth, isCustomApiCallSelected, @@ -1035,6 +1019,5 @@ export function useNodeHelpers() { isSingleExecution, getNodeHints, nodeIssuesToString, - getDefaultNodeName, }; } diff --git a/packages/workflow/src/node-helpers.ts b/packages/workflow/src/node-helpers.ts index 0fc0f59cf3..7c48d28722 100644 --- a/packages/workflow/src/node-helpers.ts +++ b/packages/workflow/src/node-helpers.ts @@ -1570,35 +1570,6 @@ export function isNodeWithWorkflowSelector(node: INode) { return [EXECUTE_WORKFLOW_NODE_TYPE, WORKFLOW_TOOL_LANGCHAIN_NODE_TYPE].includes(node.type); } -/** - * @returns An object containing either the resolved operation's action if available, - * else the resource and operation if both exist. - * If neither can be resolved, returns an empty object. - */ -function resolveResourceAndOperation( - nodeParameters: INodeParameters, - nodeTypeDescription: INodeTypeDescription, -) { - const resource = nodeParameters.resource as string; - const operation = nodeParameters.operation as string; - const nodeTypeOperation = nodeTypeDescription.properties.find( - (p) => p.name === 'operation' && p.displayOptions?.show?.resource?.includes(resource), - ); - - if (nodeTypeOperation?.options && isINodePropertyOptionsList(nodeTypeOperation.options)) { - const foundOperation = nodeTypeOperation.options.find((option) => option.value === operation); - if (foundOperation?.action) { - return { action: foundOperation.action }; - } - } - - if (resource && operation) { - return { operation, resource }; - } else { - return {}; - } -} - /** * Generates a human-readable description for a node based on its parameters and type definition. * @@ -1614,93 +1585,28 @@ export function makeDescription( nodeParameters: INodeParameters, nodeTypeDescription: INodeTypeDescription, ): string { - const { action, operation, resource } = resolveResourceAndOperation( - nodeParameters, - nodeTypeDescription, + let description = ''; + const resource = nodeParameters.resource as string; + const operation = nodeParameters.operation as string; + const nodeTypeOperation = nodeTypeDescription.properties.find( + (p) => p.name === 'operation' && p.displayOptions?.show?.resource?.includes(resource), ); - if (action) { - return `${action} in ${nodeTypeDescription.defaults.name}`; - } - - if (resource && operation) { - return `${operation} ${resource} in ${nodeTypeDescription.defaults.name}`; - } - - return nodeTypeDescription.description; -} - -export function isTool( - nodeTypeDescription: INodeTypeDescription, - parameters: INodeParameters, -): boolean { - // Check if node is a vector store in retrieve-as-tool mode - if (nodeTypeDescription.name.includes('vectorStore')) { - const mode = parameters.mode; - return mode === 'retrieve-as-tool'; - } - - // Check for other tool nodes - for (const output of nodeTypeDescription.outputs) { - if (typeof output === 'string') { - return output === NodeConnectionTypes.AiTool; - } else if (output?.type && output.type === NodeConnectionTypes.AiTool) { - return true; + if (nodeTypeOperation?.options && isINodePropertyOptionsList(nodeTypeOperation.options)) { + const foundOperation = nodeTypeOperation.options.find((option) => option.value === operation); + if (foundOperation?.action) { + description = `${foundOperation.action} in ${nodeTypeDescription.defaults.name}`; + return description; } } - return false; -} - -/** - * Generates a resource and operation aware node name. - * - * Appends `in {nodeTypeDisplayName}` if nodeType is a tool - * - * 1. "{action}" if the operation has a defined action - * 2. "{operation} {resource}" if resource and operation exist - * 3. The node type's defaults.name field or displayName as a fallback - */ -export function makeNodeName( - nodeParameters: INodeParameters, - nodeTypeDescription: INodeTypeDescription, -): string { - const { action, operation, resource } = resolveResourceAndOperation( - nodeParameters, - nodeTypeDescription, - ); - - const postfix = isTool(nodeTypeDescription, nodeParameters) - ? ` in ${nodeTypeDescription.defaults.name}` - : ''; - - if (action) { - return `${action}${postfix}`; + if (!description && resource && operation) { + description = `${operation} ${resource} in ${nodeTypeDescription.defaults.name}`; + } else { + description = nodeTypeDescription.description; } - if (resource && operation) { - const operationProper = operation[0].toUpperCase() + operation.slice(1); - return `${operationProper} ${resource}${postfix}`; - } - - return nodeTypeDescription.defaults.name ?? nodeTypeDescription.displayName; -} - -/** - * Returns true if the node name is of format `\d*` , which includes auto-renamed nodes - */ -export function isDefaultNodeName( - name: string, - nodeType: INodeTypeDescription, - parameters: INodeParameters, -): boolean { - const legacyDefaultName = nodeType.defaults.name ?? nodeType.displayName; - const currentDefaultName = makeNodeName(parameters, nodeType); - for (const defaultName of [legacyDefaultName, currentDefaultName]) { - if (name.startsWith(defaultName) && /^\d*$/.test(name.slice(defaultName.length))) return true; - } - - return false; + return description; } /** diff --git a/packages/workflow/test/node-helpers.test.ts b/packages/workflow/test/node-helpers.test.ts index 151ab63486..b6e7d04cfd 100644 --- a/packages/workflow/test/node-helpers.test.ts +++ b/packages/workflow/test/node-helpers.test.ts @@ -18,9 +18,6 @@ import { makeDescription, getUpdatedToolDescription, getToolDescriptionForNode, - isDefaultNodeName, - makeNodeName, - isTool, } from '@/node-helpers'; import type { Workflow } from '@/workflow'; @@ -5249,366 +5246,4 @@ describe('NodeHelpers', () => { expect(result).toBe('This is the default node description'); }); }); - describe('isDefaultNodeName', () => { - let mockNodeTypeDescription: INodeTypeDescription; - - beforeEach(() => { - // Arrange a basic mock node type description - mockNodeTypeDescription = { - displayName: 'Test Node', - name: 'testNode', - icon: 'fa:test', - group: ['transform'], - version: 1, - description: 'This is a test node', - defaults: { - name: 'Test Node', - }, - inputs: ['main'], - outputs: ['main'], - properties: [], - usableAsTool: true, - }; - }); - - it.each([ - ['Create a new user', true], - ['Test Node', true], - ['Test Node1', true], - ['Create a new user5', true], - ['Create a new user in Test Node5', false], - ['Create a new user 5', false], - ['Update user', false], - ['Update user5', false], - ['TestNode', false], - ])('should detect default names for input %s', (input, expected) => { - // Arrange - const name = input; - - mockNodeTypeDescription.properties = [ - { - displayName: 'Operation', - name: 'operation', - type: 'options', - displayOptions: { - show: { - resource: ['user'], - }, - }, - options: [ - { - name: 'Create', - value: 'create', - action: 'Create a new user', - }, - { - name: 'Update', - value: 'update', - action: 'Update a new user', - }, - ], - default: 'create', - }, - ]; - - const parameters: INodeParameters = { - descriptionType: 'manual', - resource: 'user', - operation: 'create', - }; - - // Act - const result = isDefaultNodeName(name, mockNodeTypeDescription, parameters); - - // Assert - expect(result).toBe(expected); - }); - it('should detect default names for tool node types', () => { - // Arrange - const name = 'Create user in Test Node'; - mockNodeTypeDescription.outputs = [NodeConnectionTypes.AiTool]; - - const parameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - // Act - const result = isDefaultNodeName(name, mockNodeTypeDescription, parameters); - - // Assert - expect(result).toBe(true); - }); - it('should detect non-default names for tool node types', () => { - // Arrange - // The default for tools would include ` in Test Node` - const name = 'Create user'; - mockNodeTypeDescription.outputs = [NodeConnectionTypes.AiTool]; - - const parameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - // Act - const result = isDefaultNodeName(name, mockNodeTypeDescription, parameters); - - // Assert - expect(result).toBe(false); - }); - }); - describe('makeNodeName', () => { - let mockNodeTypeDescription: INodeTypeDescription; - - beforeEach(() => { - // Arrange a basic mock node type description - mockNodeTypeDescription = { - displayName: 'Test Node', - name: 'testNode', - icon: 'fa:test', - group: ['transform'], - version: 1, - description: 'This is a test node', - defaults: { - name: 'Test Node', - }, - inputs: ['main'], - outputs: ['main'], - properties: [], - }; - }); - - test('should return action-based name when action is available', () => { - // Arrange - const nodeParameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - mockNodeTypeDescription.properties = [ - { - displayName: 'Operation', - name: 'operation', - type: 'options', - displayOptions: { - show: { - resource: ['user'], - }, - }, - options: [ - { - name: 'Create', - value: 'create', - action: 'Create a new user', - }, - ], - default: 'create', - }, - ]; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Create a new user'); - }); - - test('should return resource-operation-based name when action is not available', () => { - // Arrange - const nodeParameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - mockNodeTypeDescription.properties = [ - { - displayName: 'Operation', - name: 'operation', - type: 'options', - displayOptions: { - show: { - resource: ['user'], - }, - }, - options: [ - { - name: 'Create', - value: 'create', - // No action property - }, - ], - default: 'create', - }, - ]; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Create user'); - }); - - test('should return default name when resource or operation is missing', () => { - // Arrange - const nodeParameters: INodeParameters = { - // No resource or operation - }; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Test Node'); - }); - - test('should handle case where nodeTypeOperation is not found', () => { - // Arrange - const nodeParameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - mockNodeTypeDescription.properties = [ - // No matching operation property - ]; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Create user'); - }); - - test('should handle case where options are not a list of INodePropertyOptions', () => { - // Arrange - const nodeParameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - mockNodeTypeDescription.properties = [ - { - displayName: 'Operation', - name: 'operation', - type: 'options', - displayOptions: { - show: { - resource: ['user'], - }, - }, - // Options are not INodePropertyOptions[] - options: [ - //@ts-expect-error - {}, - ], - default: 'create', - }, - ]; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Create user'); - }); - test('should handle case where node is a tool', () => { - // Arrange - const nodeParameters: INodeParameters = { - resource: 'user', - operation: 'create', - }; - - mockNodeTypeDescription.outputs = [NodeConnectionTypes.AiTool]; - mockNodeTypeDescription.properties = [ - // No matching operation property - ]; - - // Act - const result = makeNodeName(nodeParameters, mockNodeTypeDescription); - - // Assert - expect(result).toBe('Create user in Test Node'); - }); - }); - describe('isTool', () => { - it('should return true for a node with AiTool output', () => { - const description = { - outputs: [NodeConnectionTypes.AiTool], - version: 0, - defaults: { - name: '', - color: '', - }, - inputs: [NodeConnectionTypes.Main], - properties: [], - displayName: '', - group: [], - description: '', - name: 'n8n-nodes-base.someTool', - }; - const parameters = {}; - const result = isTool(description, parameters); - expect(result).toBe(true); - }); - - it('should return true for a node with AiTool output in NodeOutputConfiguration', () => { - const description = { - outputs: [{ type: NodeConnectionTypes.AiTool }, { type: NodeConnectionTypes.Main }], - version: 0, - defaults: { - name: '', - color: '', - }, - inputs: [NodeConnectionTypes.Main], - properties: [], - displayName: '', - group: [], - description: '', - name: 'n8n-nodes-base.someTool', - }; - const parameters = {}; - const result = isTool(description, parameters); - expect(result).toBe(true); - }); - - it('returns true for a vector store node in retrieve-as-tool mode', () => { - const description = { - outputs: [NodeConnectionTypes.Main], - version: 0, - defaults: { - name: '', - color: '', - }, - inputs: [NodeConnectionTypes.Main], - properties: [], - displayName: '', - description: '', - group: [], - name: 'n8n-nodes-base.vectorStore', - }; - const parameters = { mode: 'retrieve-as-tool' }; - const result = isTool(description, parameters); - expect(result).toBe(true); - }); - - it('returns false for node with no AiTool output', () => { - const description = { - outputs: [NodeConnectionTypes.Main], - version: 0, - defaults: { - name: '', - color: '', - }, - inputs: [NodeConnectionTypes.Main], - properties: [], - displayName: '', - group: [], - description: '', - name: 'n8n-nodes-base.someTool', - }; - const parameters = { mode: 'retrieve-as-tool' }; - const result = isTool(description, parameters); - expect(result).toBe(false); - }); - }); });