mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-11 22:50:06 +00:00
fix(editor): Preserve canvas groups when converting a selection to a sub-workflow (#34664)
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
Miguel Ángel Moreno
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
0147aa7d6a
commit
1666f1ec67
@@ -1,6 +1,6 @@
|
||||
import { setActivePinia } from 'pinia';
|
||||
import { createTestingPinia } from '@pinia/testing';
|
||||
import type { IConnections, INode } from 'n8n-workflow';
|
||||
import type { IConnections, INode, IWorkflowGroup } from 'n8n-workflow';
|
||||
import { NodeConnectionTypes } from 'n8n-workflow';
|
||||
import type { INodeUi } from '@/Interface';
|
||||
import type { WorkflowDataCreate } from '@n8n/rest-api-client/api/workflows';
|
||||
@@ -22,6 +22,7 @@ const {
|
||||
},
|
||||
mockWorkflowDocumentStore: {
|
||||
allNodes: [] as INodeUi[],
|
||||
allGroups: [] as IWorkflowGroup[],
|
||||
connectionsBySourceNode: {} as IConnections,
|
||||
homeProject: { id: 'home-project' },
|
||||
parentFolder: null as { id: string } | null,
|
||||
@@ -189,6 +190,7 @@ describe('useWorkflowExtraction', () => {
|
||||
mockHistoryStore.stopRecordingUndo.mockClear();
|
||||
mockHistoryStore.pushCommandToUndo.mockClear();
|
||||
mockWorkflowDocumentStore.allNodes = [];
|
||||
mockWorkflowDocumentStore.allGroups = [];
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {};
|
||||
});
|
||||
|
||||
@@ -572,4 +574,147 @@ describe('useWorkflowExtraction', () => {
|
||||
expect(mockCanvasOperations.deleteNodes.mock.calls[0][0]).toEqual([agentB.id]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('sub-workflow node groups', () => {
|
||||
it('preserves a group when the selection includes that group plus extra nodes', async () => {
|
||||
const nodeA = makeNode('A', [0, 0]);
|
||||
const nodeB = makeNode('B', [200, 0]);
|
||||
const nodeC = makeNode('C', [400, 0]); // extra node outside the group
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [nodeA.id, nodeB.id] };
|
||||
|
||||
setWorkflowNodes([nodeA, nodeB, nodeC]);
|
||||
mockWorkflowDocumentStore.allGroups = [group];
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {
|
||||
A: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
B: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'C', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
mockSuccessfulWorkflowCreation();
|
||||
|
||||
const { extractNodesIntoSubworkflow } = useWorkflowExtraction();
|
||||
|
||||
await extractNodesIntoSubworkflow({ start: 'A', end: 'C' }, [nodeA, nodeB, nodeC], 'Sub');
|
||||
|
||||
const created = mockWorkflowsStore.createNewWorkflow.mock.calls[0][0] as WorkflowDataCreate;
|
||||
expect(created.nodeGroups).toHaveLength(1);
|
||||
expect(created.nodeGroups?.[0].name).toBe('Group 1');
|
||||
expect(created.nodeGroups?.[0].nodeIds).toEqual([nodeA.id, nodeB.id]);
|
||||
// A fresh id is minted so the sub-workflow copy doesn't share the parent group id.
|
||||
expect(created.nodeGroups?.[0].id).not.toBe(group.id);
|
||||
});
|
||||
|
||||
it('preserves every group when the selection spans multiple groups', async () => {
|
||||
const nodeA1 = makeNode('A1', [0, 0]);
|
||||
const nodeA2 = makeNode('A2', [200, 0]);
|
||||
const nodeB1 = makeNode('B1', [400, 0]);
|
||||
const nodeB2 = makeNode('B2', [600, 0]);
|
||||
const groupA = { id: 'gA', name: 'Group A', nodeIds: [nodeA1.id, nodeA2.id] };
|
||||
const groupB = { id: 'gB', name: 'Group B', nodeIds: [nodeB1.id, nodeB2.id] };
|
||||
|
||||
setWorkflowNodes([nodeA1, nodeA2, nodeB1, nodeB2]);
|
||||
mockWorkflowDocumentStore.allGroups = [groupA, groupB];
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {
|
||||
A1: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'A2', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
A2: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B1', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
B1: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B2', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
mockSuccessfulWorkflowCreation();
|
||||
|
||||
const { extractNodesIntoSubworkflow } = useWorkflowExtraction();
|
||||
|
||||
await extractNodesIntoSubworkflow(
|
||||
{ start: 'A1', end: 'B2' },
|
||||
[nodeA1, nodeA2, nodeB1, nodeB2],
|
||||
'Sub',
|
||||
);
|
||||
|
||||
const created = mockWorkflowsStore.createNewWorkflow.mock.calls[0][0] as WorkflowDataCreate;
|
||||
expect(created.nodeGroups).toHaveLength(2);
|
||||
const byName = Object.fromEntries((created.nodeGroups ?? []).map((g) => [g.name, g]));
|
||||
expect(byName['Group A'].nodeIds).toEqual([nodeA1.id, nodeA2.id]);
|
||||
expect(byName['Group B'].nodeIds).toEqual([nodeB1.id, nodeB2.id]);
|
||||
});
|
||||
|
||||
it('does not recreate the group when the whole selection is exactly the same as that group', async () => {
|
||||
const nodeA = makeNode('A', [0, 0]);
|
||||
const nodeB = makeNode('B', [200, 0]);
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [nodeA.id, nodeB.id] };
|
||||
|
||||
setWorkflowNodes([nodeA, nodeB]);
|
||||
mockWorkflowDocumentStore.allGroups = [group];
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {
|
||||
A: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
mockSuccessfulWorkflowCreation();
|
||||
|
||||
const { extractNodesIntoSubworkflow } = useWorkflowExtraction();
|
||||
|
||||
await extractNodesIntoSubworkflow({ start: 'A', end: 'B' }, [nodeA, nodeB], 'Sub');
|
||||
|
||||
const created = mockWorkflowsStore.createNewWorkflow.mock.calls[0][0] as WorkflowDataCreate;
|
||||
expect(created.nodeGroups).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not recreate a group that is only partially in the selection', async () => {
|
||||
const nodeA = makeNode('A', [0, 0]);
|
||||
const nodeB = makeNode('B', [200, 0]);
|
||||
const nodeC = makeNode('C', [400, 0]); // group member left behind
|
||||
const group = { id: 'g1', name: 'Group 1', nodeIds: [nodeA.id, nodeB.id, nodeC.id] };
|
||||
|
||||
setWorkflowNodes([nodeA, nodeB, nodeC]);
|
||||
mockWorkflowDocumentStore.allGroups = [group];
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {
|
||||
A: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
B: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'C', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
mockSuccessfulWorkflowCreation();
|
||||
|
||||
const { extractNodesIntoSubworkflow } = useWorkflowExtraction();
|
||||
|
||||
await extractNodesIntoSubworkflow({ start: 'A', end: 'B' }, [nodeA, nodeB], 'Sub');
|
||||
|
||||
const created = mockWorkflowsStore.createNewWorkflow.mock.calls[0][0] as WorkflowDataCreate;
|
||||
expect(created.nodeGroups).toEqual([]);
|
||||
});
|
||||
|
||||
it('creates no groups when the workflow has none', async () => {
|
||||
const nodeA = makeNode('A', [0, 0]);
|
||||
const nodeB = makeNode('B', [200, 0]);
|
||||
|
||||
setWorkflowNodes([nodeA, nodeB]);
|
||||
mockWorkflowDocumentStore.connectionsBySourceNode = {
|
||||
A: {
|
||||
[NodeConnectionTypes.Main]: [[{ node: 'B', type: NodeConnectionTypes.Main, index: 0 }]],
|
||||
},
|
||||
};
|
||||
|
||||
mockSuccessfulWorkflowCreation();
|
||||
|
||||
const { extractNodesIntoSubworkflow } = useWorkflowExtraction();
|
||||
|
||||
await extractNodesIntoSubworkflow({ start: 'A', end: 'B' }, [nodeA, nodeB], 'Sub');
|
||||
|
||||
const created = mockWorkflowsStore.createNewWorkflow.mock.calls[0][0] as WorkflowDataCreate;
|
||||
expect(created.nodeGroups).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
ExtractableErrorResult,
|
||||
IConnections,
|
||||
INode,
|
||||
IWorkflowGroup,
|
||||
} from 'n8n-workflow';
|
||||
import { useToast } from './useToast';
|
||||
import { useRouter } from 'vue-router';
|
||||
@@ -132,6 +133,42 @@ export function useWorkflowExtraction() {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Copies the parent workflow's groups that should survive extraction into the
|
||||
* new sub-workflow. A group is carried over only when every one of its members
|
||||
* is extracted (a partial subset may no longer form a valid group) AND the
|
||||
* selection contains at least one node outside the group. The whole-group-only
|
||||
* case is intentionally excluded: the nodes become the sub-workflow itself and
|
||||
* the parent group is dissolved separately (ADO-5580). A fresh id is minted so
|
||||
* the copy never collides with the group still present in the parent.
|
||||
*/
|
||||
function computeSubworkflowNodeGroups(extractedNodes: INodeUi[]): IWorkflowGroup[] {
|
||||
const extractedIds = new Set(extractedNodes.map((node) => node.id));
|
||||
const groups: IWorkflowGroup[] = [];
|
||||
|
||||
for (const group of workflowDocumentStore.value.allGroups) {
|
||||
const fullyContained = group.nodeIds.every((id) => extractedIds.has(id));
|
||||
|
||||
if (!fullyContained) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const hasExtraOutsideGroup = extractedIds.size > group.nodeIds.length;
|
||||
if (!hasExtraOutsideGroup) {
|
||||
continue;
|
||||
}
|
||||
|
||||
groups.push({
|
||||
id: window.crypto.randomUUID(),
|
||||
name: group.name,
|
||||
nodeIds: [...group.nodeIds],
|
||||
...(group.description ? { description: group.description } : {}),
|
||||
});
|
||||
}
|
||||
|
||||
return groups;
|
||||
}
|
||||
|
||||
function makeSubworkflow(
|
||||
newWorkflowName: string,
|
||||
{ start, end }: ExtractableSubgraphData,
|
||||
@@ -262,6 +299,7 @@ export function useWorkflowExtraction() {
|
||||
settings: { executionOrder: 'v1' },
|
||||
projectId: workflowDocumentStore.value.homeProject?.id,
|
||||
parentFolderId: workflowDocumentStore.value.parentFolder?.id ?? undefined,
|
||||
nodeGroups: computeSubworkflowNodeGroups(nodes),
|
||||
};
|
||||
result.connections = sanitizeConnections(
|
||||
result.connections,
|
||||
|
||||
Reference in New Issue
Block a user