From 16d59e98edc427bf68edbce4cd2174a44d6dcfb1 Mon Sep 17 00:00:00 2001 From: oleg Date: Wed, 5 Feb 2025 16:45:26 +0100 Subject: [PATCH] fix(editor): Allow to re-open sub-connection node creator if already active (#13041) --- packages/editor-ui/src/Interface.ts | 4 +- .../Node/NodeCreator/Panel/NodesListPanel.vue | 2 + .../NodeCreator/composables/useViewStacks.ts | 92 ++++++++++-------- .../src/stores/nodeCreator.store.test.ts | 97 ++++++++++++++++++- .../editor-ui/src/stores/nodeCreator.store.ts | 15 +-- 5 files changed, 156 insertions(+), 54 deletions(-) diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts index 7753ba8e18..af71d5349c 100644 --- a/packages/editor-ui/src/Interface.ts +++ b/packages/editor-ui/src/Interface.ts @@ -54,6 +54,7 @@ import type { REGULAR_NODE_CREATOR_VIEW, AI_OTHERS_NODE_CREATOR_VIEW, ROLE, + AI_UNCATEGORIZED_CATEGORY, } from '@/constants'; import type { BulkCommand, Undoable } from '@/models/history'; @@ -1012,7 +1013,8 @@ export type NodeFilterType = | typeof REGULAR_NODE_CREATOR_VIEW | typeof TRIGGER_NODE_CREATOR_VIEW | typeof AI_NODE_CREATOR_VIEW - | typeof AI_OTHERS_NODE_CREATOR_VIEW; + | typeof AI_OTHERS_NODE_CREATOR_VIEW + | typeof AI_UNCATEGORIZED_CATEGORY; export type NodeCreatorOpenSource = | '' diff --git a/packages/editor-ui/src/components/Node/NodeCreator/Panel/NodesListPanel.vue b/packages/editor-ui/src/components/Node/NodeCreator/Panel/NodesListPanel.vue index e430ba0ed5..061c6ea244 100644 --- a/packages/editor-ui/src/components/Node/NodeCreator/Panel/NodesListPanel.vue +++ b/packages/editor-ui/src/components/Node/NodeCreator/Panel/NodesListPanel.vue @@ -6,6 +6,7 @@ import { AI_NODE_CREATOR_VIEW, REGULAR_NODE_CREATOR_VIEW, TRIGGER_NODE_CREATOR_VIEW, + AI_UNCATEGORIZED_CATEGORY, } from '@/constants'; import { useNodeCreatorStore } from '@/stores/nodeCreator.store'; @@ -95,6 +96,7 @@ watch( [REGULAR_NODE_CREATOR_VIEW]: RegularView, [AI_NODE_CREATOR_VIEW]: AIView, [AI_OTHERS_NODE_CREATOR_VIEW]: AINodesView, + [AI_UNCATEGORIZED_CATEGORY]: AINodesView, }; const itemKey = selectedView; diff --git a/packages/editor-ui/src/components/Node/NodeCreator/composables/useViewStacks.ts b/packages/editor-ui/src/components/Node/NodeCreator/composables/useViewStacks.ts index c57d7f5464..7ec71cc840 100644 --- a/packages/editor-ui/src/components/Node/NodeCreator/composables/useViewStacks.ts +++ b/packages/editor-ui/src/components/Node/NodeCreator/composables/useViewStacks.ts @@ -313,50 +313,54 @@ export const useViewStacks = defineStore('nodeCreatorViewStacks', () => { } await nextTick(); - pushViewStack({ - title: relatedAIView?.properties.title, - ...extendedInfo, - rootView: AI_OTHERS_NODE_CREATOR_VIEW, - mode: 'nodes', - items: nodeCreatorStore.allNodeCreatorNodes, - nodeIcon: { - iconType: 'icon', - icon: relatedAIView?.properties.icon, - color: relatedAIView?.properties.iconProps?.color, - }, - panelClass: relatedAIView?.properties.panelClass, - baseFilter: (i: INodeCreateElement) => { - // AI Code node could have any connection type so we don't want to display it - // in the compatible connection view as it would be displayed in all of them - if (i.key === AI_CODE_NODE_TYPE) return false; - const displayNode = nodesByConnectionType[connectionType].includes(i.key); - // TODO: Filtering works currently fine for displaying compatible node when dropping - // input connections. However, it does not work for output connections. - // For that reason does it currently display nodes that are maybe not compatible - // but then errors once it got selected by the user. - if (displayNode && filter?.nodes?.length) { - return filter.nodes.includes(i.key); - } + pushViewStack( + { + title: relatedAIView?.properties.title, + ...extendedInfo, + rootView: AI_OTHERS_NODE_CREATOR_VIEW, + mode: 'nodes', + items: nodeCreatorStore.allNodeCreatorNodes, + nodeIcon: { + iconType: 'icon', + icon: relatedAIView?.properties.icon, + color: relatedAIView?.properties.iconProps?.color, + }, + panelClass: relatedAIView?.properties.panelClass, + baseFilter: (i: INodeCreateElement) => { + // AI Code node could have any connection type so we don't want to display it + // in the compatible connection view as it would be displayed in all of them + if (i.key === AI_CODE_NODE_TYPE) return false; + const displayNode = nodesByConnectionType[connectionType].includes(i.key); - return displayNode; + // TODO: Filtering works currently fine for displaying compatible node when dropping + // input connections. However, it does not work for output connections. + // For that reason does it currently display nodes that are maybe not compatible + // but then errors once it got selected by the user. + if (displayNode && filter?.nodes?.length) { + return filter.nodes.includes(i.key); + } + + return displayNode; + }, + itemsMapper(item) { + return { + ...item, + subcategory: connectionType, + }; + }, + actionsFilter: (items: ActionTypeDescription[]) => { + // Filter out actions that are not compatible with the connection type + if (items.some((item) => item.outputConnectionType)) { + return items.filter((item) => item.outputConnectionType === connectionType); + } + return items; + }, + hideActions: true, + preventBack: true, }, - itemsMapper(item) { - return { - ...item, - subcategory: connectionType, - }; - }, - actionsFilter: (items: ActionTypeDescription[]) => { - // Filter out actions that are not compatible with the connection type - if (items.some((item) => item.outputConnectionType)) { - return items.filter((item) => item.outputConnectionType === connectionType); - } - return items; - }, - hideActions: true, - preventBack: true, - }); + { resetStacks: true }, + ); } function setStackBaselineItems() { @@ -417,7 +421,11 @@ export const useViewStacks = defineStore('nodeCreatorViewStacks', () => { })); } - function pushViewStack(stack: ViewStack) { + function pushViewStack(stack: ViewStack, options: { resetStacks?: boolean } = {}) { + if (options.resetStacks) { + resetViewStacks(); + } + if (activeViewStack.value.uuid) { updateCurrentViewStack({ activeIndex: getActiveItemIndex() }); } diff --git a/packages/editor-ui/src/stores/nodeCreator.store.test.ts b/packages/editor-ui/src/stores/nodeCreator.store.test.ts index b59d763e97..30ef31a60a 100644 --- a/packages/editor-ui/src/stores/nodeCreator.store.test.ts +++ b/packages/editor-ui/src/stores/nodeCreator.store.test.ts @@ -1,8 +1,15 @@ import { createPinia, setActivePinia } from 'pinia'; import { useNodeCreatorStore } from './nodeCreator.store'; import { useTelemetry } from '@/composables/useTelemetry'; -import { CUSTOM_API_CALL_KEY, REGULAR_NODE_CREATOR_VIEW } from '@/constants'; +import { + AI_UNCATEGORIZED_CATEGORY, + CUSTOM_API_CALL_KEY, + REGULAR_NODE_CREATOR_VIEW, +} from '@/constants'; import type { INodeCreateElement } from '@/Interface'; +import { parseCanvasConnectionHandleString } from '@/utils/canvasUtilsV2'; +import { NodeConnectionType } from 'n8n-workflow'; +import { CanvasConnectionMode } from '@/types'; const workflow_id = 'workflow-id'; const category_name = 'category-name'; @@ -29,6 +36,31 @@ vi.mock('@/composables/useTelemetry', () => { }; }); +// Mock the workflows store so that getNodeById returns a dummy node. +vi.mock('@/stores/workflows.store', () => { + return { + useWorkflowsStore: () => ({ + getNodeById: vi.fn((id?: string) => { + return id ? { id, name: 'Test Node' } : null; + }), + workflowTriggerNodes: [], + workflowId: 'dummy-workflow-id', + getCurrentWorkflow: vi.fn(() => ({ + getNode: vi.fn(() => ({ + type: 'n8n-node.example', + typeVersion: 1, + })), + })), + }), + }; +}); + +vi.mock('@/utils/canvasUtilsV2', () => { + return { + parseCanvasConnectionHandleString: vi.fn(), + }; +}); + describe('useNodeCreatorStore', () => { let nodeCreatorStore: ReturnType; @@ -290,6 +322,69 @@ describe('useNodeCreatorStore', () => { }, ); }); + describe('selective connection view', () => { + const mockedParseCanvasConnectionHandleString = vi.mocked( + parseCanvasConnectionHandleString, + true, + ); + + it('sets nodeCreatorView to AI_UNCATEGORIZED_CATEGORY when connection type is not Main', async () => { + mockedParseCanvasConnectionHandleString.mockReturnValue({ + type: NodeConnectionType.AiLanguageModel, // any value that is not NodeConnectionType.Main + index: 0, + mode: CanvasConnectionMode.Input, + }); + + const connection = { + source: 'node-1', + sourceHandle: 'fake-handle', + }; + + nodeCreatorStore.openNodeCreatorForConnectingNode({ + connection, + eventSource: 'plus_endpoint', + nodeCreatorView: REGULAR_NODE_CREATOR_VIEW, + }); + + expect(nodeCreatorStore.selectedView).toEqual(AI_UNCATEGORIZED_CATEGORY); + }); + + it('uses the provided nodeCreatorView when connection type is Main', async () => { + mockedParseCanvasConnectionHandleString.mockReturnValue({ + type: NodeConnectionType.Main, + index: 0, + mode: CanvasConnectionMode.Input, + }); + + const connection = { + source: 'node-2', + sourceHandle: 'fake-handle-main', + }; + + nodeCreatorStore.openNodeCreatorForConnectingNode({ + connection, + eventSource: 'plus_endpoint', + nodeCreatorView: REGULAR_NODE_CREATOR_VIEW, + }); + + expect(nodeCreatorStore.selectedView).toEqual(REGULAR_NODE_CREATOR_VIEW); + }); + + it('does not update state if no source node is found', async () => { + const connection = { + source: '', + sourceHandle: 'any-handle', + }; + + nodeCreatorStore.openNodeCreatorForConnectingNode({ + connection, + eventSource: 'plus_endpoint', + nodeCreatorView: REGULAR_NODE_CREATOR_VIEW, + }); + + expect(nodeCreatorStore.selectedView).not.toEqual(REGULAR_NODE_CREATOR_VIEW); + }); + }); }); function getSessionId(time: number) { diff --git a/packages/editor-ui/src/stores/nodeCreator.store.ts b/packages/editor-ui/src/stores/nodeCreator.store.ts index 05dd4a9795..e0c4320245 100644 --- a/packages/editor-ui/src/stores/nodeCreator.store.ts +++ b/packages/editor-ui/src/stores/nodeCreator.store.ts @@ -2,6 +2,7 @@ import { defineStore } from 'pinia'; import { AI_NODE_CREATOR_VIEW, AI_OTHERS_NODE_CREATOR_VIEW, + AI_UNCATEGORIZED_CATEGORY, CUSTOM_API_CALL_KEY, NODE_CREATOR_OPEN_SOURCES, REGULAR_NODE_CREATOR_VIEW, @@ -20,7 +21,7 @@ import type { import { computed, ref } from 'vue'; import { transformNodeType } from '@/components/Node/NodeCreator/utils'; import type { IDataObject, INodeInputConfiguration, NodeParameterValueType } from 'n8n-workflow'; -import { NodeConnectionType, nodeConnectionTypes, NodeHelpers } from 'n8n-workflow'; +import { NodeConnectionType, NodeHelpers } from 'n8n-workflow'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { useUIStore } from '@/stores/ui.store'; import { useNDVStore } from '@/stores/ndv.store'; @@ -121,10 +122,6 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, () => { createNodeActive, nodeCreatorView, }: ToggleNodeCreatorOptions) { - if (createNodeActive === isCreateNodeActive.value) { - return; - } - if (!nodeCreatorView) { nodeCreatorView = workflowsStore.workflowTriggerNodes.length > 0 @@ -183,18 +180,16 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, () => { uiStore.lastInteractedWithNodeHandle = connection.sourceHandle ?? null; uiStore.lastInteractedWithNodeId = sourceNode.id; + const isOutput = mode === CanvasConnectionMode.Output; + const isScopedConnection = type !== NodeConnectionType.Main; setNodeCreatorState({ source: eventSource, createNodeActive: true, - nodeCreatorView, + nodeCreatorView: isScopedConnection ? AI_UNCATEGORIZED_CATEGORY : nodeCreatorView, }); // TODO: The animation is a bit glitchy because we're updating view stack immediately // after the node creator is opened - const isOutput = mode === CanvasConnectionMode.Output; - const isScopedConnection = - type !== NodeConnectionType.Main && nodeConnectionTypes.includes(type); - if (isScopedConnection) { useViewStacks() .gotoCompatibleConnectionView(type, isOutput, getNodeCreatorFilter(sourceNode.name, type))