fix(editor): Empty Group Last Refinements (no-changelog) (#39632)

This commit is contained in:
Daria 2026-09-28 12:57:32 +00:00 committed by GitHub
parent b3ce351cde
commit 06c61d9437
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
20 changed files with 587 additions and 76 deletions

View File

@ -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', () => {

View File

@ -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) {

View File

@ -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', () => {

View File

@ -692,11 +692,11 @@ async function onClipboardPaste(plainTextData: string): Promise<void> {
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);
}
}

View File

@ -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) => {

View File

@ -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.

View File

@ -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

View File

@ -184,7 +184,9 @@ function flushPendingSearchOnNavigation(event: KeyboardEvent) {
}
function onTransitionEnd() {
cleanupopeningContext();
if (viewStacks.value.length === 0) {
cleanupopeningContext();
}
void setActiveItemIndex(getDefaultActiveIndex());
}

View File

@ -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[]) {

View File

@ -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'] };

View File

@ -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

View File

@ -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<GraphNode>();
const lastInteractedNode = ref<{ id: string; documentId: string }>();
const triggerNodes = computed<CanvasNode[]>(() =>
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':

View File

@ -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({

View File

@ -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);

View File

@ -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],

View File

@ -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', () => {

View File

@ -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: '<div><slot /></div>',
};
const Handle = defineComponent({
props: {
connectableEnd: Boolean,
},
template: '<div :data-connectable-end="connectableEnd"><slot /></div>',
});
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 () => {

View File

@ -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);

View File

@ -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<typeof workflowDocumentStore.getExpressionHandler>);
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();
});
});
});

View File

@ -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;