mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(editor): Manage the empty-group placeholder lifecycle (no-changelog) (#38704)
This commit is contained in:
Daria
GitHub
parent
37c160eec0
commit
acaa281c54
|
|
@ -28,7 +28,9 @@ import type {
|
|||
import type { IWorkflowTemplate, IWorkflowTemplateNode } from '@n8n/rest-api-client/api/templates';
|
||||
import {
|
||||
AddConnectionCommand,
|
||||
AddNodeCommand,
|
||||
AddNodeGroupCommand,
|
||||
BulkCommand,
|
||||
RemoveNodeCommand,
|
||||
RemoveNodeGroupCommand,
|
||||
ReplaceNodeParametersCommand,
|
||||
|
|
@ -69,6 +71,7 @@ import {
|
|||
HTTP_REQUEST_NODE_TYPE,
|
||||
MCP_TRIGGER_NODE_TYPE,
|
||||
MESSAGE_AN_AGENT_NODE_TYPE,
|
||||
NO_OP_NODE_TYPE,
|
||||
OPEN_AI_CHAT_MODEL_NODE_TYPE,
|
||||
SET_NODE_TYPE,
|
||||
STICKY_NODE_TYPE,
|
||||
|
|
@ -137,6 +140,7 @@ vi.mock('@n8n/rest-api-client/api/workflowHistory', () => ({
|
|||
|
||||
import { useCanvasOperations } from '@/app/composables/useCanvasOperations';
|
||||
import * as workflowHelpersModule from '@/app/composables/useWorkflowHelpers';
|
||||
import * as nodeGroupOperationGuards from '@/features/workflows/canvas/composables/useCanvasNodeGroupOperationGuards';
|
||||
import { DEFAULT_NODE_SIZE, GRID_SIZE, HORIZONTAL_NODE_STEP } from '@/app/utils/nodeViewUtils';
|
||||
import { AGENT_NODE_SIZE } from '@/features/agents/utils/agentNode';
|
||||
|
||||
|
|
@ -1840,11 +1844,11 @@ describe('useCanvasOperations', () => {
|
|||
expect(groupCommand?.after.nodeIds).toEqual(['a', 'c']);
|
||||
});
|
||||
|
||||
it('records a group removal when deleting the last grouped node', () => {
|
||||
it('records a group removal when deleting the last grouped sticky note', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
|
||||
const node = createTestNode({ id: 'b', name: 'B' });
|
||||
const node = createTestNode({ id: 'b', name: 'B', type: STICKY_NODE_TYPE });
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getNodeById').mockReturnValue(node);
|
||||
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: ['b'] };
|
||||
|
|
@ -1863,6 +1867,203 @@ describe('useCanvasOperations', () => {
|
|||
expect(groupCommand?.group.nodeIds).toEqual(['b']);
|
||||
});
|
||||
|
||||
it('restores a placeholder anchor when deleting the last real group member', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
const nodeTypesStore = mockedStore(useNodeTypesStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
|
||||
const node = createTestNode({ id: 'b', name: 'B', position: [112, 208] });
|
||||
const anchorType = mockNodeTypeDescription({
|
||||
name: NO_OP_NODE_TYPE,
|
||||
inputs: [NodeConnectionTypes.Main],
|
||||
outputs: [NodeConnectionTypes.Main],
|
||||
properties: [
|
||||
{
|
||||
displayName: 'Empty Group Anchor',
|
||||
name: 'emptyGroupAnchor',
|
||||
type: 'hidden',
|
||||
default: false,
|
||||
validateType: undefined,
|
||||
},
|
||||
],
|
||||
});
|
||||
const nodeType = mockNodeTypeDescription({
|
||||
name: node.type,
|
||||
inputs: [NodeConnectionTypes.Main],
|
||||
outputs: [NodeConnectionTypes.Main],
|
||||
});
|
||||
nodeTypesStore.nodeTypes = {
|
||||
[node.type]: { 1: nodeType },
|
||||
[NO_OP_NODE_TYPE]: { 1: anchorType },
|
||||
};
|
||||
nodeTypesStore.getNodeType = vi.fn((type) =>
|
||||
type === NO_OP_NODE_TYPE ? anchorType : type === node.type ? nodeType : null,
|
||||
);
|
||||
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [node.id] };
|
||||
const nodesById = new Map([[node.id, node]]);
|
||||
workflowDocumentStoreInstance.connectionsBySourceNode = {};
|
||||
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(group);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'replaceNodeInGroup').mockImplementation(
|
||||
(_groupId, previousNodeId, newNodeId) => {
|
||||
group.nodeIds = group.nodeIds.map((id) => (id === previousNodeId ? newNodeId : id));
|
||||
},
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getParentNodes').mockReturnValue([]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getChildNodes').mockReturnValue([]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getConnectionsBetweenNodes').mockReturnValue([]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'addNode').mockImplementation((addedNode) => {
|
||||
nodesById.set(addedNode.id, addedNode);
|
||||
});
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'removeNodeById').mockImplementation((id) => {
|
||||
nodesById.delete(id);
|
||||
});
|
||||
|
||||
const { deleteNode } = useCanvasOperations();
|
||||
deleteNode(node.id, { trackHistory: true });
|
||||
|
||||
expect(workflowDocumentStoreInstance.addNode).toHaveBeenCalled();
|
||||
expect(workflowDocumentStoreInstance.replaceNodeInGroup).toHaveBeenCalled();
|
||||
|
||||
const anchor = [...nodesById.values()].find(
|
||||
(candidate) => candidate.parameters?.emptyGroupAnchor === true,
|
||||
);
|
||||
expect(anchor).toMatchObject({
|
||||
type: NO_OP_NODE_TYPE,
|
||||
position: node.position,
|
||||
parameters: { emptyGroupAnchor: true },
|
||||
placeholder: true,
|
||||
});
|
||||
expect(group.nodeIds).toEqual([anchor?.id]);
|
||||
expect(nodesById.has(node.id)).toBe(false);
|
||||
expect(historyStore.pushCommandToUndo).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not restore an anchor when deleting the selected group', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
|
||||
const node = createTestNode({ id: 'b', name: 'B' });
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [node.id] };
|
||||
const nodesById = new Map([[node.id, node]]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getNodeById').mockImplementation((id) =>
|
||||
nodesById.get(id),
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupForNode').mockImplementation((id) =>
|
||||
group.nodeIds.includes(id) ? group : undefined,
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupById').mockReturnValue(undefined);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'removeNodeById').mockImplementation((id) => {
|
||||
nodesById.delete(id);
|
||||
});
|
||||
|
||||
const { deleteNodes } = useCanvasOperations();
|
||||
deleteNodes([node.id], { trackHistory: true, preserveEmptyGroupAnchor: false });
|
||||
|
||||
expect(workflowDocumentStoreInstance.addNode).not.toHaveBeenCalled();
|
||||
expect(workflowDocumentStoreInstance.removeNodeById).toHaveBeenCalledWith(node.id);
|
||||
expect(nodesById.has(node.id)).toBe(false);
|
||||
expect(historyStore.pushCommandToUndo).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not leave an anchor add command when replacement is rejected', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
const nodeTypesStore = mockedStore(useNodeTypesStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
historyStore.currentBulkAction = new BulkCommand([]);
|
||||
historyStore.pushCommandToUndo.mockImplementation((command) => {
|
||||
historyStore.currentBulkAction?.commands.push(command);
|
||||
});
|
||||
|
||||
const node = createTestNode({ id: 'b', name: 'B', position: [112, 208] });
|
||||
const anchorType = mockNodeTypeDescription({
|
||||
name: NO_OP_NODE_TYPE,
|
||||
inputs: [NodeConnectionTypes.Main],
|
||||
outputs: [NodeConnectionTypes.Main],
|
||||
});
|
||||
nodeTypesStore.nodeTypes = { [NO_OP_NODE_TYPE]: { 1: anchorType } };
|
||||
nodeTypesStore.getNodeType = vi.fn().mockReturnValue(anchorType);
|
||||
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [node.id] };
|
||||
const nodesById = new Map([[node.id, node]]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getNodeById').mockImplementation((id) =>
|
||||
nodesById.get(id),
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupForNode').mockImplementation((id) =>
|
||||
group.nodeIds.includes(id) ? group : undefined,
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupById').mockReturnValue(undefined);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'addNode').mockImplementation((addedNode) => {
|
||||
nodesById.set(addedNode.id, addedNode);
|
||||
});
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'removeNodeById').mockImplementation((id) => {
|
||||
nodesById.delete(id);
|
||||
});
|
||||
|
||||
const replacementAllowedSpy = vi.fn().mockReturnValue(false);
|
||||
vi.spyOn(nodeGroupOperationGuards, 'useCanvasNodeGroupOperationGuards').mockReturnValue({
|
||||
isConnectionRemovalAllowedForNodeGroups: vi.fn().mockReturnValue(true),
|
||||
isConnectionReplacementAllowedForNodeGroups: vi
|
||||
.fn()
|
||||
.mockReturnValue({ outcome: 'proceed' }),
|
||||
isNodeReplacementAllowedForNodeGroups: replacementAllowedSpy,
|
||||
applyNodeGroupAutoExtend: vi.fn(),
|
||||
});
|
||||
|
||||
const { deleteNode } = useCanvasOperations();
|
||||
deleteNode(node.id, { trackHistory: true });
|
||||
|
||||
expect(replacementAllowedSpy).toHaveBeenCalled();
|
||||
expect(workflowDocumentStoreInstance.removeNodeById).toHaveBeenCalledWith(node.id);
|
||||
const commands = historyStore.currentBulkAction?.commands ?? [];
|
||||
expect(commands.some((command) => command instanceof AddNodeCommand)).toBe(false);
|
||||
});
|
||||
|
||||
it('deletes an existing empty-group anchor without restoring another anchor', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
|
||||
const anchor = createTestNode({
|
||||
id: 'anchor',
|
||||
name: 'No Operation, do nothing',
|
||||
type: NO_OP_NODE_TYPE,
|
||||
parameters: { emptyGroupAnchor: true },
|
||||
});
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [anchor.id] };
|
||||
const nodesById = new Map([[anchor.id, anchor]]);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getNodeById').mockImplementation((id) =>
|
||||
nodesById.get(id),
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupForNode').mockImplementation((id) =>
|
||||
group.nodeIds.includes(id) ? group : undefined,
|
||||
);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'getGroupById').mockReturnValue(undefined);
|
||||
vi.spyOn(workflowDocumentStoreInstance, 'removeNodeById').mockImplementation((id) => {
|
||||
nodesById.delete(id);
|
||||
});
|
||||
|
||||
const { deleteNode } = useCanvasOperations();
|
||||
deleteNode(anchor.id, { trackHistory: true });
|
||||
|
||||
expect(workflowDocumentStoreInstance.addNode).not.toHaveBeenCalled();
|
||||
expect(workflowDocumentStoreInstance.replaceNodeInGroup).not.toHaveBeenCalled();
|
||||
expect(workflowDocumentStoreInstance.removeNodeById).toHaveBeenCalledWith(anchor.id);
|
||||
expect(
|
||||
historyStore.pushCommandToUndo.mock.calls.some(
|
||||
([command]) => command instanceof RemoveNodeGroupCommand,
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('does not record any group command when the deleted node is ungrouped', () => {
|
||||
const historyStore = mockedStore(useHistoryStore);
|
||||
vi.mocked(workflowDocumentStoreInstance.incomingConnectionsByNodeName).mockReturnValue({});
|
||||
|
|
@ -7652,11 +7853,11 @@ describe('useCanvasOperations', () => {
|
|||
});
|
||||
});
|
||||
describe('node group validation', () => {
|
||||
it('should replace a grouped node against the final graph and transfer group membership', () => {
|
||||
it('should replace the sole group anchor and transfer all group connections', () => {
|
||||
const toast = useToast();
|
||||
const group = {
|
||||
id: 'group',
|
||||
nodeIds: [sourceNode.id, targetNode.id, nextNode.id],
|
||||
nodeIds: [targetNode.id],
|
||||
name: 'Group 1',
|
||||
};
|
||||
const connections: IConnections = {
|
||||
|
|
@ -7757,7 +7958,7 @@ describe('useCanvasOperations', () => {
|
|||
targetNode.id,
|
||||
replacementNode.id,
|
||||
);
|
||||
expect(group.nodeIds).toEqual([sourceNode.id, replacementNode.id, nextNode.id]);
|
||||
expect(group.nodeIds).toEqual([replacementNode.id]);
|
||||
|
||||
const groupCommand = historyStore.pushCommandToUndo.mock.calls
|
||||
.map(([command]) => command)
|
||||
|
|
@ -7765,12 +7966,8 @@ describe('useCanvasOperations', () => {
|
|||
| UpdateNodeGroupCommand
|
||||
| undefined;
|
||||
expect(groupCommand).toBeInstanceOf(UpdateNodeGroupCommand);
|
||||
expect(groupCommand?.before.nodeIds).toEqual([sourceNode.id, targetNode.id, nextNode.id]);
|
||||
expect(groupCommand?.after.nodeIds).toEqual([
|
||||
sourceNode.id,
|
||||
replacementNode.id,
|
||||
nextNode.id,
|
||||
]);
|
||||
expect(groupCommand?.before.nodeIds).toEqual([targetNode.id]);
|
||||
expect(groupCommand?.after.nodeIds).toEqual([replacementNode.id]);
|
||||
|
||||
expect(workflowDocumentStoreInstance.removeConnection).toHaveBeenCalledTimes(2);
|
||||
expectConnectionRemoved(sourceNode, targetNode);
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ import {
|
|||
HTTP_REQUEST_NODE_TYPE,
|
||||
HTTP_REQUEST_TOOL_NODE_TYPE,
|
||||
MESSAGE_AN_AGENT_NODE_TYPE,
|
||||
NO_OP_NODE_TYPE,
|
||||
STICKY_NODE_TYPE,
|
||||
UPDATE_WEBHOOK_ID_NODE_TYPES,
|
||||
VIEWS,
|
||||
|
|
@ -139,6 +140,7 @@ import {
|
|||
NodeHelpers,
|
||||
TelemetryHelpers,
|
||||
isCommunityPackageName,
|
||||
isEmptyGroupAnchor,
|
||||
isHitlToolType,
|
||||
isResourceLocatorValue,
|
||||
} from 'n8n-workflow';
|
||||
|
|
@ -636,7 +638,10 @@ export function useCanvasOperations() {
|
|||
}
|
||||
}
|
||||
|
||||
function deleteNode(id: string, { trackHistory = false, trackBulk = true } = {}) {
|
||||
function deleteNode(
|
||||
id: string,
|
||||
{ trackHistory = false, trackBulk = true, preserveEmptyGroupAnchor = true } = {},
|
||||
) {
|
||||
const node = workflowDocumentStore.value.getNodeById(id);
|
||||
if (!node) {
|
||||
return;
|
||||
|
|
@ -652,6 +657,53 @@ export function useCanvasOperations() {
|
|||
uiStore.lastInteractedWithNodeId = undefined;
|
||||
}
|
||||
|
||||
const group = workflowDocumentStore.value.getGroupForNode(id);
|
||||
const shouldRestoreEmptyGroupAnchor =
|
||||
preserveEmptyGroupAnchor &&
|
||||
group?.nodeIds.length === 1 &&
|
||||
node.type !== STICKY_NODE_TYPE &&
|
||||
!isEmptyGroupAnchor(node);
|
||||
|
||||
if (shouldRestoreEmptyGroupAnchor) {
|
||||
const anchorNodeType = requireNodeTypeDescription(NO_OP_NODE_TYPE);
|
||||
const anchorHistoryIndex = trackHistory
|
||||
? historyStore.currentBulkAction?.commands.length
|
||||
: undefined;
|
||||
const anchor = addNode(
|
||||
{
|
||||
type: NO_OP_NODE_TYPE,
|
||||
typeVersion: resolveNodeVersion(anchorNodeType),
|
||||
position: [...node.position],
|
||||
parameters: { emptyGroupAnchor: true },
|
||||
placeholder: true,
|
||||
},
|
||||
anchorNodeType,
|
||||
{
|
||||
forcePosition: true,
|
||||
isAutoAdd: true,
|
||||
openNDV: false,
|
||||
trackHistory,
|
||||
},
|
||||
);
|
||||
const didReplace = replaceNode(id, anchor.id, { trackHistory, trackBulk: false });
|
||||
if (didReplace) {
|
||||
if (trackHistory && trackBulk) {
|
||||
historyStore.stopRecordingUndo();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// If the replacement is rejected by node-group connection policy, leave
|
||||
// the original deletion path to remove the node and its group.
|
||||
if (anchorHistoryIndex !== undefined) {
|
||||
const anchorCommand = historyStore.currentBulkAction?.commands[anchorHistoryIndex];
|
||||
if (anchorCommand instanceof AddNodeCommand && anchorCommand.node.id === anchor.id) {
|
||||
historyStore.currentBulkAction?.commands.splice(anchorHistoryIndex, 1);
|
||||
}
|
||||
}
|
||||
workflowDocumentStore.value.removeNodeById(anchor.id);
|
||||
}
|
||||
|
||||
connectAdjacentNodes(id, { trackHistory, validateNodeGroups: false });
|
||||
deleteConnectionsByNodeId(id, { trackHistory, trackBulk: false });
|
||||
|
||||
|
|
@ -700,12 +752,21 @@ export function useCanvasOperations() {
|
|||
trackDeleteNode(id);
|
||||
}
|
||||
|
||||
function deleteNodes(ids: string[], { trackHistory = true, trackBulk = true } = {}) {
|
||||
function deleteNodes(
|
||||
ids: string[],
|
||||
{ trackHistory = true, trackBulk = true, preserveEmptyGroupAnchor = true } = {},
|
||||
) {
|
||||
if (trackHistory && trackBulk) {
|
||||
historyStore.startRecordingUndo();
|
||||
}
|
||||
|
||||
ids.forEach((id) => deleteNode(id, { trackHistory, trackBulk: false }));
|
||||
ids.forEach((id) =>
|
||||
deleteNode(id, {
|
||||
trackHistory,
|
||||
trackBulk: false,
|
||||
preserveEmptyGroupAnchor,
|
||||
}),
|
||||
);
|
||||
|
||||
if (trackHistory && trackBulk) {
|
||||
historyStore.stopRecordingUndo();
|
||||
|
|
@ -3642,7 +3703,7 @@ export function useCanvasOperations() {
|
|||
const previousNode = workflowDocumentStore.value.getNodeById(previousId);
|
||||
const newNode = workflowDocumentStore.value.getNodeById(newId);
|
||||
|
||||
if (!previousNode || !newNode) return;
|
||||
if (!previousNode || !newNode) return false;
|
||||
|
||||
if (trackHistory && trackBulk) {
|
||||
historyStore.startRecordingUndo();
|
||||
|
|
@ -3662,7 +3723,7 @@ export function useCanvasOperations() {
|
|||
if (trackHistory && trackBulk) {
|
||||
historyStore.stopRecordingUndo();
|
||||
}
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
moveNewNodeToPreviousPosition();
|
||||
} else {
|
||||
|
|
@ -3680,6 +3741,8 @@ export function useCanvasOperations() {
|
|||
if (trackHistory && trackBulk) {
|
||||
historyStore.stopRecordingUndo();
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
async function addNodesAndConnections(
|
||||
|
|
@ -3706,6 +3769,24 @@ export function useCanvasOperations() {
|
|||
telemetry: true,
|
||||
});
|
||||
|
||||
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) : addedNodes[replacementNodeIndex];
|
||||
if (replacementNode) {
|
||||
const didReplace = replaceNode(options.replaceNodeId, replacementNode.id, {
|
||||
trackHistory,
|
||||
trackBulk: false,
|
||||
});
|
||||
if (didReplace) {
|
||||
replacementGroupId = workflowDocumentStore.value.getGroupForNode(replacementNode.id)?.id;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const allNodes = workflowDocumentStore.value.allNodes;
|
||||
const offsetIndex = allNodes.length - nodes.length;
|
||||
const connections: CanvasConnectionCreateData[] = addedConnections.map(({ from, to }) => {
|
||||
|
|
@ -3741,16 +3822,35 @@ export function useCanvasOperations() {
|
|||
|
||||
await addConnections(connections, { trackHistory, trackBulk: false });
|
||||
|
||||
uiStore.resetLastInteractedWith();
|
||||
|
||||
if (addedNodes.length > 0 && options.replaceNodeId) {
|
||||
const lastAddedNodeId = addedNodes[addedNodes.length - 1].id;
|
||||
replaceNode(options.replaceNodeId, lastAddedNodeId, {
|
||||
trackHistory,
|
||||
trackBulk: false,
|
||||
});
|
||||
if (replacementGroupId) {
|
||||
const groupBeforeExtend = workflowDocumentStore.value.getGroupById(replacementGroupId);
|
||||
if (groupBeforeExtend) {
|
||||
// A creator result can include helpers for the selected node. Keep the
|
||||
// connected batch together when it replaces a grouped node.
|
||||
const beforeSnapshot = { ...groupBeforeExtend, nodeIds: [...groupBeforeExtend.nodeIds] };
|
||||
workflowDocumentStore.value.addNodesToGroup(
|
||||
replacementGroupId,
|
||||
addedNodes.map((node) => node.id),
|
||||
);
|
||||
const groupAfterExtend = workflowDocumentStore.value.getGroupById(replacementGroupId);
|
||||
if (
|
||||
trackHistory &&
|
||||
groupAfterExtend &&
|
||||
groupAfterExtend.nodeIds.length !== beforeSnapshot.nodeIds.length
|
||||
) {
|
||||
historyStore.pushCommandToUndo(
|
||||
new UpdateNodeGroupCommand(
|
||||
beforeSnapshot,
|
||||
{ ...groupAfterExtend, nodeIds: [...groupAfterExtend.nodeIds] },
|
||||
Date.now(),
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
uiStore.resetLastInteractedWith();
|
||||
|
||||
if (trackHistory && trackBulk) {
|
||||
historyStore.stopRecordingUndo();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,12 @@
|
|||
import { createTestNode, createTestWorkflow, mockNodeTypeDescription } from '@/__tests__/mocks';
|
||||
import type { AddedNodesAndConnections } from '@/Interface';
|
||||
import { waitFor } from '@testing-library/vue';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { EVALUATION_TRIGGER_NODE_TYPE, MANUAL_TRIGGER_NODE_TYPE } from 'n8n-workflow';
|
||||
import {
|
||||
EVALUATION_TRIGGER_NODE_TYPE,
|
||||
MANUAL_TRIGGER_NODE_TYPE,
|
||||
NodeConnectionTypes,
|
||||
} from 'n8n-workflow';
|
||||
import {
|
||||
createWorkflowDocumentId,
|
||||
useWorkflowDocumentStore,
|
||||
|
|
@ -14,9 +19,9 @@ import { useNodeTypesStore } from '../stores/nodeTypes.store';
|
|||
import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/sourceControl.store';
|
||||
import { renderComponent } from '@/__tests__/render';
|
||||
import NodeView from './NodeView.vue';
|
||||
import { NO_OP_NODE_TYPE, VIEWS } from '../constants';
|
||||
import { NO_OP_NODE_TYPE, SPLIT_IN_BATCHES_NODE_TYPE, VIEWS } from '../constants';
|
||||
import { WorkflowIdKey, WorkflowDocumentStoreKey } from '../constants/injectionKeys';
|
||||
import { computed, defineComponent, shallowRef } from 'vue';
|
||||
import { computed, defineComponent, nextTick, shallowRef } from 'vue';
|
||||
import { nodeViewEventBus } from '@/app/event-bus';
|
||||
import { useProjectsStore } from '@/features/collaboration/projects/projects.store';
|
||||
import type { Project } from '@/features/collaboration/projects/projects.types';
|
||||
|
|
@ -62,6 +67,22 @@ vi.mock('@/features/ndv/shared/views/NodeDetailsView.vue', () => ({
|
|||
}));
|
||||
|
||||
describe('NodeView', () => {
|
||||
const loopReplacementBatch: AddedNodesAndConnections = {
|
||||
nodes: [
|
||||
{ type: SPLIT_IN_BATCHES_NODE_TYPE, name: 'Loop Over Items' },
|
||||
{
|
||||
type: NO_OP_NODE_TYPE,
|
||||
name: 'Replace Me',
|
||||
isAutoAdd: true,
|
||||
placeholder: true,
|
||||
},
|
||||
],
|
||||
connections: [
|
||||
{ from: { nodeIndex: 0, outputIndex: 1 }, to: { nodeIndex: 1 } },
|
||||
{ from: { nodeIndex: 1 }, to: { nodeIndex: 0 } },
|
||||
],
|
||||
};
|
||||
|
||||
let workflowsStore: ReturnType<typeof useWorkflowsStore>;
|
||||
let workflowDocumentStore: ReturnType<typeof useWorkflowDocumentStore>;
|
||||
let ensureNodesAreVisible: ReturnType<typeof vi.fn>;
|
||||
|
|
@ -101,7 +122,12 @@ describe('NodeView', () => {
|
|||
stubs: {
|
||||
// Component stubs avoid loading async subtrees that can outlive the test environment.
|
||||
LazyNodeCreation: defineComponent({
|
||||
emits: ['addEmptyGroup'],
|
||||
emits: ['addEmptyGroup', 'addNodes'],
|
||||
setup(_, { emit }) {
|
||||
return {
|
||||
addLoopReplacement: () => emit('addNodes', loopReplacementBatch),
|
||||
};
|
||||
},
|
||||
template: `<>
|
||||
<button
|
||||
data-test-id="node-creation-stub-add-empty-group"
|
||||
|
|
@ -111,17 +137,30 @@ describe('NodeView', () => {
|
|||
data-test-id="node-creation-stub-add-empty-groups"
|
||||
@click="$emit('addEmptyGroup', [320, 240]); $emit('addEmptyGroup', [640, 480])"
|
||||
/>
|
||||
<button
|
||||
data-test-id="node-creation-stub-add-loop-replacement"
|
||||
@click="addLoopReplacement"
|
||||
/>
|
||||
</>`,
|
||||
}),
|
||||
LazySetupWorkflowCredentialsButton: { render: () => null },
|
||||
WorkflowCanvas: defineComponent({
|
||||
emits: ['copy:nodes'],
|
||||
setup(_, { expose }) {
|
||||
emits: ['copy:nodes', 'replace:node'],
|
||||
setup(_, { emit, expose }) {
|
||||
expose({ ensureNodesAreVisible });
|
||||
return { copyNodeIds };
|
||||
return {
|
||||
copyNodeIds,
|
||||
replaceFirstNode: () => {
|
||||
const nodeId = workflowDocStore.allNodes[0]?.id;
|
||||
if (nodeId) emit('replace:node', nodeId);
|
||||
},
|
||||
};
|
||||
},
|
||||
template:
|
||||
'<div><button data-test-id="canvas-stub-copy" @click="$emit(\'copy:nodes\', copyNodeIds)" /><slot /></div>',
|
||||
template: `<div>
|
||||
<button data-test-id="canvas-stub-copy" @click="$emit('copy:nodes', copyNodeIds)" />
|
||||
<button data-test-id="canvas-stub-replace-first" @click="replaceFirstNode" />
|
||||
<slot />
|
||||
</div>`,
|
||||
}),
|
||||
},
|
||||
},
|
||||
|
|
@ -129,6 +168,32 @@ describe('NodeView', () => {
|
|||
}
|
||||
|
||||
describe('Empty group creation', () => {
|
||||
function addReplacementNodeTypes() {
|
||||
useNodeTypesStore().setNodeTypes([
|
||||
mockNodeTypeDescription({
|
||||
name: NO_OP_NODE_TYPE,
|
||||
displayName: 'No Operation, do nothing',
|
||||
inputs: [NodeConnectionTypes.Main],
|
||||
outputs: [NodeConnectionTypes.Main],
|
||||
properties: [
|
||||
{
|
||||
displayName: 'Empty Group Anchor',
|
||||
name: 'emptyGroupAnchor',
|
||||
type: 'hidden',
|
||||
default: false,
|
||||
validateType: undefined,
|
||||
},
|
||||
],
|
||||
}),
|
||||
mockNodeTypeDescription({
|
||||
name: SPLIT_IN_BATCHES_NODE_TYPE,
|
||||
displayName: 'Loop Over Items',
|
||||
inputs: [NodeConnectionTypes.Main],
|
||||
outputs: [NodeConnectionTypes.Main, NodeConnectionTypes.Main],
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
it('creates a marked NoOp and its group as one undoable action', async () => {
|
||||
routeMock.meta = { nodeView: true };
|
||||
useWorkflowsListStore().addWorkflow(
|
||||
|
|
@ -157,8 +222,10 @@ describe('NodeView', () => {
|
|||
const anchor = workflowDocumentStore.allNodes[0];
|
||||
expect(anchor).toMatchObject({
|
||||
type: NO_OP_NODE_TYPE,
|
||||
name: 'No Operation, do nothing',
|
||||
position: [320, 240],
|
||||
parameters: { emptyGroupAnchor: true },
|
||||
placeholder: true,
|
||||
});
|
||||
expect(workflowDocumentStore.allGroups[0]).toMatchObject({
|
||||
name: 'Group 1',
|
||||
|
|
@ -203,6 +270,51 @@ describe('NodeView', () => {
|
|||
expect(workflowDocumentStore.allNodes).toHaveLength(1);
|
||||
expect(useHistoryStore().undoStack).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('replaces the empty-group anchor with the selected batch and restores it on undo', async () => {
|
||||
routeMock.meta = { nodeView: true };
|
||||
useWorkflowsListStore().addWorkflow(
|
||||
createTestWorkflow({ id: 'w0', scopes: ['workflow:read', 'workflow:update'] }),
|
||||
);
|
||||
addReplacementNodeTypes();
|
||||
const { findByTestId } = renderNodeView();
|
||||
|
||||
await userEvent.click(await findByTestId('node-creation-stub-add-empty-group'));
|
||||
await waitFor(() => expect(workflowDocumentStore.allGroups).toHaveLength(1));
|
||||
const anchorId = workflowDocumentStore.allNodes[0].id;
|
||||
|
||||
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(2));
|
||||
expect(workflowDocumentStore.getNodeById(anchorId)).toBeUndefined();
|
||||
expect(workflowDocumentStore.allGroups[0].nodeIds).toEqual(
|
||||
workflowDocumentStore.allNodes.map((node) => node.id),
|
||||
);
|
||||
expect(workflowDocumentStore.allNodes.map((node) => node.name)).toEqual([
|
||||
'Loop Over Items',
|
||||
'Replace Me',
|
||||
]);
|
||||
|
||||
const historyStore = useHistoryStore();
|
||||
expect(historyStore.undoStack).toHaveLength(2);
|
||||
const replacementUndo = historyStore.popUndoableToUndo();
|
||||
expect(replacementUndo).toBeInstanceOf(BulkCommand);
|
||||
if (!(replacementUndo instanceof BulkCommand)) {
|
||||
throw new Error('Expected a bulk history action');
|
||||
}
|
||||
|
||||
historyStore.bulkInProgress = true;
|
||||
for (let index = replacementUndo.commands.length - 1; index >= 0; index--) {
|
||||
await replacementUndo.commands[index].revert();
|
||||
}
|
||||
await nextTick();
|
||||
historyStore.bulkInProgress = false;
|
||||
|
||||
await waitFor(() => expect(workflowDocumentStore.allNodes).toHaveLength(1));
|
||||
expect(workflowDocumentStore.allNodes[0].id).toBe(anchorId);
|
||||
expect(workflowDocumentStore.allGroups[0].nodeIds).toEqual([anchorId]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Trigger node selection', () => {
|
||||
|
|
|
|||
|
|
@ -528,8 +528,8 @@ function onDeleteNode(id: string) {
|
|||
}
|
||||
}
|
||||
|
||||
function onDeleteNodes(ids: string[]) {
|
||||
deleteNodes(ids);
|
||||
function onDeleteNodes(ids: string[], deleteWholeGroups = false) {
|
||||
deleteNodes(ids, { preserveEmptyGroupAnchor: !deleteWholeGroups });
|
||||
}
|
||||
|
||||
function onRevertDeleteNode({ node }: { node: INodeUi }) {
|
||||
|
|
@ -1039,8 +1039,10 @@ async function onAddEmptyGroup(position: XYPosition, connectToLastInteractedNode
|
|||
[
|
||||
{
|
||||
type: NO_OP_NODE_TYPE,
|
||||
name: 'No Operation, do nothing',
|
||||
position,
|
||||
parameters: { emptyGroupAnchor: true },
|
||||
placeholder: true,
|
||||
isAutoAdd: !connectToLastInteractedNode,
|
||||
openDetail: false,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -1130,6 +1130,32 @@ describe('Canvas', () => {
|
|||
await waitFor(() => expect(selectedIds(vueFlow)).toEqual(['node-1']));
|
||||
});
|
||||
|
||||
it('keeps a sole expanded group member selectable without selecting the title bar', async () => {
|
||||
workflowDocumentStore.setNodeGroups([{ id: 'g1', name: 'Group 1', nodeIds: ['node-1'] }]);
|
||||
const rendered = renderComponent({
|
||||
props: {
|
||||
nodes: [
|
||||
createCanvasNodeElement({ id: 'node-1', label: 'Node 1' }),
|
||||
createCanvasGroupNode({ nodeIds: ['node-1'] }),
|
||||
],
|
||||
},
|
||||
global: {
|
||||
provide: { [NodeGroupViewKey as symbol]: createNodeGroupViewMock(false) },
|
||||
},
|
||||
});
|
||||
|
||||
await waitFor(() =>
|
||||
expect(rendered.container.querySelectorAll('.vue-flow__node')).toHaveLength(2),
|
||||
);
|
||||
|
||||
const vueFlow = useVueFlow(canvasId);
|
||||
vueFlow.addSelectedNodes([vueFlow.findNode('node-1')!]);
|
||||
|
||||
await waitFor(() =>
|
||||
expect(vueFlow.getSelectedNodes.value.map(({ id }) => id)).toEqual(['node-1']),
|
||||
);
|
||||
});
|
||||
|
||||
it('extends the selection to members when a selected group is expanded', async () => {
|
||||
const { vueFlow, getByTestId } = await setupExpandedGroup(true);
|
||||
|
||||
|
|
@ -1605,7 +1631,7 @@ describe('Canvas', () => {
|
|||
await fireEvent.keyDown(document, { key: 'Backspace' });
|
||||
await fireEvent.keyUp(document, { key: 'Backspace' });
|
||||
|
||||
expect(emitted()['delete:nodes']?.[0]).toEqual([['a', 'b']]);
|
||||
expect(emitted()['delete:nodes']?.[0]).toEqual([['a', 'b'], true]);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -1659,6 +1685,40 @@ describe('Canvas', () => {
|
|||
// The selection now folds into a single element — the box goes away
|
||||
expect(nodesSelectionActive.value).toBe(false);
|
||||
});
|
||||
|
||||
it('folds an already-selected single node into a newly created group', async () => {
|
||||
workflowDocumentStore.setScopes(['workflow:update']);
|
||||
workflowDocumentStore.setNodes([createTestNode({ id: 'a', name: 'Node A' })]);
|
||||
const member = createCanvasNodeElement({ id: 'a', label: 'Node A' });
|
||||
const rendered = renderComponent({
|
||||
props: { nodes: [member] },
|
||||
global: {
|
||||
provide: { [NodeGroupViewKey as symbol]: createNodeGroupViewMock(false) },
|
||||
},
|
||||
});
|
||||
await waitFor(() =>
|
||||
expect(rendered.container.querySelectorAll('.vue-flow__node')).toHaveLength(1),
|
||||
);
|
||||
|
||||
const { addSelectedNodes, findNode, getSelectedNodes } = useVueFlow(canvasId);
|
||||
addSelectedNodes([findNode('a')!]);
|
||||
await waitFor(() => expect(getSelectedNodes.value.map(({ id }) => id)).toEqual(['a']));
|
||||
|
||||
const group = workflowDocumentStore.createGroup(['a'], 'My Group');
|
||||
await rendered.rerender({
|
||||
nodes: [
|
||||
createCanvasGroupElement({ id: group.id, name: group.name, nodeIds: ['a'] }),
|
||||
member,
|
||||
],
|
||||
});
|
||||
|
||||
await waitFor(() =>
|
||||
expect([...getSelectedNodes.value.map(({ id }) => id)].sort()).toEqual([
|
||||
'a',
|
||||
`group:${group.id}`,
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('group context menu', () => {
|
||||
|
|
|
|||
|
|
@ -147,7 +147,7 @@ const emit = defineEmits<{
|
|||
'replace:node': [id: string];
|
||||
'create:node': [source: NodeCreatorOpenSource];
|
||||
'create:sticky': [];
|
||||
'delete:nodes': [ids: string[]];
|
||||
'delete:nodes': [ids: string[], deleteWholeGroups?: boolean];
|
||||
'update:nodes:enabled': [ids: string[]];
|
||||
'copy:nodes': [ids: string[]];
|
||||
'duplicate:nodes': [ids: string[]];
|
||||
|
|
@ -1121,8 +1121,10 @@ function onKeyboardSetGroupsExpanded(expanded: boolean) {
|
|||
*/
|
||||
function onDeleteSelection() {
|
||||
const ids = selectedNodeIdsWithGroupMembers.value;
|
||||
// Removing the last group member also deletes the group via the document store
|
||||
if (ids.length > 0) emit('delete:nodes', ids);
|
||||
// Expand selected groups to their member nodes before deletion.
|
||||
if (ids.length > 0) {
|
||||
emit('delete:nodes', ids, selectedNodesAndGroups.value.some(isCanvasGroupNode));
|
||||
}
|
||||
}
|
||||
|
||||
// Last header-click toggle, for double-click suppression in onNodeClick.
|
||||
|
|
@ -1625,7 +1627,11 @@ async function onContextMenuAction(action: ContextMenuAction, nodeIds: string[],
|
|||
case 'copy':
|
||||
return emit('copy:nodes', nodeIds);
|
||||
case 'delete':
|
||||
return emit('delete:nodes', nodeIds);
|
||||
return emit(
|
||||
'delete:nodes',
|
||||
nodeIds,
|
||||
Boolean(groupId) || selectedNodesAndGroups.value.some(isCanvasGroupNode),
|
||||
);
|
||||
case 'select_all':
|
||||
return onSelectAllNodes();
|
||||
case 'deselect_all':
|
||||
|
|
|
|||
|
|
@ -103,6 +103,15 @@ export function useCanvasNodeGroupSelection(deps: UseCanvasNodeGroupSelectionDep
|
|||
}
|
||||
for (const group of groupsToCheck.values()) {
|
||||
const groupNodeId = createCanvasGroupNodeId(group.id);
|
||||
// A one-member expanded group must not swallow a direct click on its
|
||||
// only member. The title bar still selects the group explicitly (the
|
||||
// first reconciliation pass adds the member), but selecting the member
|
||||
// alone should leave the member as the selection target. A group that
|
||||
// mounts around an already-selected member is a grouping operation, so
|
||||
// it must still fold the selection into the new group.
|
||||
const isDirectMemberSelection =
|
||||
group.nodeIds.length === 1 && !target.has(groupNodeId) && added.includes(group.nodeIds[0]);
|
||||
if (isDirectMemberSelection) continue;
|
||||
if (group.nodeIds.every((memberId) => target.has(memberId))) {
|
||||
target.add(groupNodeId);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -140,7 +140,7 @@ test.describe(
|
|||
await expect(n8n.canvas.getNodeGroupTitle(DEFAULT_GROUP_TITLE)).toBeVisible();
|
||||
});
|
||||
|
||||
test('removes the group when the last member is deleted', async ({ n8n }) => {
|
||||
test('keeps the group when the last real member is deleted', async ({ n8n }) => {
|
||||
await n8n.canvas.selectNodes(['Set A', 'Set B']);
|
||||
await n8n.canvas.selectionToolbar.groupButton().click();
|
||||
await n8n.canvas.deselectAll();
|
||||
|
|
@ -148,7 +148,8 @@ test.describe(
|
|||
|
||||
await n8n.canvas.deleteNodeFromContextMenu('Set B');
|
||||
await n8n.canvas.deleteNodeFromContextMenu('Set A');
|
||||
await expect(n8n.canvas.getNodeGroups()).toHaveCount(0);
|
||||
await expect(n8n.canvas.getNodeGroups()).toHaveCount(1);
|
||||
await expect(n8n.canvas.getNodeGroupTitle(DEFAULT_GROUP_TITLE)).toBeVisible();
|
||||
});
|
||||
|
||||
test('keeps the group when one of three members is deleted', async ({ n8n }) => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user