From 06c61d94374e3d1db84cd803de63aadc3d69f856 Mon Sep 17 00:00:00 2001 From: Daria Date: Mon, 28 Sep 2026 12:57:32 +0000 Subject: [PATCH] fix(editor): Empty Group Last Refinements (no-changelog) (#39632) --- .../composables/useCanvasOperations.test.ts | 41 +++++++++ .../app/composables/useCanvasOperations.ts | 61 ++++++++------ .../editor-ui/src/app/views/NodeView.test.ts | 82 ++++++++++++++++++ .../editor-ui/src/app/views/NodeView.vue | 4 +- .../components/Modes/NodesMode.test.ts | 23 ++++- .../components/Modes/NodesMode.vue | 10 ++- .../components/Panel/NodesListPanel.test.ts | 37 ++++++++ .../components/Panel/NodesListPanel.vue | 4 +- .../shared/nodeCreator/nodeCreator.utils.ts | 10 ++- .../features/workflows/canvas/canvas.types.ts | 2 + .../canvas/components/Canvas.test.ts | 62 ++++++++++++++ .../workflows/canvas/components/Canvas.vue | 84 ++++++++++++++----- .../edges/CanvasConnectionLine.test.ts | 39 +++++++++ .../elements/edges/CanvasConnectionLine.vue | 11 ++- .../elements/edges/utils/getEdgeRenderData.ts | 9 +- .../groups/CanvasNodeGroupTitleBar.test.ts | 36 +++++++- .../handles/CanvasHandleRenderer.test.ts | 42 +++++++++- .../elements/handles/CanvasHandleRenderer.vue | 13 ++- .../useCanvasNodeGroupOperationGuards.test.ts | 83 +++++++++++++++++- .../useCanvasNodeGroupOperationGuards.ts | 10 ++- 20 files changed, 587 insertions(+), 76 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts index dfce0e5bcfa..59b1711fab0 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts @@ -5314,6 +5314,47 @@ describe('useCanvasOperations', () => { expect(useClipboard().copy).toHaveBeenCalledTimes(1); expect(vi.mocked(useClipboard().copy).mock.calls).toMatchSnapshot(); }); + + it('does not restore an anchor when cutting an explicitly selected group', async () => { + const nodeTypesStore = useNodeTypesStore(); + const nodeTypeDescription = mockNodeTypeDescription({ name: SET_NODE_TYPE }); + nodeTypesStore.nodeTypes = { + [SET_NODE_TYPE]: { 1: nodeTypeDescription }, + }; + + const node = createTestNode({ id: 'group-member', type: SET_NODE_TYPE }); + const group: IWorkflowGroup = { id: 'group-1', name: 'Group 1', nodeIds: [node.id] }; + const nodesById = new Map([[node.id, node]]); + workflowDocumentStoreInstance.allNodes = [node]; + workflowDocumentStoreInstance.connectionsBySourceNode = {}; + vi.spyOn(workflowDocumentStoreInstance, 'getNodesByIds').mockReturnValue([node]); + vi.spyOn(workflowDocumentStoreInstance, 'getNodeById').mockImplementation((id) => + nodesById.get(id), + ); + vi.spyOn(workflowDocumentStoreInstance, 'getNodeByName').mockImplementation( + (name) => [...nodesById.values()].find((candidate) => candidate.name === name) ?? null, + ); + vi.spyOn(workflowDocumentStoreInstance, 'getGroupForNode').mockImplementation((id) => + group.nodeIds.includes(id) ? group : undefined, + ); + vi.spyOn(workflowDocumentStoreInstance, 'getGroupById').mockReturnValue(undefined); + vi.spyOn(workflowDocumentStoreInstance, 'getParentNodes').mockReturnValue([]); + vi.spyOn(workflowDocumentStoreInstance, 'getChildNodes').mockReturnValue([]); + vi.spyOn(workflowDocumentStoreInstance, 'getConnectionsBetweenNodes').mockReturnValue([]); + vi.spyOn(workflowDocumentStoreInstance, 'removeNodeById').mockImplementation((id) => { + nodesById.delete(id); + workflowDocumentStoreInstance.allNodes = [...nodesById.values()]; + }); + vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({}); + vi.mocked(workflowDocumentStoreInstance.outgoingConnectionsByNodeName).mockReturnValue({}); + + const { cutNodes } = useCanvasOperations(); + await cutNodes([node.id], [group.id]); + + expect(workflowDocumentStoreInstance.addNode).not.toHaveBeenCalled(); + expect(workflowDocumentStoreInstance.removeNodeById).toHaveBeenCalledWith(node.id); + expect(nodesById.size).toBe(0); + }); }); describe('resolveNodeWebhook', () => { diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 1decab4ed9c..8f93da3c626 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -983,7 +983,10 @@ export function useCanvasOperations() { function replaceGroupedNodeConnections( previousNode: INodeUi, newNode: INodeUi, - { trackHistory = false } = {}, + { + trackHistory = false, + additionalGroupNodeIds = [], + }: { trackHistory?: boolean; additionalGroupNodeIds?: string[] } = {}, ): boolean { const group = workflowDocumentStore.value.getGroupForNode(previousNode.id); if (!group) return false; @@ -1018,6 +1021,7 @@ export function useCanvasOperations() { nodeIds, connectionsToRemove, connectionsToAdd, + additionalGroupNodeIds, connectionsBySourceNode: workflowDocumentStore.value.connectionsBySourceNode, }); if (!isReplacementAllowed) return false; @@ -3692,10 +3696,10 @@ export function useCanvasOperations() { return true; } - async function cutNodes(ids: string[]) { + async function cutNodes(ids: string[], deleteWholeGroupIds: string[] = []) { if (!(await copyNodes(ids))) return; - deleteNodes(ids); + deleteNodes(ids, { deleteWholeGroupIds }); } async function openExecution(executionId: string, nodeId?: string) { @@ -3821,7 +3825,11 @@ export function useCanvasOperations() { function replaceNode( previousId: string, newId: string, - { trackHistory = true, trackBulk = true } = {}, + { + trackHistory = true, + trackBulk = true, + additionalGroupNodeIds = [], + }: { trackHistory?: boolean; trackBulk?: boolean; additionalGroupNodeIds?: string[] } = {}, ) { const previousNode = workflowDocumentStore.value.getNodeById(previousId); const newNode = workflowDocumentStore.value.getNodeById(newId); @@ -3841,6 +3849,7 @@ export function useCanvasOperations() { if (previousGroup) { const didReplaceConnections = replaceGroupedNodeConnections(previousNode, newNode, { trackHistory, + additionalGroupNodeIds, }); if (!didReplaceConnections) { if (trackHistory && trackBulk) { @@ -3909,26 +3918,6 @@ export function useCanvasOperations() { addedNodesByInputIndex, }); - let replacementGroupId: string | undefined; - if (addedNodes.length > 0 && options.replaceNodeId) { - // Auto-added helpers can follow the node that the user selected, so they - // must not become the replacement target. - const replacementNodeIndex = nodes.findLastIndex((node) => !node.isAutoAdd); - const replacementNode = - replacementNodeIndex === -1 - ? addedNodes.at(-1) - : addedNodesByInputIndex.get(replacementNodeIndex); - if (replacementNode) { - const didReplace = replaceNode(options.replaceNodeId, replacementNode.id, { - trackHistory, - trackBulk: false, - }); - if (didReplace) { - replacementGroupId = workflowDocumentStore.value.getGroupForNode(replacementNode.id)?.id; - } - } - } - const connections: CanvasConnectionCreateData[] = addedConnections.flatMap(({ from, to }) => { const fromNode = addedNodesByInputIndex.get(from.nodeIndex); const toNode = addedNodesByInputIndex.get(to.nodeIndex); @@ -3966,6 +3955,30 @@ export function useCanvasOperations() { await addConnections(connections, { trackHistory, trackBulk: false }); + let replacementGroupId: string | undefined; + if (addedNodes.length > 0 && options.replaceNodeId) { + // Auto-added helpers can follow the node that the user selected, so they + // must not become the replacement target. + const replacementNodeIndex = nodes.findLastIndex((node) => !node.isAutoAdd); + const replacementNode = + replacementNodeIndex === -1 + ? addedNodes.at(-1) + : addedNodesByInputIndex.get(replacementNodeIndex); + if (replacementNode) { + const didReplace = replaceNode(options.replaceNodeId, replacementNode.id, { + trackHistory, + trackBulk: false, + // The batch connections are already part of the candidate graph. Let + // grouped replacement validate the whole batch as one unit before the + // remaining batch nodes are added to the group below. + additionalGroupNodeIds: addedNodes.map((node) => node.id), + }); + if (didReplace) { + replacementGroupId = workflowDocumentStore.value.getGroupForNode(replacementNode.id)?.id; + } + } + } + if (replacementGroupId) { const groupBeforeExtend = workflowDocumentStore.value.getGroupById(replacementGroupId); if (groupBeforeExtend) { diff --git a/packages/frontend/editor-ui/src/app/views/NodeView.test.ts b/packages/frontend/editor-ui/src/app/views/NodeView.test.ts index 95b685a9ee0..1252631c4b8 100644 --- a/packages/frontend/editor-ui/src/app/views/NodeView.test.ts +++ b/packages/frontend/editor-ui/src/app/views/NodeView.test.ts @@ -683,6 +683,88 @@ describe('NodeView', () => { expect(workflowDocumentStore.allNodes[0].id).toBe(anchorId); expect(workflowDocumentStore.allGroups[0].nodeIds).toEqual([anchorId]); }); + + it('keeps internal replacement-batch connections when replacing a node in a regular group', async () => { + routeMock.meta = { nodeView: true }; + useWorkflowsListStore().addWorkflow( + createTestWorkflow({ id: 'w0', scopes: ['workflow:read', 'workflow:update'] }), + ); + addReplacementNodeTypes(); + const source = createTestNode({ + id: 'source', + name: 'Source', + type: SPLIT_IN_BATCHES_NODE_TYPE, + position: [0, 0], + }); + const target = createTestNode({ + id: 'target', + name: 'Target', + type: SPLIT_IN_BATCHES_NODE_TYPE, + position: [300, 0], + }); + const next = createTestNode({ + id: 'next', + name: 'Next', + type: SPLIT_IN_BATCHES_NODE_TYPE, + position: [600, 0], + }); + workflowDocumentStore.setNodes([source, target, next]); + workflowDocumentStore.setConnections({ + Source: { + main: [[{ node: 'Target', type: 'main', index: 0 }]], + }, + Target: { + main: [[{ node: 'Next', type: 'main', index: 0 }]], + }, + }); + const group = workflowDocumentStore.createGroup([source.id, target.id, next.id], 'Group 1'); + const operationOrder: string[] = []; + const addConnectionToDocument = + workflowDocumentStore.addConnection.bind(workflowDocumentStore); + const addConnection = vi.spyOn(workflowDocumentStore, 'addConnection'); + addConnection.mockImplementation((args) => { + const [from, to] = args.connection; + operationOrder.push(`${from?.node}->${to?.node}`); + return addConnectionToDocument(args); + }); + const replaceNodeInGroupInDocument = + workflowDocumentStore.replaceNodeInGroup.bind(workflowDocumentStore); + const replaceNodeInGroup = vi.spyOn(workflowDocumentStore, 'replaceNodeInGroup'); + replaceNodeInGroup.mockImplementation((groupId, previousNodeId, newNodeId) => { + operationOrder.push('replace-node-in-group'); + return replaceNodeInGroupInDocument(groupId, previousNodeId, newNodeId); + }); + const { findByTestId } = renderNodeView(); + + await userEvent.click(await findByTestId('canvas-stub-replace-first')); + await userEvent.click(await findByTestId('node-creation-stub-add-loop-replacement')); + + await waitFor(() => expect(workflowDocumentStore.allNodes).toHaveLength(4)); + const loop = workflowDocumentStore.allNodes.find((node) => node.name === 'Loop Over Items'); + const helper = workflowDocumentStore.allNodes.find((node) => node.name === 'Replace Me'); + expect(loop).toBeDefined(); + expect(helper).toBeDefined(); + const groupNodeIds = workflowDocumentStore.getGroupById(group.id)?.nodeIds ?? []; + expect(groupNodeIds).toHaveLength(4); + expect(new Set(groupNodeIds)).toEqual(new Set([loop?.id, helper?.id, target.id, next.id])); + const internalConnectionIndex = operationOrder.findIndex( + (entry) => entry.includes('Loop Over Items') && entry.includes('Replace Me'), + ); + expect(internalConnectionIndex, JSON.stringify(operationOrder)).toBeLessThan( + operationOrder.indexOf('replace-node-in-group'), + ); + expect(workflowDocumentStore.connectionsBySourceNode).toMatchObject({ + 'Loop Over Items': { + main: expect.arrayContaining([ + [{ node: 'Target', type: 'main', index: 0 }], + [{ node: 'Replace Me', type: 'main', index: 0 }], + ]), + }, + 'Replace Me': { + main: expect.arrayContaining([[{ node: 'Loop Over Items', type: 'main', index: 0 }]]), + }, + }); + }); }); describe('Trigger node selection', () => { diff --git a/packages/frontend/editor-ui/src/app/views/NodeView.vue b/packages/frontend/editor-ui/src/app/views/NodeView.vue index 0cc12502b86..4124dd48e22 100644 --- a/packages/frontend/editor-ui/src/app/views/NodeView.vue +++ b/packages/frontend/editor-ui/src/app/views/NodeView.vue @@ -692,11 +692,11 @@ async function onClipboardPaste(plainTextData: string): Promise { await mcpJsonNudgeTrigger.gate('paste', paste); } -async function onCutNodes(ids: string[]) { +async function onCutNodes(ids: string[], deleteWholeGroupIds: string[] = []) { if (isCanvasReadOnly.value) { await copyNodes(ids); } else { - await cutNodes(ids); + await cutNodes(ids, deleteWholeGroupIds); } } diff --git a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.test.ts b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.test.ts index 665c9fbf29d..831e3f83aa8 100644 --- a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.test.ts +++ b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.test.ts @@ -19,6 +19,7 @@ import { import type { CommandCreateElement, NodeCreateElement } from '@/Interface'; import { useViewStacks } from '@/features/shared/nodeCreator/composables/useViewStacks'; import { useKeyboardNavigation } from '@/features/shared/nodeCreator/composables/useKeyboardNavigation'; +import { useNodeCreatorStore } from '@/features/shared/nodeCreator/nodeCreator.store'; import { createComponentRenderer } from '@/__tests__/render'; import { waitAllPromises } from '@n8n/frontend-test-utils'; import { mockRestrictedNodeTypes } from '@/__tests__/mocks'; @@ -298,7 +299,7 @@ describe('NodesMode', () => { }); it.each([TRIGGER_NODE_CREATOR_VIEW, REGULAR_NODE_CREATOR_VIEW] as const)( - 'does not render or select the Group item when empty groups are disabled in the %s view', + 'does not render the Group item when empty groups are disabled in the %s view', async (rootView) => { mockIsFeatureEnabled.mockReturnValue(false); useViewStacks().pushViewStack({ @@ -309,15 +310,29 @@ describe('NodesMode', () => { items: [groupCommandElement()], }); - const { emitted } = render({ pinia }); + render({ pinia }); await nextTick(); expect(screen.queryByText('Group')).not.toBeInTheDocument(); - document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); - expect(emitted('emptyGroupSelected')).toBeUndefined(); }, ); + it('does not render the Group item during node replacement', async () => { + useNodeCreatorStore().openingContext = 'replacement'; + useViewStacks().pushViewStack({ + title: 'Replace node', + mode: 'nodes', + rootView: REGULAR_NODE_CREATOR_VIEW, + hasSearch: true, + items: [groupCommandElement()], + }); + + render({ pinia }); + await nextTick(); + + expect(screen.queryByText('Group')).not.toBeInTheDocument(); + }); + it.each(['group', 'organisational', 'container'])( 'shows the Group item when searching for %s', async (search) => { diff --git a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.vue b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.vue index 4369c15d003..d2d04f62559 100644 --- a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.vue +++ b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Modes/NodesMode.vue @@ -72,7 +72,8 @@ const i18n = useI18n(); const { isRagStarterCalloutVisible, openSampleWorkflowTemplate } = useCalloutHelpers(); -const { mergedNodes, actions, onSubcategorySelected } = useNodeCreatorStore(); +const nodeCreatorStore = useNodeCreatorStore(); +const { mergedNodes, actions, onSubcategorySelected } = nodeCreatorStore; const { pushViewStack, popViewStack, isAiSubcategoryView, isHitlSubcategoryView } = useViewStacks(); const { setAddedNodeActionParameters, nodeCreateElementToNodeTypeSelectedPayload } = useActions(); const emptyCanvasGroupsEnabled = useEmptyCanvasGroupsFlag(); @@ -80,9 +81,12 @@ const emptyCanvasGroupsEnabled = useEmptyCanvasGroupsFlag(); const { registerKeyHook } = useKeyboardNavigation(); const activeViewStack = computed(() => useViewStacks().activeViewStack); +const shouldHideEmptyGroupCommand = computed( + () => !emptyCanvasGroupsEnabled.value || nodeCreatorStore.openingContext === 'replacement', +); const visibleItems = computed(() => { const items = activeViewStack.value.items ?? []; - if (emptyCanvasGroupsEnabled.value) return items; + if (!shouldHideEmptyGroupCommand.value) return items; return items.filter((item) => item.key !== ADD_EMPTY_GROUP_NODE_CREATOR_ITEM); }); @@ -137,7 +141,7 @@ function getFilteredActions( } function onSelected(item: INodeCreateElement) { - if (item.key === ADD_EMPTY_GROUP_NODE_CREATOR_ITEM && !emptyCanvasGroupsEnabled.value) return; + if (item.key === ADD_EMPTY_GROUP_NODE_CREATOR_ITEM && shouldHideEmptyGroupCommand.value) return; // Insertion itself is refused in getAddedNodesAndConnections; this keeps a restricted // node from opening its actions view. diff --git a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.test.ts b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.test.ts index 29e4a4145b8..e68a2757bb8 100644 --- a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.test.ts +++ b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.test.ts @@ -365,6 +365,43 @@ describe('NodesListPanel', () => { expect(screen.getByText('Group')).toBeInTheDocument(); }); + + it('keeps Group hidden while navigating in replacement mode', async () => { + getWrapperComponent(() => { + const source = createTestNode({ id: 'source', name: 'Source' }); + const workflowDocumentStore = useWorkflowDocumentStore(createWorkflowDocumentId('')); + workflowDocumentStore.setNodes([source]); + + const nodeCreatorStore = useNodeCreatorStore(); + nodeCreatorStore.openingContext = 'replacement'; + nodeCreatorStore.setSelectedView(REGULAR_NODE_CREATOR_VIEW); + }); + await nextTick(); + + expect(screen.queryByText('Group')).not.toBeInTheDocument(); + + await fireEvent.click(screen.getByText('Action in an app')); + await nextTick(); + await fireEvent.click(document.querySelector('.backButton')!); + await waitFor(() => expect(screen.getByTestId('node-creator-search-bar')).toBeVisible()); + expect(useNodeCreatorStore().openingContext).toBe('replacement'); + expect(screen.queryByText('Group')).not.toBeInTheDocument(); + }); + + it('clears replacement context when the panel is closed', async () => { + const { unmount } = getWrapperComponent(() => { + const nodeCreatorStore = useNodeCreatorStore(); + nodeCreatorStore.openingContext = 'replacement'; + nodeCreatorStore.setSelectedView(REGULAR_NODE_CREATOR_VIEW); + }); + await nextTick(); + + const nodeCreatorStore = useNodeCreatorStore(); + expect(nodeCreatorStore.openingContext).toBe('replacement'); + + unmount(); + expect(nodeCreatorStore.openingContext).toBeNull(); + }); }); // Reproduces ADO-5590 (GH #33955): typing in the node-creator search field diff --git a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.vue b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.vue index 7e09ad4f911..657e59cfadc 100644 --- a/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.vue +++ b/packages/frontend/editor-ui/src/features/shared/nodeCreator/components/Panel/NodesListPanel.vue @@ -184,7 +184,9 @@ function flushPendingSearchOnNavigation(event: KeyboardEvent) { } function onTransitionEnd() { - cleanupopeningContext(); + if (viewStacks.value.length === 0) { + cleanupopeningContext(); + } void setActiveItemIndex(getDefaultActiveIndex()); } diff --git a/packages/frontend/editor-ui/src/features/shared/nodeCreator/nodeCreator.utils.ts b/packages/frontend/editor-ui/src/features/shared/nodeCreator/nodeCreator.utils.ts index c0435df253f..a66668b8bcc 100644 --- a/packages/frontend/editor-ui/src/features/shared/nodeCreator/nodeCreator.utils.ts +++ b/packages/frontend/editor-ui/src/features/shared/nodeCreator/nodeCreator.utils.ts @@ -1,6 +1,7 @@ import type { ActionCreateElement, ActionTypeDescription, + CommandCreateElement, INodeCreateElement, LinkCreateElement, NodeCreateElement, @@ -61,6 +62,10 @@ const NODE_CREATOR_COMMAND_SEARCH_KEYS = [ { key: 'properties.description', weight: 0.8 }, ]; +function isCommandCreateElement(item: { type: string }): item is CommandCreateElement { + return item.type === 'command'; +} + export function transformNodeType( node: SimplifiedNodeType, subcategory?: string, @@ -91,10 +96,7 @@ export function getNodeCreatorSearchItems( nodes: SimplifiedNodeType[], items: ReadonlyArray<{ type: string }>, ): INodeCreateElement[] { - return [ - ...nodes.map((node) => transformNodeType(node)), - ...(items.filter((item) => item.type === 'command') as INodeCreateElement[]), - ]; + return [...nodes.map((node) => transformNodeType(node)), ...items.filter(isCommandCreateElement)]; } export function subcategorizeItems(items: SimplifiedNodeType[]) { diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts index 26f23b25297..a5150eb7c63 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/canvas.types.ts @@ -317,6 +317,8 @@ export interface CanvasNodeHandleInjectionData { export type ConnectStartEvent = { event?: MouseEvent | undefined; + /** True when a collapsed empty group's left input started the drag. */ + isEmptyGroupTargetStart?: boolean; } & OnConnectStartParams; export type CanvasNodeMoveEvent = { id: string; position: CanvasNode['position'] }; diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.test.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.test.ts index 7078d558506..af8cb0570a6 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.test.ts @@ -1721,6 +1721,53 @@ describe('Canvas', () => { }); describe('group selection reconciliation', () => { + it('replaces the node clicked inside a fully selected regular group', async () => { + workflowDocumentStore.setScopes(['workflow:update']); + workflowDocumentStore.setNodes([ + createTestNode({ id: 'a', name: 'Node A' }), + createTestNode({ id: 'b', name: 'Node B' }), + ]); + const group = workflowDocumentStore.createGroup(['a', 'b'], 'My Group'); + const memberA = createCanvasNodeElement({ id: 'a', label: 'Node A' }); + const memberB = createCanvasNodeElement({ + id: 'b', + label: 'Node B', + position: { x: 300, y: 100 }, + }); + const rendered = renderComponent({ + props: { + nodes: [ + createCanvasGroupElement({ id: group.id, name: group.name, nodeIds: ['a', 'b'] }), + memberA, + memberB, + ], + }, + global: { + provide: { [NodeGroupViewKey as symbol]: createNodeGroupViewMock(false) }, + }, + }); + await waitFor(() => + expect(rendered.container.querySelectorAll('.vue-flow__node')).toHaveLength(3), + ); + + const vueFlow = useVueFlow(canvasId); + vueFlow.addSelectedNodes([ + vueFlow.findNode(`group:${group.id}`)!, + vueFlow.findNode('a')!, + vueFlow.findNode('b')!, + ]); + await waitFor(() => expect(vueFlow.getSelectedNodes.value).toHaveLength(3)); + + await fireEvent.click(rendered.container.querySelector('[data-id="b"]')!); + await waitFor(() => + expect(vueFlow.getSelectedNodes.value.map(({ id }) => id)).toEqual(['b']), + ); + await fireEvent.keyDown(document, { key: 'r' }); + await fireEvent.keyUp(document, { key: 'r' }); + + expect(rendered.emitted()['replace:node']).toEqual([['b']]); + }); + it('folds the selection into the group when one is created around fully selected nodes', async () => { workflowDocumentStore.setScopes(['workflow:update']); workflowDocumentStore.setNodes([ @@ -1959,6 +2006,21 @@ describe('Canvas', () => { expect(emitted()['copy:nodes']).toEqual([[['a', 'b']]]); }); + it('passes explicitly selected groups through the cut action', async () => { + const { group, groupNode, emitted } = await renderWithGroup(); + const { addSelectedNodes, findNode } = useVueFlow(canvasId); + addSelectedNodes([findNode(groupNode.id)!]); + await waitFor(() => expect(findNode(groupNode.id)?.selected).toBe(true)); + + await fireEvent.keyDown(document, { key: 'x', ctrlKey: true, metaKey: true }); + await fireEvent.keyUp(document, { key: 'x', ctrlKey: true, metaKey: true }); + + const cutEvent = emitted()['cut:nodes']?.at(-1) as [string[], string[]?] | undefined; + const [ids, deleteWholeGroupIds] = cutEvent ?? []; + expect(ids).toEqual(['a', 'b']); + expect(deleteWholeGroupIds).toEqual([group.id]); + }); + it('opens the group context menu on a read-only canvas, like node menus, with mutating items disabled', async () => { // The workflow itself stays editable (see renderWithGroup) — the // canvas prop alone must disable the mutating items, so embedded diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue index 8271bcaf2db..8ea6c3fdba9 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/Canvas.vue @@ -37,6 +37,7 @@ import type { } from '../canvas.types'; import { CanvasNodeRenderType, + CanvasConnectionMode, createCanvasGroupNodeId, isCanvasGroupNode, parseCanvasGroupNodeId, @@ -156,7 +157,7 @@ const emit = defineEmits<{ 'copy:nodes': [ids: string[]]; 'duplicate:nodes': [ids: string[]]; 'update:nodes:pin': [ids: string[], source: PinDataSource]; - 'cut:nodes': [ids: string[]]; + 'cut:nodes': [ids: string[], deleteWholeGroupIds?: string[]]; 'delete:connection': [connection: Connection]; 'create:connection:start': [handle: ConnectStartEvent]; 'create:connection': [connection: Connection]; @@ -581,7 +582,7 @@ const keyMap = computed(() => { const fullKeymap: KeyMap = { ...readOnlyKeymap, - ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids)), + ctrl_x: emitWithSelectedNodes((ids) => emit('cut:nodes', ids, getDeleteWholeGroupIds())), 'delete|backspace': onDeleteSelection, ctrl_d: emitWithSelectedNodes((ids) => emit('duplicate:nodes', ids)), d: emitWithSelectedNodes((ids) => emit('update:nodes:enabled', ids)), @@ -613,7 +614,10 @@ const keyMap = computed(() => { run: emitWithSelectedNodes((ids) => emit('extract-workflow', ids)), }, c: () => emit('start-chat'), - r: emitWithLastSelectedNode((id) => emit('replace:node', id)), + r: () => { + const id = getReplaceTargetId(); + if (id) emit('replace:node', id); + }, shift_alt_u: emitWithLastSelectedNode((id) => emit('copy:test:url', id)), alt_u: emitWithLastSelectedNode((id) => emit('copy:production:url', id)), // Alt+I adds the selected nodes to the AI chat. The two features are mutually @@ -692,6 +696,7 @@ async function onAddNodesToChat( } const lastSelectedNode = ref(); +const lastInteractedNode = ref<{ id: string; documentId: string }>(); const triggerNodes = computed(() => props.nodes.filter((node): node is CanvasNode => { if (isCanvasGroupNode(node)) return false; @@ -1159,19 +1164,22 @@ function onDeleteSelection() { const ids = selectedNodeIdsWithGroupMembers.value; // Expand selected groups to their member nodes before deletion. if (ids.length > 0) { - const deleteWholeGroupIds = selectedNodesAndGroups.value - .filter(isCanvasGroupNode) - .map((node) => parseCanvasGroupNodeId(node.id)) - .filter( - (groupId) => - groupId && - (selectedNodeIds.value.length === 0 || explicitlySelectedGroupIds.value.has(groupId)), - ) - .filter(isPresent); - emit('delete:nodes', ids, deleteWholeGroupIds); + emit('delete:nodes', ids, getDeleteWholeGroupIds()); } } +function getDeleteWholeGroupIds() { + return selectedNodesAndGroups.value + .filter(isCanvasGroupNode) + .map((node) => parseCanvasGroupNodeId(node.id)) + .filter( + (groupId) => + groupId && + (selectedNodeIds.value.length === 0 || explicitlySelectedGroupIds.value.has(groupId)), + ) + .filter(isPresent); +} + // Last header-click toggle, for double-click suppression in onNodeClick. let lastHeaderToggle: { groupId: string; at: number } | undefined; @@ -1196,6 +1204,11 @@ function onNodeClick({ event, node }: NodeMouseEvent) { return; } + lastInteractedNode.value = { + id: node.id, + documentId: workflowDocumentStore.value.documentId, + }; + if (chatPanelStore.isOpen && focusedNodesStore.isFeatureEnabled) { focusedNodesStore.setUnconfirmedFromCanvasSelection([node.id]); } @@ -1381,7 +1394,13 @@ function normalizeEmptyGroupConnectionStart(handle: ConnectStartEvent): ConnectS return { ...handle, nodeId: group.nodeId, - handleId: createCanvasConnectionHandleString({ mode: 'outputs' }), + isEmptyGroupTargetStart: handle.handleType === 'target', + // Preserve the side the user started from. A left/input drag should add + // a node into the group's input, not turn into an output replacement. + handleId: createCanvasConnectionHandleString({ + mode: + handle.handleType === 'target' ? CanvasConnectionMode.Input : CanvasConnectionMode.Output, + }), }; } @@ -1501,6 +1520,26 @@ function emitWithLastSelectedNode(emitFn: (id: string) => void) { }; } +function getReplaceTargetId(): string | undefined { + const lastInteractedNodeId = + lastInteractedNode.value?.documentId === workflowDocumentStore.value.documentId + ? lastInteractedNode.value.id + : undefined; + + if ( + lastInteractedNodeId && + selectedNodes.value.some((node) => node.id === lastInteractedNodeId) + ) { + return lastInteractedNodeId; + } + + if (selectedNodes.value.length === 1) { + return selectedNodes.value[0].id; + } + + return lastSelectedNode.value?.id; +} + /** * View */ @@ -1672,13 +1711,7 @@ async function onContextMenuAction(action: ContextMenuAction, nodeIds: string[], case 'copy': return emit('copy:nodes', nodeIds); case 'delete': { - const deleteWholeGroupIds = new Set( - selectedNodesAndGroups.value - .filter(isCanvasGroupNode) - .map((node) => parseCanvasGroupNodeId(node.id)) - .filter((groupId) => groupId && explicitlySelectedGroupIds.value.has(groupId)) - .filter(isPresent), - ); + const deleteWholeGroupIds = new Set(getDeleteWholeGroupIds()); if (groupId) deleteWholeGroupIds.add(groupId); return emit('delete:nodes', nodeIds, [...deleteWholeGroupIds]); @@ -1703,8 +1736,13 @@ async function onContextMenuAction(action: ContextMenuAction, nodeIds: string[], return onSetNodeActivated(nodeIds[0]); case 'rename': return emit('update:node:name', nodeIds[0]); - case 'replace': - return emit('replace:node', nodeIds[0]); + case 'replace': { + const contextTarget = contextMenu.target.value; + const targetNodeId = + contextTarget && contextTarget.source !== 'group' ? contextTarget.nodeId : nodeIds[0]; + if (targetNodeId) return emit('replace:node', targetNodeId); + return; + } case 'change_color': return props.eventBus.emit('nodes:action', { ids: nodeIds, action: 'update:sticky:color' }); case 'tidy_up': diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.test.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.test.ts index a007bd0423f..8e963c6bb6f 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.test.ts @@ -5,6 +5,7 @@ import { setActivePinia } from 'pinia'; import type { ConnectionLineProps } from '@vue-flow/core'; import { Position } from '@vue-flow/core'; import { createCanvasProvide } from '@/features/workflows/canvas/__tests__/utils'; +import { CANVAS_NODE_GROUP_INPUT_HANDLE } from '../../../canvas.types'; import { waitFor } from '@testing-library/vue'; const DEFAULT_PROPS = { @@ -24,6 +25,22 @@ const renderComponent = createComponentRenderer(CanvasConnectionLine, { }, }); +const renderComponentFromInputHandle = createComponentRenderer(CanvasConnectionLine, { + props: DEFAULT_PROPS, + global: { + provide: { + ...createCanvasProvide({ + connectingHandle: { + nodeId: 'group:g1', + handleId: CANVAS_NODE_GROUP_INPUT_HANDLE, + handleType: 'target', + isEmptyGroupTargetStart: true, + }, + }), + }, + }, +}); + beforeEach(() => { const pinia = createTestingPinia(); setActivePinia(pinia); @@ -38,6 +55,10 @@ describe('CanvasConnectionLine', () => { const edge = container.querySelector('.vue-flow__edge-path'); expect(edge).toHaveAttribute('d', 'M0,0 C0,-62.5 100,162.5 100,100'); + expect(edge).toHaveStyle({ + strokeLinecap: 'round', + strokeLinejoin: 'round', + }); }); it('should render a correct smooth step path when the connection is backwards', () => { @@ -65,6 +86,24 @@ describe('CanvasConnectionLine', () => { ); }); + it('should render a bezier path when the connection starts from an input handle', () => { + const { container } = renderComponentFromInputHandle({ + props: { + ...DEFAULT_PROPS, + sourceX: 100, + sourceY: 0, + sourcePosition: Position.Left, + targetX: -100, + targetY: -100, + targetPosition: Position.Left, + }, + }); + + const edge = container.querySelector('.vue-flow__edge-path'); + + expect(edge).toHaveAttribute('d', 'M100,0 C0,0 -188.38834764831844,-100 -100,-100'); + }); + it('should show the connection line after a short delay', async () => { vi.useFakeTimers(); const { container } = renderComponent({ diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.vue b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.vue index e1088d1a5d5..ab34aec05f5 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.vue +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/CanvasConnectionLine.vue @@ -18,6 +18,10 @@ const connectionType = computed( () => parseCanvasConnectionHandleString(connectingHandle.value?.handleId).type, ); +const isEmptyGroupInputDrag = computed(() => { + return connectingHandle.value?.isEmptyGroupTargetStart === true; +}); + const classes = computed(() => { return { [$style.edge]: true, @@ -29,10 +33,15 @@ const edgeStyle = computed(() => ({ ...(connectionType.value === NodeConnectionTypes.Main ? {} : { strokeDasharray: '5,6' }), strokeWidth: 2, stroke: 'var(--color--foreground--shade-2)', + strokeLinecap: 'round', + strokeLinejoin: 'round', })); const renderData = computed(() => - getEdgeRenderData(props, { connectionType: connectionType.value }), + getEdgeRenderData(props, { + connectionType: connectionType.value, + useBezierPath: isEmptyGroupInputDrag.value, + }), ); const segments = computed(() => renderData.value.segments); diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/utils/getEdgeRenderData.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/utils/getEdgeRenderData.ts index 89db7ab558c..0983f105529 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/utils/getEdgeRenderData.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/edges/utils/getEdgeRenderData.ts @@ -17,14 +17,21 @@ export function getEdgeRenderData( >, { connectionType = NodeConnectionTypes.Main, + useBezierPath = false, }: { connectionType?: NodeConnectionType; + /** Live connections from an input handle should not use backwards-edge routing. */ + useBezierPath?: boolean; } = {}, ) { const { targetX, targetY, sourceX, sourceY, sourcePosition, targetPosition } = props; const isConnectorStraight = sourceY === targetY; - if (!isRightOfSourceHandle(sourceX, targetX) || connectionType !== NodeConnectionTypes.Main) { + if ( + useBezierPath || + !isRightOfSourceHandle(sourceX, targetX) || + connectionType !== NodeConnectionTypes.Main + ) { const segment = getBezierPath(props); return { segments: [segment], diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/groups/CanvasNodeGroupTitleBar.test.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/groups/CanvasNodeGroupTitleBar.test.ts index 03183c1f7cb..ccda319bdf1 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/groups/CanvasNodeGroupTitleBar.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/groups/CanvasNodeGroupTitleBar.test.ts @@ -28,12 +28,20 @@ const viewportRef = { value: { x: 0, y: 0, zoom: 1 } }; vi.mock('@vue-flow/core', () => ({ Handle: { name: 'Handle', - props: ['id', 'type', 'position', 'connectable'], + props: ['id', 'type', 'position', 'connectable', 'connectableStart', 'connectableEnd'], render() { + const handle = this as unknown as { + id: string; + connectable: boolean; + connectableStart: boolean; + connectableEnd: boolean; + }; return h('div', { class: 'vue-flow__handle', - 'data-handle-id': (this as unknown as { id: string }).id, - 'data-connectable': String((this as unknown as { connectable: boolean }).connectable), + 'data-handle-id': handle.id, + 'data-connectable': String(handle.connectable), + 'data-connectable-start': String(handle.connectableStart), + 'data-connectable-end': String(handle.connectableEnd), }); }, }, @@ -152,7 +160,7 @@ describe('CanvasNodeGroupTitleBar', () => { }); describe('empty-group connection handles', () => { - it('enables both title-bar handles only for a collapsed empty group', () => { + it('enables both directions on title-bar handles only for a collapsed empty group', () => { const wrapper = render({ data: makeData({ isCollapsed: true, @@ -162,6 +170,26 @@ describe('CanvasNodeGroupTitleBar', () => { }); expect(wrapper.container.querySelectorAll('[data-connectable="true"]')).toHaveLength(2); + expect( + wrapper.container + .querySelector(`[data-handle-id="${CANVAS_NODE_GROUP_INPUT_HANDLE}"]`) + ?.getAttribute('data-connectable-start'), + ).toBe('true'); + expect( + wrapper.container + .querySelector(`[data-handle-id="${CANVAS_NODE_GROUP_INPUT_HANDLE}"]`) + ?.getAttribute('data-connectable-end'), + ).toBe('true'); + expect( + wrapper.container + .querySelector(`[data-handle-id="${CANVAS_NODE_GROUP_OUTPUT_HANDLE}"]`) + ?.getAttribute('data-connectable-start'), + ).toBe('true'); + expect( + wrapper.container + .querySelector(`[data-handle-id="${CANVAS_NODE_GROUP_OUTPUT_HANDLE}"]`) + ?.getAttribute('data-connectable-end'), + ).toBe('true'); }); it('does not enable handles for expanded or non-empty groups', () => { diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.test.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.test.ts index abe51fbb5fe..2f7af93851d 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.test.ts @@ -2,8 +2,9 @@ import CanvasHandleRenderer from './CanvasHandleRenderer.vue'; import { NodeConnectionTypes } from 'n8n-workflow'; import { createComponentRenderer } from '@/__tests__/render'; import { CanvasNodeHandleKey } from '@/app/constants'; -import { ref, type ComputedRef } from 'vue'; +import { defineComponent, ref, type ComputedRef } from 'vue'; import { + CANVAS_NODE_GROUP_INPUT_HANDLE, CanvasConnectionMode, type CanvasConnectionPort, type CanvasElementPortWithRenderData, @@ -39,9 +40,12 @@ const renderComponent = createComponentRenderer(CanvasHandleRenderer, { }, }); -const Handle = { - template: '
', -}; +const Handle = defineComponent({ + props: { + connectableEnd: Boolean, + }, + template: '
', +}); describe('CanvasHandleRenderer', () => { it('should render the main input handle correctly', async () => { @@ -84,6 +88,36 @@ describe('CanvasHandleRenderer', () => { expect(container.querySelector('.handle')).toBeInTheDocument(); expect(container.querySelector('.outputs.main')).toBeInTheDocument(); + expect(container.querySelector('[data-connectable-end="false"]')).toBeInTheDocument(); + }); + + it('allows a main output to receive a drag from an empty-group input', async () => { + const { container } = renderComponent({ + props: { + mode: CanvasConnectionMode.Output, + type: NodeConnectionTypes.Main, + index: 0, + position: Position.Right, + offset: { right: '10px', bottom: '10px' } as CanvasElementPortWithRenderData['offset'], + label: 'Main Output', + }, + global: { + provide: { + ...createCanvasProvide({ + connectingHandle: { + nodeId: 'anchor', + handleId: CANVAS_NODE_GROUP_INPUT_HANDLE, + handleType: 'target', + isEmptyGroupTargetStart: true, + }, + }), + ...createCanvasNodeProvide(), + }, + stubs: { Handle }, + }, + }); + + expect(container.querySelector('[data-connectable-end="true"]')).toBeInTheDocument(); }); it('should render the non-main handle correctly', async () => { diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.vue b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.vue index 1ebd01e7ae3..db82d34c68a 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.vue +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/components/elements/handles/CanvasHandleRenderer.vue @@ -11,6 +11,7 @@ import CanvasHandleNonMainInput from './render-types/CanvasHandleNonMainInput.vu import CanvasHandleNonMainOutput from './render-types/CanvasHandleNonMainOutput.vue'; import { CanvasNodeHandleKey } from '@/app/constants'; import { useCanvasNode } from '../../../composables/useCanvasNode'; +import { useCanvas } from '../../../composables/useCanvas'; const props = defineProps< CanvasElementPortWithRenderData & { @@ -40,6 +41,8 @@ defineOptions({ const style = useCssModule(); +const { connectingHandle } = useCanvas(); + const handleType = computed(() => props.mode === CanvasConnectionMode.Input ? 'target' : 'source', ); @@ -54,6 +57,10 @@ const connectionsLimitReached = computed(() => { return props.maxConnections && props.connectionsCount >= props.maxConnections; }); +const isEmptyGroupInputDrag = computed(() => { + return connectingHandle.value?.isEmptyGroupTargetStart === true; +}); + const isConnectableStart = computed(() => { if (connectionsLimitReached.value) return false; @@ -63,7 +70,11 @@ const isConnectableStart = computed(() => { const isConnectableEnd = computed(() => { if (connectionsLimitReached.value) return false; - return props.mode === CanvasConnectionMode.Input || props.type !== NodeConnectionTypes.Main; + return ( + props.mode === CanvasConnectionMode.Input || + props.type !== NodeConnectionTypes.Main || + (props.mode === CanvasConnectionMode.Output && isEmptyGroupInputDrag.value) + ); }); const isConnected = computed(() => props.connectionsCount > 0); diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.test.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.test.ts index 029fd18b5bf..d2bc400bc23 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.test.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.test.ts @@ -1,7 +1,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { createPinia, setActivePinia } from 'pinia'; import type { VNode } from 'vue'; -import type { IConnection, INodeTypeDescription } from 'n8n-workflow'; +import { + NodeConnectionTypes, + type IConnection, + type IConnections, + type INodeTypeDescription, +} from 'n8n-workflow'; import { useCanvasNodeGroupOperationGuards } from './useCanvasNodeGroupOperationGuards'; import { @@ -13,6 +18,7 @@ import { useNodeTypesStore } from '@/app/stores/nodeTypes.store'; import { useTelemetry } from '@n8n/composables/useTelemetry'; import { STICKY_NODE_TYPE } from '@/app/constants/nodeTypes'; import type { INodeUi } from '@/Interface'; +import { mockNodeTypeDescription } from '@/__tests__/mocks'; const trackSpy = vi.hoisted(() => vi.fn()); const showToastSpy = vi.hoisted(() => vi.fn((_config: { message: VNode }) => ({ close: vi.fn() }))); @@ -160,4 +166,79 @@ describe('useCanvasNodeGroupOperationGuards', () => { expect(showToastSpy).toHaveBeenCalledTimes(1); }); }); + + describe('replacement batches', () => { + const mainConnection = (source: string, target: string): [IConnection, IConnection] => [ + { node: source, type: NodeConnectionTypes.Main, index: 0 }, + { node: target, type: NodeConnectionTypes.Main, index: 0 }, + ]; + + function setupBatchGroup(): { connectionsBySourceNode: IConnections } { + const nodes = ['previous', 'existing', 'replacement', 'helper'].map((id) => ({ + id, + name: id.toUpperCase(), + type: 'n8n-nodes-base.set', + typeVersion: 1, + position: [0, 0] as [number, number], + parameters: {}, + })) as INodeUi[]; + workflowDocumentStore.setNodes(nodes); + workflowDocumentStore.createGroup(['previous', 'existing'], 'Group A'); + vi.spyOn(workflowDocumentStore, 'getExpressionHandler').mockReturnValue({ + getSimpleParameterValue: () => undefined, + } as unknown as ReturnType); + vi.spyOn(useNodeTypesStore() as any, 'getNodeType', 'get').mockReturnValue(() => + mockNodeTypeDescription({ + name: 'n8n-nodes-base.set', + inputs: [NodeConnectionTypes.Main], + outputs: [NodeConnectionTypes.Main], + }), + ); + + return { + connectionsBySourceNode: { + PREVIOUS: { main: [[{ node: 'EXISTING', type: 'main', index: 0 }]] }, + }, + }; + } + + it('blocks a batch whose final group membership is disconnected', () => { + const connectionsBySourceNode = setupBatchGroup().connectionsBySourceNode; + const guards = useCanvasNodeGroupOperationGuards(); + + const allowed = guards.isNodeReplacementAllowedForNodeGroups({ + previousNodeId: 'previous', + newNodeId: 'replacement', + additionalGroupNodeIds: ['helper'], + nodeIds: [], + connectionsToRemove: [mainConnection('PREVIOUS', 'EXISTING')], + connectionsToAdd: [mainConnection('REPLACEMENT', 'HELPER')], + connectionsBySourceNode, + }); + + expect(allowed).toBe(false); + expect(showToastSpy).toHaveBeenCalledTimes(1); + }); + + it('allows a batch whose final group membership is a connected path', () => { + const connectionsBySourceNode = setupBatchGroup().connectionsBySourceNode; + const guards = useCanvasNodeGroupOperationGuards(); + + const allowed = guards.isNodeReplacementAllowedForNodeGroups({ + previousNodeId: 'previous', + newNodeId: 'replacement', + additionalGroupNodeIds: ['helper'], + nodeIds: [], + connectionsToRemove: [mainConnection('PREVIOUS', 'EXISTING')], + connectionsToAdd: [ + mainConnection('REPLACEMENT', 'HELPER'), + mainConnection('HELPER', 'EXISTING'), + ], + connectionsBySourceNode, + }); + + expect(allowed).toBe(true); + expect(showToastSpy).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.ts b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.ts index a00efae0805..8ac317ce2a4 100644 --- a/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.ts +++ b/packages/frontend/editor-ui/src/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards.ts @@ -347,6 +347,7 @@ export function useCanvasNodeGroupOperationGuards() { nodeIds, connectionsToRemove, connectionsToAdd, + additionalGroupNodeIds = [], connectionsBySourceNode, }: { previousNodeId: string; @@ -354,6 +355,8 @@ export function useCanvasNodeGroupOperationGuards() { nodeIds: string[]; connectionsToRemove: Array<[IConnection, IConnection]>; connectionsToAdd: Array<[IConnection, IConnection]>; + /** Nodes that will be added to the previous node's group as part of the same operation. */ + additionalGroupNodeIds?: string[]; connectionsBySourceNode: IConnections; }): boolean { const previousGroup = workflowDocumentStore.value.getGroupForNode(previousNodeId); @@ -380,9 +383,10 @@ export function useCanvasNodeGroupOperationGuards() { connectionsToRemove, connectionsToAdd, }); - const swappedPreviousGroupNodeIds = uniq( - previousGroup.nodeIds.map((nodeId) => (nodeId === previousNodeId ? newNodeId : nodeId)), - ); + const swappedPreviousGroupNodeIds = uniq([ + ...previousGroup.nodeIds.map((nodeId) => (nodeId === previousNodeId ? newNodeId : nodeId)), + ...additionalGroupNodeIds, + ]); const getNodeIdsForGroup = (group: IWorkflowGroup) => group.id === previousGroup.id ? swappedPreviousGroupNodeIds : group.nodeIds;