mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-28 03:24:59 +02:00
feat(editor): Preserve and author workflow node groups across update surfaces (no-changelog) (#32771)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8284c1aa64
commit
511e34fd7f
|
|
@ -247,8 +247,14 @@ export class ParseValidateHandler {
|
|||
builder.generatePinData({ beforeWorkflow: currentWorkflow });
|
||||
}
|
||||
|
||||
// Preserve IDs of groups that already exist (by name) so editing a workflow doesn't
|
||||
// skew the diff; new groups fall back to a deterministic ID.
|
||||
const existingGroupIdsByName = new Map(
|
||||
(currentWorkflow?.nodeGroups ?? []).map((group) => [group.name, group.id]),
|
||||
);
|
||||
|
||||
// Convert to JSON with Dagre layout matching the FE's tidy-up
|
||||
const workflowJson: WorkflowJSON = builder.toJSON({ tidyUp: true });
|
||||
const workflowJson: WorkflowJSON = builder.toJSON({ tidyUp: true, existingGroupIdsByName });
|
||||
|
||||
this.logger?.debug('Parsed workflow', {
|
||||
id: workflowJson.id,
|
||||
|
|
|
|||
|
|
@ -276,6 +276,39 @@ describe('ParseValidateHandler', () => {
|
|||
expect(mockBuilder.regenerateNodeIds).toHaveBeenCalledWith(new Map());
|
||||
});
|
||||
|
||||
it('should preserve existing group IDs (matched by name) when serializing', async () => {
|
||||
const currentWorkflow = {
|
||||
id: 'current',
|
||||
name: 'Current',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [
|
||||
{ id: 'group-random-1', name: 'Ingestion', nodeIds: [] },
|
||||
{ id: 'group-random-2', name: 'Processing', nodeIds: [] },
|
||||
],
|
||||
} as unknown as WorkflowJSON;
|
||||
|
||||
const mockBuilder = {
|
||||
regenerateNodeIds: vi.fn(),
|
||||
validate: vi.fn().mockReturnValue({ valid: true, errors: [], warnings: [] }),
|
||||
generatePinData: vi.fn(),
|
||||
toJSON: vi.fn().mockReturnValue({ id: 'test', name: 'Test', nodes: [], connections: {} }),
|
||||
};
|
||||
|
||||
mockParseWorkflowCodeToBuilder.mockReturnValue(mockBuilder);
|
||||
mockValidateWorkflow.mockReturnValue({ valid: true, errors: [], warnings: [] });
|
||||
|
||||
await handler.parseAndValidate('code', currentWorkflow);
|
||||
|
||||
expect(mockBuilder.toJSON).toHaveBeenCalledWith({
|
||||
tidyUp: true,
|
||||
existingGroupIdsByName: new Map([
|
||||
['Ingestion', 'group-random-1'],
|
||||
['Processing', 'group-random-2'],
|
||||
]),
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw on parse error', async () => {
|
||||
mockParseWorkflowCodeToBuilder.mockImplementation(() => {
|
||||
throw new Error('Syntax error at line 5');
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import type { WorkflowJSON } from '@n8n/workflow-sdk';
|
||||
|
||||
import type { InstanceAiContext } from '../../../types';
|
||||
import { ensureWebhookIds } from '../workflow-json-utils';
|
||||
import { ensureWebhookIds, preserveExistingNodeGroupIds } from '../workflow-json-utils';
|
||||
|
||||
describe('ensureWebhookIds', () => {
|
||||
it('fails updates when existing webhook IDs cannot be loaded', async () => {
|
||||
|
|
@ -31,3 +31,72 @@ describe('ensureWebhookIds', () => {
|
|||
expect(workflow.nodes[0]?.webhookId).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('preserveExistingNodeGroupIds', () => {
|
||||
const buildWorkflow = (groupId: string): WorkflowJSON => ({
|
||||
name: 'Grouped workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: groupId, name: 'Group 1', nodeIds: ['node-a'] }],
|
||||
});
|
||||
|
||||
const contextWithExisting = (nodeGroups: Array<{ id: string; name: string }>) =>
|
||||
({
|
||||
workflowService: {
|
||||
getAsWorkflowJSON: vi.fn().mockResolvedValue({
|
||||
name: 'Grouped workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: nodeGroups.map((g) => ({ ...g, nodeIds: [] })),
|
||||
}),
|
||||
},
|
||||
}) as unknown as InstanceAiContext;
|
||||
|
||||
it('reuses the existing group id when the group name matches', async () => {
|
||||
const workflow = buildWorkflow('deterministic-id');
|
||||
|
||||
await preserveExistingNodeGroupIds(
|
||||
workflow,
|
||||
'wf-1',
|
||||
contextWithExisting([{ id: 'editor-id', name: 'Group 1' }]),
|
||||
);
|
||||
|
||||
expect(workflow.nodeGroups?.[0]?.id).toBe('editor-id');
|
||||
});
|
||||
|
||||
it('keeps the generated id for a group not present in the existing workflow', async () => {
|
||||
const workflow = buildWorkflow('deterministic-id');
|
||||
|
||||
await preserveExistingNodeGroupIds(
|
||||
workflow,
|
||||
'wf-1',
|
||||
contextWithExisting([{ id: 'other-id', name: 'Some Other Group' }]),
|
||||
);
|
||||
|
||||
expect(workflow.nodeGroups?.[0]?.id).toBe('deterministic-id');
|
||||
});
|
||||
|
||||
it('does not fetch or modify groups for new workflows (no workflowId)', async () => {
|
||||
const workflow = buildWorkflow('deterministic-id');
|
||||
const getAsWorkflowJSON = vi.fn();
|
||||
const context = { workflowService: { getAsWorkflowJSON } } as unknown as InstanceAiContext;
|
||||
|
||||
await preserveExistingNodeGroupIds(workflow, undefined, context);
|
||||
|
||||
expect(getAsWorkflowJSON).not.toHaveBeenCalled();
|
||||
expect(workflow.nodeGroups?.[0]?.id).toBe('deterministic-id');
|
||||
});
|
||||
|
||||
it('fails updates when the existing workflow cannot be loaded', async () => {
|
||||
const workflow = buildWorkflow('deterministic-id');
|
||||
const context = {
|
||||
workflowService: {
|
||||
getAsWorkflowJSON: vi.fn().mockRejectedValue(new Error('Workflow not found')),
|
||||
},
|
||||
} as unknown as InstanceAiContext;
|
||||
|
||||
await expect(preserveExistingNodeGroupIds(workflow, 'wf-1', context)).rejects.toThrow(
|
||||
'Failed to load existing workflow wf-1 to preserve node-group IDs: Workflow not found',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ import {
|
|||
ensureWebhookIds,
|
||||
getReferencedWorkflowIds,
|
||||
isTriggerNodeType,
|
||||
preserveExistingNodeGroupIds,
|
||||
} from './workflow-json-utils';
|
||||
import { compileWorkflowSource } from './workflow-source-compiler';
|
||||
import { partitionWarnings, type ValidationWarning } from './workflow-validation-warnings';
|
||||
|
|
@ -521,6 +522,7 @@ export function createBuildWorkflowTool(context: InstanceAiContext) {
|
|||
|
||||
try {
|
||||
await ensureWebhookIds(json, targetWorkflowId, context);
|
||||
await preserveExistingNodeGroupIds(json, targetWorkflowId, context);
|
||||
|
||||
const hasMockedCredentialNodes = mockResult.mockedNodeNames.length > 0;
|
||||
const referencedWorkflowIds = getReferencedWorkflowIds(json);
|
||||
|
|
|
|||
|
|
@ -95,3 +95,38 @@ export async function ensureWebhookIds(
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* For updates, preserve existing node-group IDs by group name. The sandbox SDK
|
||||
* build has no view of the saved workflow, so toJSON() mints a fresh deterministic
|
||||
* ID for every group — overwriting the stable ID of a group the user created in
|
||||
* the editor. Reconciling by name here keeps it stable, mirroring ensureWebhookIds.
|
||||
*/
|
||||
export async function preserveExistingNodeGroupIds(
|
||||
json: WorkflowJSON,
|
||||
workflowId: string | undefined,
|
||||
ctx: InstanceAiContext,
|
||||
): Promise<void> {
|
||||
if (!workflowId || !json.nodeGroups?.length) return;
|
||||
|
||||
let existingGroupIdsByName: Map<string, string>;
|
||||
try {
|
||||
const existing = await ctx.workflowService.getAsWorkflowJSON(workflowId);
|
||||
existingGroupIdsByName = new Map(
|
||||
(existing.nodeGroups ?? []).map((group): [string, string] => [group.name, group.id]),
|
||||
);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
throw new Error(
|
||||
`Failed to load existing workflow ${workflowId} to preserve node-group IDs: ${message}`,
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
|
||||
for (const group of json.nodeGroups) {
|
||||
const existingId = existingGroupIdsByName.get(group.name);
|
||||
if (existingId) {
|
||||
group.id = existingId;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,6 +82,7 @@ export const SDK_METHODS: readonly SdkMethodSpec[] = [
|
|||
// Workflow builder methods
|
||||
{ name: 'add', group: 'workflow', public: true },
|
||||
{ name: 'to', group: 'workflow', public: true },
|
||||
{ name: 'group', group: 'workflow', public: true },
|
||||
|
||||
// Node builder methods
|
||||
{ name: 'input', group: 'node', public: true },
|
||||
|
|
|
|||
|
|
@ -1446,6 +1446,22 @@ export function generateCode(
|
|||
}
|
||||
}
|
||||
|
||||
// Emit node group definitions. Members are referenced by node variable (the declared
|
||||
// `const`), so a workflow → code → build round-trip preserves groups.
|
||||
if (json.nodeGroups && json.nodeGroups.length > 0) {
|
||||
const idToNodeName = new Map<string, string>();
|
||||
for (const node of json.nodes) {
|
||||
if (node.name !== undefined) idToNodeName.set(node.id, node.name);
|
||||
}
|
||||
for (const group of json.nodeGroups) {
|
||||
const memberVars = group.nodeIds.flatMap((id) => {
|
||||
const nodeName = idToNodeName.get(id);
|
||||
return nodeName !== undefined ? [getVarName(nodeName, ctx)] : [];
|
||||
});
|
||||
workflowCalls.push(` .group('${escapeString(group.name)}', [${memberVars.join(', ')}])`);
|
||||
}
|
||||
}
|
||||
|
||||
// Add workflow calls and return statement
|
||||
lines.push('');
|
||||
|
||||
|
|
|
|||
|
|
@ -2748,6 +2748,22 @@ describe('Codegen Roundtrip with Real Workflows', () => {
|
|||
expect(parsedJson.settings).toEqual(json.settings);
|
||||
}
|
||||
|
||||
// Verify node groups survive the roundtrip. Node and group ids are
|
||||
// regenerated deterministically on parse, so compare each group by name
|
||||
// and the names of its member nodes rather than by raw id.
|
||||
if (json.nodeGroups && json.nodeGroups.length > 0) {
|
||||
const groupsByMemberName = (wf: WorkflowJSON) => {
|
||||
const nameById = new Map(wf.nodes.map((n) => [n.id, n.name]));
|
||||
return (wf.nodeGroups ?? [])
|
||||
.map((g) => ({
|
||||
name: g.name,
|
||||
members: g.nodeIds.flatMap((id) => nameById.get(id) ?? []).sort(),
|
||||
}))
|
||||
.sort((a, b) => a.name.localeCompare(b.name));
|
||||
};
|
||||
expect(groupsByMemberName(parsedJson)).toEqual(groupsByMemberName(json));
|
||||
}
|
||||
|
||||
// Filter connections from non-existent nodes (orphaned connections in original workflow)
|
||||
const validNodeNames = new Set(
|
||||
json.nodes.map((n) => n.name).filter((name): name is string => !!name),
|
||||
|
|
|
|||
287
packages/@n8n/workflow-sdk/src/node-groups.test.ts
Normal file
287
packages/@n8n/workflow-sdk/src/node-groups.test.ts
Normal file
|
|
@ -0,0 +1,287 @@
|
|||
import { generateWorkflowCode } from './codegen';
|
||||
import { parseWorkflowCodeToBuilder } from './codegen/parse-workflow-code';
|
||||
import type { WorkflowJSON } from './types/base';
|
||||
import { workflow } from './workflow-builder';
|
||||
import { node, trigger } from './workflow-builder/node-builders/node-builder';
|
||||
import { generateDeterministicGroupId } from './workflow-builder/string-utils';
|
||||
|
||||
const WF_ID = 'wf-groups-1';
|
||||
|
||||
function buildGroupedWorkflow() {
|
||||
const start = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
const fetchNode = node({
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
version: 4.2,
|
||||
config: { name: 'Fetch data' },
|
||||
});
|
||||
const transform = node({
|
||||
type: 'n8n-nodes-base.set',
|
||||
version: 3,
|
||||
config: { name: 'Transform' },
|
||||
});
|
||||
|
||||
return workflow(WF_ID, 'Grouped workflow')
|
||||
.add(start)
|
||||
.to(fetchNode)
|
||||
.to(transform)
|
||||
.group('Ingestion', [fetchNode, transform]);
|
||||
}
|
||||
|
||||
describe('SDK node groups', () => {
|
||||
describe('builder.group() → toJSON()', () => {
|
||||
it('emits nodeGroups with a deterministic id and member ids resolved from names', () => {
|
||||
const json = buildGroupedWorkflow().toJSON();
|
||||
|
||||
expect(json.nodeGroups).toHaveLength(1);
|
||||
const group = json.nodeGroups![0];
|
||||
expect(group.name).toBe('Ingestion');
|
||||
expect(group.id).toBe(generateDeterministicGroupId(WF_ID, 'Ingestion'));
|
||||
|
||||
const idByName = new Map(json.nodes.map((n) => [n.name, n.id]));
|
||||
expect(group.nodeIds).toEqual([idByName.get('Fetch data'), idByName.get('Transform')]);
|
||||
});
|
||||
|
||||
it('drops members that do not resolve to a node in the workflow', () => {
|
||||
const start = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
const a = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } });
|
||||
// `orphan` is never added to the workflow, so it resolves to nothing and is dropped.
|
||||
const orphan = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'Orphan' } });
|
||||
|
||||
const json = workflow(WF_ID, 'wf').add(start).to(a).group('G', [a, orphan]).toJSON();
|
||||
|
||||
const idByName = new Map(json.nodes.map((n) => [n.name, n.id]));
|
||||
expect(json.nodeGroups![0].nodeIds).toEqual([idByName.get('A')]);
|
||||
});
|
||||
|
||||
it('omits nodeGroups entirely when no group was declared', () => {
|
||||
const start = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
const json = workflow(WF_ID, 'wf').add(start).toJSON();
|
||||
expect(json.nodeGroups).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('regenerateNodeIds()', () => {
|
||||
it('keeps group member ids consistent with the regenerated node ids (never dangling)', () => {
|
||||
const builder = buildGroupedWorkflow();
|
||||
builder.regenerateNodeIds();
|
||||
const json = builder.toJSON();
|
||||
|
||||
const emittedIds = new Set(json.nodes.map((n) => n.id));
|
||||
expect(json.nodeGroups![0].nodeIds.length).toBe(2);
|
||||
for (const id of json.nodeGroups![0].nodeIds) {
|
||||
expect(emittedIds.has(id)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('resolves a grouped auto-renamed duplicate handle to the right node after regeneration', () => {
|
||||
const start = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
// Two nodes named "A": the builder auto-renames the second to "A 1".
|
||||
const first = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } });
|
||||
const second = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } });
|
||||
|
||||
const builder = workflow(WF_ID, 'wf').add(start).to(first).to(second).group('G', [second]); // group the SECOND duplicate by handle
|
||||
builder.regenerateNodeIds();
|
||||
const json = builder.toJSON();
|
||||
|
||||
const renamed = json.nodes.find((n) => n.name === 'A 1');
|
||||
expect(renamed).toBeDefined();
|
||||
// The group must reference the renamed duplicate, not the first "A".
|
||||
expect(json.nodeGroups![0].nodeIds).toEqual([renamed!.id]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fromJSON() → toJSON() round-trip', () => {
|
||||
it('preserves the group id and members across a round-trip', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: WF_ID,
|
||||
name: 'wf',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'id-b',
|
||||
name: 'B',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [10, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } },
|
||||
// A UI-assigned (non-deterministic) id, as the canvas creates.
|
||||
nodeGroups: [{ id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'id-b'] }],
|
||||
};
|
||||
|
||||
const json = workflow.fromJSON(source).toJSON();
|
||||
|
||||
// Compare the whole group so a dropped/changed field (e.g. id) is caught.
|
||||
// The source id is carried through, not re-derived — round-trip is lossless.
|
||||
expect(json.nodeGroups).toEqual([
|
||||
{ id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'id-b'] },
|
||||
]);
|
||||
});
|
||||
|
||||
it('drops a group member whose id has no matching node', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: WF_ID,
|
||||
name: 'wf',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
// `ghost-id` references no node, so it is dropped on import.
|
||||
nodeGroups: [{ id: 'random-uuid', name: 'G', nodeIds: ['id-a', 'ghost-id'] }],
|
||||
};
|
||||
|
||||
const json = workflow.fromJSON(source).toJSON();
|
||||
|
||||
expect(json.nodeGroups![0].nodeIds).toEqual(['id-a']);
|
||||
});
|
||||
|
||||
it('leaves nodeGroups undefined when the source declares none', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: WF_ID,
|
||||
name: 'wf',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
};
|
||||
|
||||
const json = workflow.fromJSON(source).toJSON();
|
||||
|
||||
expect(json.nodeGroups).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('deterministic group id', () => {
|
||||
it('produces the same id for the same name and a different id for a different workflow', () => {
|
||||
expect(generateDeterministicGroupId(WF_ID, 'G')).toBe(
|
||||
generateDeterministicGroupId(WF_ID, 'G'),
|
||||
);
|
||||
expect(generateDeterministicGroupId(WF_ID, 'G')).not.toBe(
|
||||
generateDeterministicGroupId('other-wf', 'G'),
|
||||
);
|
||||
});
|
||||
|
||||
it('never collides with a node id derived from the same inputs', () => {
|
||||
const json = buildGroupedWorkflow().toJSON();
|
||||
const nodeIds = new Set(json.nodes.map((n) => n.id));
|
||||
expect(nodeIds.has(json.nodeGroups![0].id)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('existingGroupIdsByName (toJSON option)', () => {
|
||||
it('reuses an existing group id matched by name and derives one for a new group', () => {
|
||||
const start = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
const a = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'A' } });
|
||||
const b = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'B' } });
|
||||
|
||||
const json = workflow(WF_ID, 'wf')
|
||||
.add(start)
|
||||
.to(a)
|
||||
.to(b)
|
||||
.group('Existing', [a])
|
||||
.group('New', [b])
|
||||
// 'Existing' matches a (random) UI-assigned id; 'New' has no match.
|
||||
.toJSON({ existingGroupIdsByName: new Map([['Existing', 'ui-random-id']]) });
|
||||
|
||||
const idByName = new Map(json.nodes.map((n) => [n.name, n.id]));
|
||||
expect(json.nodeGroups).toEqual([
|
||||
{ id: 'ui-random-id', name: 'Existing', nodeIds: [idByName.get('A')] },
|
||||
{
|
||||
id: generateDeterministicGroupId(WF_ID, 'New'),
|
||||
name: 'New',
|
||||
nodeIds: [idByName.get('B')],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('lets a group carried from fromJSON keep its id over a name match', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: WF_ID,
|
||||
name: 'wf',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'source-id', name: 'G', nodeIds: ['id-a'] }],
|
||||
};
|
||||
|
||||
// The carried source id is authoritative and wins over the name-keyed map.
|
||||
const json = workflow
|
||||
.fromJSON(source)
|
||||
.toJSON({ existingGroupIdsByName: new Map([['G', 'other-id']]) });
|
||||
|
||||
expect(json.nodeGroups![0].id).toBe('source-id');
|
||||
});
|
||||
});
|
||||
|
||||
describe('code round-trip (generate → parse → build)', () => {
|
||||
it('emits a .group(...) call and preserves the group through a re-parse', () => {
|
||||
const builder = buildGroupedWorkflow();
|
||||
builder.regenerateNodeIds();
|
||||
const json1 = builder.toJSON();
|
||||
|
||||
const code = generateWorkflowCode(json1);
|
||||
expect(code).toContain(".group('Ingestion', [fetch_data, transform])");
|
||||
|
||||
const rebuilt = parseWorkflowCodeToBuilder(code);
|
||||
rebuilt.regenerateNodeIds();
|
||||
const json2 = rebuilt.toJSON();
|
||||
|
||||
expect(json2.nodeGroups).toHaveLength(1);
|
||||
expect(json2.nodeGroups![0].name).toBe('Ingestion');
|
||||
// Deterministic ids mean the group survives identically across the round-trip.
|
||||
expect(json2.nodeGroups![0].id).toBe(json1.nodeGroups![0].id);
|
||||
expect(json2.nodeGroups![0].nodeIds).toEqual(json1.nodeGroups![0].nodeIds);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -66,6 +66,26 @@ ${renderMethodLines()}
|
|||
|
||||
${SAFE_METHODS_SENTENCE}
|
||||
|
||||
## Node groups
|
||||
|
||||
A node group is a named, visual grouping of nodes (a frame on the canvas).
|
||||
Declare one with \`.group(name, members)\` on the workflow. Members are the node
|
||||
handles (the \`const\` from \`node(...)\`) — the same way connections reference nodes:
|
||||
|
||||
\`\`\`typescript
|
||||
const fetch = node({ /* ... name: 'Fetch data' */ });
|
||||
const transform = node({ /* ... name: 'Transform' */ });
|
||||
export default workflow('id', 'My workflow')
|
||||
.add(fetch)
|
||||
.to(transform)
|
||||
.group('Ingestion', [fetch, transform]);
|
||||
\`\`\`
|
||||
|
||||
When editing an existing workflow, **keep the \`.group(...)\` calls intact** unless
|
||||
the change is specifically about grouping. Group members must form a single
|
||||
connected section of the graph and cannot be trigger nodes; an invalid group is
|
||||
rejected on save.
|
||||
|
||||
## Forbidden constructs
|
||||
|
||||
${renderForbiddenLines()}
|
||||
|
|
|
|||
|
|
@ -4,9 +4,13 @@
|
|||
* Core types for building n8n workflows programmatically.
|
||||
*/
|
||||
|
||||
import type { IWorkflowGroup } from 'n8n-workflow';
|
||||
|
||||
import type { ValidationOptions, ValidationResult } from '../validation/index';
|
||||
import type { PluginRegistry } from '../workflow-builder/plugins/registry';
|
||||
|
||||
export type { IWorkflowGroup };
|
||||
|
||||
// =============================================================================
|
||||
// Data Types
|
||||
// =============================================================================
|
||||
|
|
@ -330,6 +334,13 @@ export interface WorkflowJSON {
|
|||
templateId?: string;
|
||||
instanceId?: string;
|
||||
};
|
||||
/**
|
||||
* Node groups, referencing their members by node ID. Internally the SDK carries
|
||||
* group members by node *handle* (the value from `node(...)`) and resolves them to
|
||||
* the emitted node IDs here at the JSON boundary, so groups survive
|
||||
* `regenerateNodeIds()` like connections do.
|
||||
*/
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
|
@ -996,11 +1007,23 @@ export interface GeneratePinDataOptions {
|
|||
export interface ToJSONOptions {
|
||||
/** Use Dagre-based layout matching the FE's tidy-up algorithm. Defaults to false (BFS layout). */
|
||||
tidyUp?: boolean;
|
||||
/**
|
||||
* Reuse existing group IDs (keyed by group name) instead of deriving deterministic ones.
|
||||
* Lets an edit of an existing workflow keep its (UI-assigned, random) group IDs so the diff
|
||||
* isn't skewed; groups without a match fall back to a deterministic ID.
|
||||
*/
|
||||
existingGroupIdsByName?: Map<string, string>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Workflow builder for constructing workflows with a fluent API
|
||||
*/
|
||||
/**
|
||||
* A reference to a node when defining a group: a node handle — the value
|
||||
* returned by `node(...)`. Trigger nodes cannot be grouped.
|
||||
*/
|
||||
export type GroupMember = NodeInstance<string, string, unknown>;
|
||||
|
||||
export interface WorkflowBuilder {
|
||||
readonly id: string;
|
||||
readonly name: string;
|
||||
|
|
@ -1073,6 +1096,25 @@ export interface WorkflowBuilder {
|
|||
|
||||
getNode(name: string): NodeInstance<string, string, unknown> | undefined;
|
||||
|
||||
/**
|
||||
* Define a node group — a named, semantic grouping of nodes shown as a frame on
|
||||
* the canvas. Members are referenced the same way connections reference nodes: by
|
||||
* node handle (the `const` from `node(...)`). Trigger nodes cannot be grouped.
|
||||
* Members resolve to the emitted node IDs in `toJSON()`, so groups survive
|
||||
* `regenerateNodeIds()` like connections do. Chainable.
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* const fetch = node({ ... });
|
||||
* const transform = node({ ... });
|
||||
* workflow('id', 'Name')
|
||||
* .add(fetch)
|
||||
* .to(transform)
|
||||
* .group('Data ingestion', [fetch, transform]);
|
||||
* ```
|
||||
*/
|
||||
group(name: string, members: GroupMember[]): WorkflowBuilder;
|
||||
|
||||
/**
|
||||
* Validate the workflow graph structure.
|
||||
* Returns errors and warnings without converting to JSON.
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
tool,
|
||||
outputParser,
|
||||
} from './workflow-builder/node-builders/subnode-builders';
|
||||
import { generateDeterministicGroupId } from './workflow-builder/string-utils';
|
||||
|
||||
describe('Workflow Builder', () => {
|
||||
describe('workflow()', () => {
|
||||
|
|
@ -951,6 +952,109 @@ describe('Workflow Builder', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('node groups', () => {
|
||||
it('emits a group authored via .group() with members resolved to node ids', () => {
|
||||
const t = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
const fetch = node({
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
version: 4.2,
|
||||
config: { name: 'Fetch' },
|
||||
});
|
||||
const transform = node({
|
||||
type: 'n8n-nodes-base.set',
|
||||
version: 3,
|
||||
config: { name: 'Transform' },
|
||||
});
|
||||
|
||||
const json = workflow('wf-1', 'Test')
|
||||
.add(t)
|
||||
.to(fetch)
|
||||
.to(transform)
|
||||
.group('Ingestion', [fetch, transform])
|
||||
.toJSON();
|
||||
|
||||
const idByName = new Map(json.nodes.map((n) => [n.name, n.id]));
|
||||
expect(json.nodeGroups).toEqual([
|
||||
{
|
||||
id: generateDeterministicGroupId('wf-1', 'Ingestion'),
|
||||
name: 'Ingestion',
|
||||
nodeIds: [idByName.get('Fetch'), idByName.get('Transform')],
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('omits nodeGroups when no group was declared', () => {
|
||||
const t = trigger({
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
version: 1,
|
||||
config: { name: 'Start' },
|
||||
});
|
||||
|
||||
expect(workflow('wf-1', 'Test').add(t).toJSON().nodeGroups).toBeUndefined();
|
||||
});
|
||||
|
||||
it('preserves a group id imported via fromJSON across a round-trip', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: 'wf-1',
|
||||
name: 'Test',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'id-b',
|
||||
name: 'B',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [10, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: { A: { main: [[{ node: 'B', type: 'main', index: 0 }]] } },
|
||||
nodeGroups: [{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a', 'id-b'] }],
|
||||
};
|
||||
|
||||
const json = workflow.fromJSON(source).toJSON();
|
||||
|
||||
expect(json.nodeGroups).toEqual([
|
||||
{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a', 'id-b'] },
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps an imported group after the builder is modified', () => {
|
||||
const source: WorkflowJSON = {
|
||||
id: 'wf-1',
|
||||
name: 'Test',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a'] }],
|
||||
};
|
||||
|
||||
const added = node({ type: 'n8n-nodes-base.set', version: 3, config: { name: 'B' } });
|
||||
const json = workflow.fromJSON(source).to(added).toJSON();
|
||||
|
||||
expect(json.nodeGroups).toEqual([{ id: 'ui-random-id', name: 'G', nodeIds: ['id-a'] }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('.toString()', () => {
|
||||
it('should serialize to JSON string', () => {
|
||||
const wf = workflow('test-id', 'Test Workflow');
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import type {
|
|||
NodeInstance,
|
||||
ConnectionTarget,
|
||||
GraphNode,
|
||||
GroupMember,
|
||||
IDataObject,
|
||||
NodeChain,
|
||||
GeneratePinDataOptions,
|
||||
|
|
@ -56,6 +57,8 @@ class WorkflowBuilderImpl implements WorkflowBuilder {
|
|||
private _branchDepth = 0;
|
||||
private _dispatchedComposites = new WeakSet<object>();
|
||||
private static readonly MAX_BRANCH_DEPTH = 500;
|
||||
/** Node groups, carried by member node handle and resolved to IDs in toJSON(). */
|
||||
private _nodeGroups: Array<{ id?: string; name: string; members: GroupMember[] }>;
|
||||
|
||||
constructor(
|
||||
id: string,
|
||||
|
|
@ -66,6 +69,7 @@ class WorkflowBuilderImpl implements WorkflowBuilder {
|
|||
pinData?: Record<string, IDataObject[]>,
|
||||
meta?: { templateId?: string; instanceId?: string; [key: string]: unknown },
|
||||
registry?: PluginRegistry,
|
||||
nodeGroups?: Array<{ id?: string; name: string; members: GroupMember[] }>,
|
||||
) {
|
||||
this.id = id;
|
||||
this.name = name;
|
||||
|
|
@ -76,6 +80,9 @@ class WorkflowBuilderImpl implements WorkflowBuilder {
|
|||
this._pinData = pinData;
|
||||
this._meta = meta;
|
||||
this._registry = registry;
|
||||
this._nodeGroups = nodeGroups
|
||||
? nodeGroups.map((g) => ({ id: g.id, name: g.name, members: [...g.members] }))
|
||||
: [];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -491,6 +498,31 @@ class WorkflowBuilderImpl implements WorkflowBuilder {
|
|||
return this;
|
||||
}
|
||||
|
||||
group(name: string, members: GroupMember[]): WorkflowBuilder {
|
||||
this._nodeGroups.push({ name, members: [...members] });
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve each group's members to the IDs the emitted nodes will carry. Each member
|
||||
* handle resolves to its current map key (stable across regenerateNodeIds() via
|
||||
* _staleIdToKeyMap, exactly as connection targets do); the live instance under that
|
||||
* key holds the ID the serializer emits. Unresolvable members are dropped.
|
||||
*/
|
||||
private resolveNodeGroups(): Array<{ id?: string; name: string; memberIds: string[] }> {
|
||||
return this._nodeGroups.map((group) => {
|
||||
const memberIds: string[] = [];
|
||||
for (const member of group.members) {
|
||||
const key = this.resolveTargetNodeName(member, this._staleIdToKeyMap);
|
||||
const id = key ? this._nodes.get(key)?.instance.id : undefined;
|
||||
if (id && !memberIds.includes(id)) {
|
||||
memberIds.push(id);
|
||||
}
|
||||
}
|
||||
return { id: group.id, name: group.name, memberIds };
|
||||
});
|
||||
}
|
||||
|
||||
getNode(name: string): NodeInstance<string, string, unknown> | undefined {
|
||||
// First try direct lookup (for backward compatibility and nodes added via add/then)
|
||||
const directLookup = this._nodes.get(name);
|
||||
|
|
@ -525,6 +557,8 @@ class WorkflowBuilderImpl implements WorkflowBuilder {
|
|||
meta: this._meta,
|
||||
tidyUp: options?.tidyUp ?? false,
|
||||
resolveTargetNodeName: (target: unknown) => this.resolveTargetNodeName(target),
|
||||
nodeGroups: this._nodeGroups.length > 0 ? this.resolveNodeGroups() : undefined,
|
||||
existingGroupIdsByName: options?.existingGroupIdsByName,
|
||||
};
|
||||
|
||||
return jsonSerializer.serialize(ctx);
|
||||
|
|
@ -1325,6 +1359,8 @@ function fromJSON(json: WorkflowJSON): WorkflowBuilder {
|
|||
parsed.lastNode,
|
||||
parsed.pinData,
|
||||
parsed.meta,
|
||||
undefined,
|
||||
parsed.nodeGroups,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { jsonSerializer } from './json-serializer';
|
||||
import type { GraphNode } from '../../../types/base';
|
||||
import { trigger } from '../../node-builders/node-builder';
|
||||
import { node, trigger } from '../../node-builders/node-builder';
|
||||
import { generateDeterministicGroupId } from '../../string-utils';
|
||||
import type { SerializerContext } from '../types';
|
||||
|
||||
// Helper to create a mock serializer context
|
||||
|
|
@ -111,5 +112,66 @@ describe('jsonSerializer', () => {
|
|||
|
||||
expect(result.nodes[0]?.parameters).toEqual({});
|
||||
});
|
||||
|
||||
describe('node groups', () => {
|
||||
it('omits nodeGroups when the context has none', () => {
|
||||
const result = jsonSerializer.serialize(createMockSerializerContext());
|
||||
|
||||
expect(result.nodeGroups).toBeUndefined();
|
||||
});
|
||||
|
||||
it('derives a deterministic id when a group carries none and no name matches', () => {
|
||||
const ctx = createMockSerializerContext({
|
||||
workflowId: 'wf-1',
|
||||
nodeGroups: [{ name: 'G', memberIds: [] }],
|
||||
});
|
||||
|
||||
const result = jsonSerializer.serialize(ctx);
|
||||
|
||||
expect(result.nodeGroups).toEqual([
|
||||
{ id: generateDeterministicGroupId('wf-1', 'G'), name: 'G', nodeIds: [] },
|
||||
]);
|
||||
});
|
||||
|
||||
it('reuses an existing id matched by name when the group carries none', () => {
|
||||
const ctx = createMockSerializerContext({
|
||||
nodeGroups: [{ name: 'G', memberIds: [] }],
|
||||
existingGroupIdsByName: new Map([['G', 'ui-id']]),
|
||||
});
|
||||
|
||||
const result = jsonSerializer.serialize(ctx);
|
||||
|
||||
expect(result.nodeGroups![0].id).toBe('ui-id');
|
||||
});
|
||||
|
||||
it("prefers the group's own id over a name match", () => {
|
||||
const ctx = createMockSerializerContext({
|
||||
nodeGroups: [{ id: 'carried-id', name: 'G', memberIds: [] }],
|
||||
existingGroupIdsByName: new Map([['G', 'ui-id']]),
|
||||
});
|
||||
|
||||
const result = jsonSerializer.serialize(ctx);
|
||||
|
||||
expect(result.nodeGroups![0].id).toBe('carried-id');
|
||||
});
|
||||
|
||||
it('drops member ids that are not present in the emitted nodes', () => {
|
||||
const fetch = node({
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
version: 4.2,
|
||||
config: { name: 'Fetch' },
|
||||
});
|
||||
const ctx = createMockSerializerContext({
|
||||
nodes: new Map<string, GraphNode>([
|
||||
['Fetch', { instance: fetch, connections: new Map() }],
|
||||
]),
|
||||
nodeGroups: [{ name: 'G', memberIds: [fetch.id, 'ghost-id'] }],
|
||||
});
|
||||
|
||||
const result = jsonSerializer.serialize(ctx);
|
||||
|
||||
expect(result.nodeGroups![0].nodeIds).toEqual([fetch.id]);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import {
|
|||
normalizeResourceLocators,
|
||||
escapeNewlinesInExpressionStrings,
|
||||
parseVersion,
|
||||
generateDeterministicGroupId,
|
||||
} from '../../string-utils';
|
||||
import type { SerializerPlugin, SerializerContext } from '../types';
|
||||
|
||||
|
|
@ -272,6 +273,23 @@ export const jsonSerializer: SerializerPlugin<WorkflowJSON> = {
|
|||
json.meta = ctx.meta;
|
||||
}
|
||||
|
||||
// Members already carry the emitted nodes' IDs; filter out any that aren't present
|
||||
// in the output (defensive — should never happen). Group ID precedence: own ID
|
||||
// (carried through fromJSON for a lossless round-trip), then a name match (preserves
|
||||
// UI-assigned IDs across edits), else a deterministic ID from the name.
|
||||
if (ctx.nodeGroups && ctx.nodeGroups.length > 0) {
|
||||
const emittedIds = new Set(nodes.map((node) => node.id));
|
||||
|
||||
json.nodeGroups = ctx.nodeGroups.map((group) => ({
|
||||
id:
|
||||
group.id ??
|
||||
ctx.existingGroupIdsByName?.get(group.name) ??
|
||||
generateDeterministicGroupId(ctx.workflowId, group.name),
|
||||
name: group.name,
|
||||
nodeIds: group.memberIds.filter((id) => emittedIds.has(id)),
|
||||
}));
|
||||
}
|
||||
|
||||
return json;
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -351,6 +351,20 @@ export interface SerializerContext extends PluginContext {
|
|||
|
||||
/** Whether to use Dagre-based layout for node positioning */
|
||||
readonly tidyUp?: boolean;
|
||||
|
||||
/**
|
||||
* Node groups carried by member node *ID* — already resolved to the IDs the emitted
|
||||
* nodes carry. `id`, when present, is the source group ID (from fromJSON); the serializer
|
||||
* reuses it, otherwise assigns one.
|
||||
*/
|
||||
readonly nodeGroups?: ReadonlyArray<{ id?: string; name: string; memberIds: string[] }>;
|
||||
|
||||
/**
|
||||
* Existing group IDs keyed by group name. When a group name matches, the serializer
|
||||
* reuses that ID instead of deriving a deterministic one (preserves UI-assigned IDs
|
||||
* across edits). Groups without a match fall back to the deterministic ID.
|
||||
*/
|
||||
readonly existingGroupIdsByName?: ReadonlyMap<string, string>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import {
|
|||
escapeNewlinesInStringLiterals,
|
||||
escapeNewlinesInExpressionStrings,
|
||||
generateDeterministicNodeId,
|
||||
generateDeterministicGroupId,
|
||||
} from './string-utils';
|
||||
|
||||
describe('workflow-builder/string-utils', () => {
|
||||
|
|
@ -276,4 +277,33 @@ describe('workflow-builder/string-utils', () => {
|
|||
expect(id1).not.toBe(id2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generateDeterministicGroupId', () => {
|
||||
it('generates UUID format', () => {
|
||||
const id = generateDeterministicGroupId('wf1', 'My Group');
|
||||
expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/);
|
||||
});
|
||||
|
||||
it('is deterministic for same inputs', () => {
|
||||
expect(generateDeterministicGroupId('wf1', 'G')).toBe(
|
||||
generateDeterministicGroupId('wf1', 'G'),
|
||||
);
|
||||
});
|
||||
|
||||
it('produces different IDs for a different workflow or group name', () => {
|
||||
expect(generateDeterministicGroupId('wf1', 'G')).not.toBe(
|
||||
generateDeterministicGroupId('wf2', 'G'),
|
||||
);
|
||||
expect(generateDeterministicGroupId('wf1', 'A')).not.toBe(
|
||||
generateDeterministicGroupId('wf1', 'B'),
|
||||
);
|
||||
});
|
||||
|
||||
it('produces a different id than a node id for the same workflow and name', () => {
|
||||
// The ':group:' seed segment keeps group ids disjoint from node ids.
|
||||
expect(generateDeterministicGroupId('wf1', 'Ingestion')).not.toBe(
|
||||
generateDeterministicNodeId('wf1', 'n8n-nodes-base.set', 'Ingestion'),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -319,19 +319,11 @@ export function escapeNewlinesInExpressionStrings(value: unknown): unknown {
|
|||
}
|
||||
|
||||
/**
|
||||
* Generate a deterministic UUID based on workflow ID, node type, and node name.
|
||||
* This ensures that the same workflow structure always produces the same node IDs,
|
||||
* which is critical for the AI workflow builder where code may be re-parsed multiple times.
|
||||
* Hash a seed string and format the digest as a deterministic UUID v4 structure.
|
||||
* Shared by the node-id and group-id generators so they format identically.
|
||||
*/
|
||||
export function generateDeterministicNodeId(
|
||||
workflowId: string,
|
||||
nodeType: string,
|
||||
nodeName: string,
|
||||
): string {
|
||||
const hash = createHash('sha256')
|
||||
.update(`${workflowId}:${nodeType}:${nodeName}`)
|
||||
.digest('hex')
|
||||
.slice(0, 32);
|
||||
function deterministicUuidFromSeed(seed: string): string {
|
||||
const hash = createHash('sha256').update(seed).digest('hex').slice(0, 32);
|
||||
|
||||
// Format as valid UUID v4 structure
|
||||
return [
|
||||
|
|
@ -342,3 +334,26 @@ export function generateDeterministicNodeId(
|
|||
hash.slice(20, 32),
|
||||
].join('-');
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a deterministic UUID based on workflow ID, node type, and node name.
|
||||
* This ensures that the same workflow structure always produces the same node IDs,
|
||||
* which is critical for the AI workflow builder where code may be re-parsed multiple times.
|
||||
*/
|
||||
export function generateDeterministicNodeId(
|
||||
workflowId: string,
|
||||
nodeType: string,
|
||||
nodeName: string,
|
||||
): string {
|
||||
return deterministicUuidFromSeed(`${workflowId}:${nodeType}:${nodeName}`);
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a deterministic UUID for a node group based on workflow ID and group name.
|
||||
* Group names are unique within a workflow, so the same name always yields the same ID
|
||||
* on every round-trip — no persisted-workflow lookup needed. The `:group:` segment keeps
|
||||
* group IDs from ever colliding with node IDs from {@link generateDeterministicNodeId}.
|
||||
*/
|
||||
export function generateDeterministicGroupId(workflowId: string, groupName: string): string {
|
||||
return deterministicUuidFromSeed(`${workflowId}:group:${groupName}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -388,4 +388,84 @@ describe('parseWorkflowJSON', () => {
|
|||
expect(json.nodes[0].typeVersion).toBe(originalTypeVersion);
|
||||
expect(JSON.stringify(json.connections)).toBe(originalConnections);
|
||||
});
|
||||
|
||||
it('rebuilds node groups, mapping member ids to their node instances', () => {
|
||||
const json: WorkflowJSON = {
|
||||
id: 'wf',
|
||||
name: 'Grouped',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: 'id-b',
|
||||
name: 'B',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [10, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
// The incoming group id is irrelevant here — parse carries members as instances.
|
||||
nodeGroups: [{ id: 'group-1', name: 'G', nodeIds: ['id-a', 'id-b'] }],
|
||||
};
|
||||
|
||||
const result = parseWorkflowJSON(json);
|
||||
|
||||
expect(result.nodeGroups).toHaveLength(1);
|
||||
expect(result.nodeGroups![0].name).toBe('G');
|
||||
expect(result.nodeGroups![0].members.map((m) => m.id)).toEqual(['id-a', 'id-b']);
|
||||
expect(result.nodeGroups![0].members.map((m) => m.name)).toEqual(['A', 'B']);
|
||||
});
|
||||
|
||||
it('drops group members whose ids do not match a node', () => {
|
||||
const json: WorkflowJSON = {
|
||||
id: 'wf',
|
||||
name: 'Grouped',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'group-1', name: 'G', nodeIds: ['id-a', 'ghost'] }],
|
||||
};
|
||||
|
||||
const result = parseWorkflowJSON(json);
|
||||
|
||||
expect(result.nodeGroups![0].members.map((m) => m.id)).toEqual(['id-a']);
|
||||
});
|
||||
|
||||
it('returns undefined nodeGroups when none are declared', () => {
|
||||
const json: WorkflowJSON = {
|
||||
id: 'wf',
|
||||
name: 'No groups',
|
||||
nodes: [
|
||||
{
|
||||
id: 'id-a',
|
||||
name: 'A',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
};
|
||||
|
||||
const result = parseWorkflowJSON(json);
|
||||
|
||||
expect(result.nodeGroups).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -30,6 +30,13 @@ export interface ParsedWorkflow {
|
|||
readonly lastNode: string | null;
|
||||
readonly pinData?: Record<string, IDataObject[]>;
|
||||
readonly meta?: { templateId?: string; instanceId?: string; [key: string]: unknown };
|
||||
/** Node groups reconstructed by mapping the JSON's member IDs back to node handles. */
|
||||
readonly nodeGroups?: Array<{
|
||||
/** Source group ID from the JSON. */
|
||||
id?: string;
|
||||
name: string;
|
||||
members: Array<NodeInstance<string, string, unknown>>;
|
||||
}>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -40,6 +47,9 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow {
|
|||
const nodes = new Map<string, GraphNode>();
|
||||
// Map from connection name (how nodes reference each other) to map key
|
||||
const nameToKey = new Map<string, string>();
|
||||
// Map from n8n node ID to the created node handle, used to rebuild groups (which
|
||||
// reference members by ID) as node refs — the same shape `.group()` authoring uses.
|
||||
const idToInstance = new Map<string, NodeInstance<string, string, unknown>>();
|
||||
|
||||
// Create node instances from JSON (shallow-clone each node to avoid mutating the input)
|
||||
let unnamedCounter = 0;
|
||||
|
|
@ -116,6 +126,10 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow {
|
|||
instance,
|
||||
connections: connectionsMap,
|
||||
});
|
||||
|
||||
// Groups reference members by ID; record ID → handle so we can carry groups as
|
||||
// node refs (resolved back to IDs in toJSON, exactly like authored groups).
|
||||
if (n8nNode.id) idToInstance.set(n8nNode.id, instance);
|
||||
}
|
||||
|
||||
// Rebuild connections (deep-clone to avoid mutating the input)
|
||||
|
|
@ -161,6 +175,19 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow {
|
|||
lastNode = name;
|
||||
}
|
||||
|
||||
// Rebuild groups by mapping each member ID back to its node handle (unresolvable IDs
|
||||
// are dropped). The group's `id` is carried through so a round-trip preserves it.
|
||||
const nodeGroups = json.nodeGroups?.length
|
||||
? json.nodeGroups.map((group) => ({
|
||||
id: group.id,
|
||||
name: group.name,
|
||||
members: group.nodeIds.flatMap((id) => {
|
||||
const instance = idToInstance.get(id);
|
||||
return instance !== undefined ? [instance] : [];
|
||||
}),
|
||||
}))
|
||||
: undefined;
|
||||
|
||||
return {
|
||||
id: json.id ?? '',
|
||||
name: json.name,
|
||||
|
|
@ -169,5 +196,6 @@ export function parseWorkflowJSON(json: WorkflowJSON): ParsedWorkflow {
|
|||
lastNode,
|
||||
pinData: json.pinData,
|
||||
meta: json.meta,
|
||||
nodeGroups,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,69 @@
|
|||
{
|
||||
"id": "wf-committed-10",
|
||||
"name": "Node groups - linear chain with a grouped section",
|
||||
"nodes": [
|
||||
{
|
||||
"parameters": {},
|
||||
"type": "n8n-nodes-base.manualTrigger",
|
||||
"typeVersion": 1,
|
||||
"position": [-200, 0],
|
||||
"id": "trigger-10-0001",
|
||||
"name": "When clicking 'Execute workflow'"
|
||||
},
|
||||
{
|
||||
"parameters": {
|
||||
"assignments": {
|
||||
"assignments": [
|
||||
{
|
||||
"id": "assign-10-0001",
|
||||
"name": "label",
|
||||
"value": "raw",
|
||||
"type": "string"
|
||||
}
|
||||
]
|
||||
},
|
||||
"options": {}
|
||||
},
|
||||
"type": "n8n-nodes-base.set",
|
||||
"typeVersion": 3.4,
|
||||
"position": [0, 0],
|
||||
"id": "set-10-0001",
|
||||
"name": "Normalize"
|
||||
},
|
||||
{
|
||||
"parameters": {
|
||||
"assignments": {
|
||||
"assignments": [
|
||||
{
|
||||
"id": "assign-10-0002",
|
||||
"name": "enriched",
|
||||
"value": "={{ $json.label }}-enriched",
|
||||
"type": "string"
|
||||
}
|
||||
]
|
||||
},
|
||||
"options": {}
|
||||
},
|
||||
"type": "n8n-nodes-base.set",
|
||||
"typeVersion": 3.4,
|
||||
"position": [200, 0],
|
||||
"id": "set-10-0002",
|
||||
"name": "Enrich"
|
||||
}
|
||||
],
|
||||
"connections": {
|
||||
"When clicking 'Execute workflow'": {
|
||||
"main": [[{ "node": "Normalize", "type": "main", "index": 0 }]]
|
||||
},
|
||||
"Normalize": {
|
||||
"main": [[{ "node": "Enrich", "type": "main", "index": 0 }]]
|
||||
}
|
||||
},
|
||||
"nodeGroups": [
|
||||
{
|
||||
"id": "grp-10-0001",
|
||||
"name": "Transform",
|
||||
"nodeIds": ["set-10-0001", "set-10-0002"]
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
@ -104,6 +104,11 @@
|
|||
"id": 9,
|
||||
"name": "If/Switch branch wired directly to multi-input merge",
|
||||
"success": true
|
||||
},
|
||||
{
|
||||
"id": 10,
|
||||
"name": "Node groups - linear chain with a grouped section",
|
||||
"success": true
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -453,66 +453,146 @@ describe('validateWorkflowNodeGroups', () => {
|
|||
|
||||
it('should pass when nodeGroups is undefined', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: undefined }),
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: undefined }, null),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should pass when nodeGroups is empty', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: [] }),
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: [] }, null),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should pass when all nodeIds reference existing nodes', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({
|
||||
nodes: [makeNode('n1'), makeNode('n2')],
|
||||
nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: ['n1', 'n2'] }],
|
||||
}),
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1'), makeNode('n2')],
|
||||
nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: ['n1', 'n2'] }],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should throw when a nodeId does not reference an existing node', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({
|
||||
nodes: [makeNode('n1')],
|
||||
nodeGroups: [{ id: 'g1', name: 'My Group', nodeIds: ['n1', 'n999'] }],
|
||||
}),
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1')],
|
||||
nodeGroups: [{ id: 'g1', name: 'My Group', nodeIds: ['n1', 'n999'] }],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).toThrow('Group "My Group" references node ID "n999" that does not exist in the workflow.');
|
||||
});
|
||||
|
||||
it('should throw for the first invalid nodeId found', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({
|
||||
nodes: [],
|
||||
nodeGroups: [{ id: 'g1', name: 'Empty Group', nodeIds: ['bad1', 'bad2'] }],
|
||||
}),
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [],
|
||||
nodeGroups: [{ id: 'g1', name: 'Empty Group', nodeIds: ['bad1', 'bad2'] }],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).toThrow('Group "Empty Group" references node ID "bad1"');
|
||||
});
|
||||
|
||||
it('should throw when a node belongs to multiple groups', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({
|
||||
nodes: [makeNode('n1'), makeNode('n2')],
|
||||
nodeGroups: [
|
||||
{ id: 'g1', name: 'Group A', nodeIds: ['n1'] },
|
||||
{ id: 'g2', name: 'Group B', nodeIds: ['n1', 'n2'] },
|
||||
],
|
||||
}),
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1'), makeNode('n2')],
|
||||
nodeGroups: [
|
||||
{ id: 'g1', name: 'Group A', nodeIds: ['n1'] },
|
||||
{ id: 'g2', name: 'Group B', nodeIds: ['n1', 'n2'] },
|
||||
],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).toThrow('Node "n1" belongs to multiple groups: "Group A" and "Group B".');
|
||||
});
|
||||
|
||||
it('should throw when group names are not unique', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({
|
||||
nodes: [makeNode('n1')],
|
||||
nodeGroups: [
|
||||
{ id: 'g1', name: 'Duplicate', nodeIds: ['n1'] },
|
||||
{ id: 'g2', name: 'Duplicate', nodeIds: [] },
|
||||
],
|
||||
}),
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1')],
|
||||
nodeGroups: [
|
||||
{ id: 'g1', name: 'Duplicate', nodeIds: ['n1'] },
|
||||
{ id: 'g2', name: 'Duplicate', nodeIds: [] },
|
||||
],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).toThrow('Duplicate node group name "Duplicate".');
|
||||
});
|
||||
|
||||
it('should throw when group ids are not unique', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1'), makeNode('n2')],
|
||||
nodeGroups: [
|
||||
{ id: 'dup', name: 'Group A', nodeIds: ['n1'] },
|
||||
{ id: 'dup', name: 'Group B', nodeIds: ['n2'] },
|
||||
],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).toThrow('Duplicate node group ID "dup".');
|
||||
});
|
||||
|
||||
describe('full validation', () => {
|
||||
const triggerType = { group: ['trigger'] } as never;
|
||||
const regularType = { group: ['transform'] } as never;
|
||||
// Two nodes connected n1 → n2 form a groupable chain.
|
||||
const connectedNodes = [makeNode('n1'), makeNode('n2')];
|
||||
const connections = {
|
||||
'Node n1': { main: [[{ node: 'Node n2', type: 'main', index: 0 }]] },
|
||||
} as never;
|
||||
|
||||
it('passes for a valid connected group', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: connectedNodes,
|
||||
connections,
|
||||
nodeGroups: [{ id: 'g1', name: 'Chain', nodeIds: ['n1', 'n2'] }],
|
||||
},
|
||||
() => regularType,
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('rejects a group that contains a trigger node', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1')],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Has trigger', nodeIds: ['n1'] }],
|
||||
},
|
||||
() => triggerType,
|
||||
),
|
||||
).toThrow('Node group "Has trigger" (g1) cannot contain trigger nodes');
|
||||
});
|
||||
|
||||
it('does not run full checks when getNodeType is null (basic-only)', () => {
|
||||
// Same trigger-in-group that fails under full validation passes under basic-only.
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: [makeNode('n1')],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Has trigger', nodeIds: ['n1'] }],
|
||||
},
|
||||
null,
|
||||
),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('validatePinDataSize', () => {
|
||||
|
|
|
|||
|
|
@ -1934,6 +1934,47 @@ describe('createWorkflowAdapter', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('clears existing node groups when the SDK workflow declares none (update is authoritative)', async () => {
|
||||
// Regression: the SDK omits `nodeGroups` when no `.group(...)` is declared. The
|
||||
// update path must treat that as "no groups" and send [] so a removed group is
|
||||
// dropped — not silently preserved and then rejected by group validation.
|
||||
const { adapter, mockWorkflowService } = createWorkflowAdapterForTests();
|
||||
|
||||
await adapter.updateFromWorkflowJSON('wf-new', minimalWorkflowJSON);
|
||||
|
||||
expect(mockWorkflowService.update).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ nodeGroups: [] }),
|
||||
expect.anything(),
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('writes the node groups the SDK workflow declares', async () => {
|
||||
const { adapter, mockWorkflowService } = createWorkflowAdapterForTests();
|
||||
const nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['node-1'] }];
|
||||
const workflow = {
|
||||
name: 'Test',
|
||||
nodes: [
|
||||
{
|
||||
id: 'node-1',
|
||||
name: 'Set',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 3,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups,
|
||||
} as unknown as WorkflowJSON;
|
||||
|
||||
await adapter.updateFromWorkflowJSON('wf-new', workflow);
|
||||
|
||||
const updateData = mockWorkflowService.update.mock.calls[0]?.[1] as { nodeGroups: unknown };
|
||||
expect(updateData.nodeGroups).toEqual(nodeGroups);
|
||||
});
|
||||
|
||||
it('strips id-less credential references before creating a workflow', async () => {
|
||||
const { adapter, mockWorkflowService } = createWorkflowAdapterForTests();
|
||||
const workflow = {
|
||||
|
|
|
|||
|
|
@ -594,6 +594,7 @@ export class InstanceAiAdapterService {
|
|||
connections: json.connections as unknown as IConnections,
|
||||
settings,
|
||||
pinData: sdkPinDataToRuntime(json.pinData),
|
||||
nodeGroups: sdkNodeGroupsToRuntime(json.nodeGroups),
|
||||
} as Partial<WorkflowEntity>);
|
||||
|
||||
let updated: WorkflowEntity;
|
||||
|
|
@ -682,6 +683,7 @@ export class InstanceAiAdapterService {
|
|||
connections: json.connections as unknown as IConnections,
|
||||
settings,
|
||||
pinData: sdkPinDataToRuntime(json.pinData),
|
||||
nodeGroups: sdkNodeGroupsToRuntime(json.nodeGroups),
|
||||
} as Partial<WorkflowEntity>);
|
||||
|
||||
let updated: WorkflowEntity;
|
||||
|
|
@ -781,6 +783,9 @@ export class InstanceAiAdapterService {
|
|||
const updateData = workflowRepository.create({
|
||||
nodes: version.nodes,
|
||||
connections: version.connections,
|
||||
// Restore the group state from the same snapshot so groups stay consistent
|
||||
// with the restored graph (history rows always carry nodeGroups).
|
||||
nodeGroups: version.nodeGroups,
|
||||
} as Partial<WorkflowEntity>);
|
||||
|
||||
await workflowService.update(user, updateData, workflowId, {
|
||||
|
|
@ -3164,6 +3169,16 @@ function sdkPinDataToRuntime(pinData: Record<string, unknown[]> | undefined): IP
|
|||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Groups are authoritative on save: persist the emitted groups, or [] to clear when the
|
||||
* agent removed every `.group(...)`. `undefined` would leave the NOT-NULL column stale.
|
||||
*/
|
||||
function sdkNodeGroupsToRuntime(
|
||||
nodeGroups: WorkflowJSON['nodeGroups'],
|
||||
): NonNullable<WorkflowJSON['nodeGroups']> {
|
||||
return nodeGroups ?? [];
|
||||
}
|
||||
|
||||
function hasCredentialId(value: unknown): boolean {
|
||||
if (typeof value !== 'object' || value === null) return false;
|
||||
const id = Reflect.get(value, 'id');
|
||||
|
|
@ -3222,6 +3237,7 @@ function toWorkflowJSON(
|
|||
})),
|
||||
connections: workflow.connections as WorkflowJSON['connections'],
|
||||
settings: workflow.settings as WorkflowJSON['settings'],
|
||||
...(workflow.nodeGroups ? { nodeGroups: workflow.nodeGroups } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import type { IConnections, INode } from 'n8n-workflow';
|
|||
import {
|
||||
applyOperations,
|
||||
partialUpdateOperationSchema,
|
||||
toWorkflowSlice,
|
||||
type PartialUpdateOperation,
|
||||
} from '../tools/workflow-builder/workflow-operations';
|
||||
|
||||
|
|
@ -870,4 +871,64 @@ describe('applyOperations', () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('setNodeGroups', () => {
|
||||
test('sets node groups on the workflow', () => {
|
||||
const result = applyOperations(baseWorkflow(), [
|
||||
{ type: 'setNodeGroups', nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }] },
|
||||
]);
|
||||
expect(result.success).toBe(true);
|
||||
if (!result.success) return;
|
||||
expect(result.workflow.nodeGroups).toEqual([
|
||||
{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] },
|
||||
]);
|
||||
});
|
||||
|
||||
test('generates an id when omitted', () => {
|
||||
const result = applyOperations(baseWorkflow(), [
|
||||
{ type: 'setNodeGroups', nodeGroups: [{ name: 'Group', nodeIds: ['a'] }] },
|
||||
]);
|
||||
expect(result.success).toBe(true);
|
||||
if (!result.success) return;
|
||||
expect(result.workflow.nodeGroups).toHaveLength(1);
|
||||
expect(result.workflow.nodeGroups![0].id).toEqual(expect.any(String));
|
||||
expect(result.workflow.nodeGroups![0].id.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('clears all groups with an empty array', () => {
|
||||
const wf = { ...baseWorkflow(), nodeGroups: [{ id: 'g1', name: 'Old', nodeIds: ['a'] }] };
|
||||
const result = applyOperations(wf, [{ type: 'setNodeGroups', nodeGroups: [] }]);
|
||||
expect(result.success).toBe(true);
|
||||
if (!result.success) return;
|
||||
expect(result.workflow.nodeGroups).toEqual([]);
|
||||
});
|
||||
|
||||
test('does not mutate the input workflow', () => {
|
||||
const wf = baseWorkflow();
|
||||
applyOperations(wf, [
|
||||
{ type: 'setNodeGroups', nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a'] }] },
|
||||
]);
|
||||
expect((wf as { nodeGroups?: unknown }).nodeGroups).toBeUndefined();
|
||||
});
|
||||
|
||||
test('schema parses a valid setNodeGroups op', () => {
|
||||
const parsed = partialUpdateOperationSchema.safeParse({
|
||||
type: 'setNodeGroups',
|
||||
nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a', 'b'] }],
|
||||
});
|
||||
expect(parsed.success).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toWorkflowSlice', () => {
|
||||
test('carries nodeGroups through from the workflow entity', () => {
|
||||
const slice = toWorkflowSlice({
|
||||
name: 'wf',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: ['a'] }],
|
||||
} as never);
|
||||
expect(slice.nodeGroups).toEqual([{ id: 'g1', name: 'Group', nodeIds: ['a'] }]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -134,6 +134,7 @@ export async function getWorkflowDetails(
|
|||
({ credentials: _credentials, ...node }) => node,
|
||||
),
|
||||
connections: workflow.activeVersion.connections ?? {},
|
||||
nodeGroups: workflow.activeVersion.nodeGroups ?? [],
|
||||
}
|
||||
: null;
|
||||
|
||||
|
|
@ -163,6 +164,7 @@ export async function getWorkflowDetails(
|
|||
settings: workflow.settings ?? null,
|
||||
connections,
|
||||
nodes: nodes.map(({ credentials: _credentials, ...node }) => node),
|
||||
nodeGroups: workflow.nodeGroups ?? [],
|
||||
activeVersion,
|
||||
tags: toTagSummary(workflow.tags),
|
||||
meta: workflow.meta ?? null,
|
||||
|
|
|
|||
|
|
@ -70,6 +70,14 @@ export const successMessageOutputSchema = {
|
|||
message: z.string().describe('Description of the result'),
|
||||
} satisfies z.ZodRawShape;
|
||||
|
||||
const nodeGroupSchema = z
|
||||
.object({
|
||||
id: z.string(),
|
||||
name: z.string(),
|
||||
nodeIds: z.array(z.string()),
|
||||
})
|
||||
.describe('A named visual grouping of nodes');
|
||||
|
||||
export const workflowDetailsOutputSchema = z.object({
|
||||
workflow: z
|
||||
.object({
|
||||
|
|
@ -88,10 +96,12 @@ export const workflowDetailsOutputSchema = z.object({
|
|||
settings: workflowSettingsSchema,
|
||||
connections: z.record(z.unknown()),
|
||||
nodes: z.array(nodeSchema),
|
||||
nodeGroups: z.array(nodeGroupSchema).describe('Node groups in the workflow'),
|
||||
activeVersion: z
|
||||
.object({
|
||||
nodes: z.array(nodeSchema),
|
||||
connections: z.record(z.unknown()),
|
||||
nodeGroups: z.array(nodeGroupSchema).describe('Node groups in the active version'),
|
||||
})
|
||||
.nullable()
|
||||
.describe('Active workflow graph, if available'),
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ const operationTypeSchema = z.enum([
|
|||
'setWorkflowMetadata',
|
||||
'addTags',
|
||||
'removeTags',
|
||||
'setNodeGroups',
|
||||
]);
|
||||
|
||||
const positionInputSchema = z.array(z.number()).length(2).describe('Canvas [x, y].');
|
||||
|
|
@ -117,6 +118,16 @@ const operationInputSchema = z
|
|||
name: z.string().max(128).optional().describe('Only used for setWorkflowMetadata.'),
|
||||
description: z.string().max(255).optional().describe('Only used for setWorkflowMetadata.'),
|
||||
names: z.array(z.string()).optional().describe('For addTags / removeTags.'),
|
||||
nodeGroups: z
|
||||
.array(
|
||||
z.object({
|
||||
id: z.string().optional(),
|
||||
name: z.string(),
|
||||
nodeIds: z.array(z.string()),
|
||||
}),
|
||||
)
|
||||
.optional()
|
||||
.describe('For setNodeGroups. Replaces all node groups; pass [] to clear.'),
|
||||
})
|
||||
.describe('Workflow update operation. Provide fields matching type.');
|
||||
|
||||
|
|
@ -356,6 +367,10 @@ export const createUpdateWorkflowTool = (
|
|||
(op) => op.type !== 'addTags' && op.type !== 'removeTags',
|
||||
);
|
||||
|
||||
// Only persist nodeGroups when a setNodeGroups op ran; otherwise omit the key so
|
||||
// WorkflowService preserves the existing groups (preserve-on-omit).
|
||||
const hasNodeGroupOperation = strictOperations.some((op) => op.type === 'setNodeGroups');
|
||||
|
||||
const workflowUpdateData = new WorkflowEntity();
|
||||
Object.assign(workflowUpdateData, {
|
||||
name: result.workflow.name,
|
||||
|
|
@ -364,6 +379,7 @@ export const createUpdateWorkflowTool = (
|
|||
: {}),
|
||||
nodes: result.workflow.nodes,
|
||||
connections: result.workflow.connections,
|
||||
...(hasNodeGroupOperation ? { nodeGroups: result.workflow.nodeGroups } : {}),
|
||||
meta: hasNonTagOperations
|
||||
? {
|
||||
...(existingWorkflow.meta ?? {}),
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import type {
|
|||
INode,
|
||||
INodeParameters,
|
||||
IWorkflowBase,
|
||||
IWorkflowGroup,
|
||||
NodeConnectionType,
|
||||
} from 'n8n-workflow';
|
||||
import { isSafeObjectProperty, NodeConnectionTypes } from 'n8n-workflow';
|
||||
|
|
@ -184,6 +185,22 @@ export const partialUpdateOperationSchema = z.discriminatedUnion('type', [
|
|||
.max(50)
|
||||
.describe('Tag names to detach from the workflow. Unknown names are ignored.'),
|
||||
}),
|
||||
z.object({
|
||||
type: z.literal('setNodeGroups'),
|
||||
nodeGroups: z
|
||||
.array(
|
||||
z.object({
|
||||
id: z.string().trim().min(1).optional().describe('Group id. Generated if omitted.'),
|
||||
name: z.string().trim().min(1).describe('Unique group name.'),
|
||||
nodeIds: z
|
||||
.array(z.string().trim().min(1))
|
||||
.describe('IDs of the nodes that belong to this group.'),
|
||||
}),
|
||||
)
|
||||
.describe(
|
||||
'Replaces the workflow node groups entirely. Pass [] to remove all groups. Each nodeId must reference an existing node, and every group must form a valid, connected, trigger-free section of the graph (validated on save).',
|
||||
),
|
||||
}),
|
||||
]);
|
||||
|
||||
export type PartialUpdateOperation = z.infer<typeof partialUpdateOperationSchema>;
|
||||
|
|
@ -193,6 +210,8 @@ interface WorkflowSlice {
|
|||
description?: string;
|
||||
nodes: INode[];
|
||||
connections: IConnections;
|
||||
/** Node groups on the workflow. Undefined when never touched; set by setNodeGroups. */
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
/** Existing tag names on the workflow. Undefined when not loaded; tag ops require this. */
|
||||
tagNames?: string[];
|
||||
}
|
||||
|
|
@ -218,6 +237,7 @@ const cloneWorkflow = (workflow: WorkflowSlice): WorkflowSlice => ({
|
|||
description: workflow.description,
|
||||
nodes: workflow.nodes.map((node) => structuredClone(node)),
|
||||
connections: structuredClone(workflow.connections),
|
||||
nodeGroups: workflow.nodeGroups ? structuredClone(workflow.nodeGroups) : undefined,
|
||||
tagNames: workflow.tagNames ? [...workflow.tagNames] : undefined,
|
||||
});
|
||||
|
||||
|
|
@ -590,6 +610,15 @@ export function applyOperations(
|
|||
break;
|
||||
}
|
||||
|
||||
case 'setNodeGroups': {
|
||||
workflow.nodeGroups = op.nodeGroups.map((group) => ({
|
||||
id: group.id ?? uuid(),
|
||||
name: group.name,
|
||||
nodeIds: [...group.nodeIds],
|
||||
}));
|
||||
break;
|
||||
}
|
||||
|
||||
case 'addTags':
|
||||
case 'removeTags': {
|
||||
if (workflow.tagNames === undefined) {
|
||||
|
|
@ -644,6 +673,7 @@ export function toWorkflowSlice(
|
|||
description: (workflow as { description?: string }).description,
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
tagNames,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -754,7 +754,9 @@ export class SourceControlImportService {
|
|||
importedWorkflow.nodeGroups ??= [];
|
||||
|
||||
try {
|
||||
validateWorkflowNodeGroups(importedWorkflow);
|
||||
// Basic checks only: a git import should not reset groups for graph-shape
|
||||
// rules, only for structural integrity (missing refs, duplicate ids/names).
|
||||
validateWorkflowNodeGroups(importedWorkflow, null);
|
||||
} catch {
|
||||
this.logger.warn(
|
||||
`Workflow file ${candidate.file} has invalid nodeGroups, resetting to empty`,
|
||||
|
|
|
|||
|
|
@ -22,6 +22,12 @@ properties:
|
|||
type: object
|
||||
readOnly: true
|
||||
example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } }
|
||||
nodeGroups:
|
||||
type: array
|
||||
readOnly: true
|
||||
description: Visual groupings of nodes shown as frames on the canvas
|
||||
items:
|
||||
$ref: './workflowNodeGroup.yml'
|
||||
authors:
|
||||
type: string
|
||||
readOnly: true
|
||||
|
|
|
|||
|
|
@ -46,6 +46,11 @@ properties:
|
|||
connections:
|
||||
type: object
|
||||
example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } }
|
||||
nodeGroups:
|
||||
type: array
|
||||
description: Visual groupings of nodes shown as frames on the canvas
|
||||
items:
|
||||
$ref: './workflowNodeGroup.yml'
|
||||
settings:
|
||||
$ref: './workflowSettings.yml'
|
||||
staticData:
|
||||
|
|
|
|||
|
|
@ -42,6 +42,11 @@ properties:
|
|||
connections:
|
||||
type: object
|
||||
example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } }
|
||||
nodeGroups:
|
||||
type: array
|
||||
description: Visual groupings of nodes shown as frames on the canvas
|
||||
items:
|
||||
$ref: './workflowNodeGroup.yml'
|
||||
settings:
|
||||
$ref: './workflowSettings.yml'
|
||||
staticData:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,20 @@
|
|||
type: object
|
||||
additionalProperties: false
|
||||
required:
|
||||
- id
|
||||
- name
|
||||
- nodeIds
|
||||
properties:
|
||||
id:
|
||||
type: string
|
||||
description: Unique identifier for the node group
|
||||
example: 9b1c8e2a-4d3f-4a6b-8c7d-1e2f3a4b5c6d
|
||||
name:
|
||||
type: string
|
||||
description: Display name of the node group
|
||||
example: Data processing
|
||||
nodeIds:
|
||||
type: array
|
||||
description: IDs of the nodes that belong to this group
|
||||
items:
|
||||
type: string
|
||||
|
|
@ -28,6 +28,12 @@ properties:
|
|||
example: { Jira: { main: [[{ node: 'Jira', type: 'main', index: 0 }]] } }
|
||||
description: Connections as they were in this version
|
||||
readOnly: true
|
||||
nodeGroups:
|
||||
type: array
|
||||
readOnly: true
|
||||
description: Node groups as they were in this version
|
||||
items:
|
||||
$ref: './workflowNodeGroup.yml'
|
||||
authors:
|
||||
type: string
|
||||
readOnly: true
|
||||
|
|
|
|||
|
|
@ -249,6 +249,7 @@ const workflowHandlers: WorkflowHandlers = {
|
|||
'isArchived',
|
||||
'nodes',
|
||||
'connections',
|
||||
'nodeGroups',
|
||||
'settings',
|
||||
'staticData',
|
||||
'meta',
|
||||
|
|
|
|||
|
|
@ -6,6 +6,8 @@ Execution:
|
|||
$ref: './../../../handlers/executions/spec/schemas/execution.yml'
|
||||
Node:
|
||||
$ref: './../../../handlers/workflows/spec/schemas/node.yml'
|
||||
WorkflowNodeGroup:
|
||||
$ref: './../../../handlers/workflows/spec/schemas/workflowNodeGroup.yml'
|
||||
Tag:
|
||||
$ref: './../../../handlers/tags/spec/schemas/tag.yml'
|
||||
Workflow:
|
||||
|
|
|
|||
|
|
@ -6,13 +6,18 @@ import {
|
|||
formatWorkflowStructureIssuePath,
|
||||
resolveNodeWebhookId,
|
||||
safeParseWorkflowStructure,
|
||||
validateNodeSelectionForGrouping,
|
||||
type IDataObject,
|
||||
type INode,
|
||||
type INodeCredentialsDetails,
|
||||
type INodeTypeDescription,
|
||||
type INodeTypes,
|
||||
type IRun,
|
||||
type ITaskData,
|
||||
type IWorkflowBase,
|
||||
type IWorkflowGroup,
|
||||
type IWorkflowSettings,
|
||||
type NodeGroupValidationResult,
|
||||
type RelatedExecution,
|
||||
type WorkflowStructureIssue,
|
||||
} from 'n8n-workflow';
|
||||
|
|
@ -138,20 +143,97 @@ export function resolveNodeWebhookIds(workflow: IWorkflowBase, nodeTypes: INodeT
|
|||
}
|
||||
|
||||
/**
|
||||
* Validates nodeGroups: unique group names, all referenced node IDs exist,
|
||||
* and each node belongs to at most one group.
|
||||
* Resolves a node to its type description, or `null` for unknown node types.
|
||||
* Used by the grouping validator to detect trigger nodes.
|
||||
*/
|
||||
type GetNodeTypeForGrouping = (node: INode) => INodeTypeDescription | null;
|
||||
|
||||
/**
|
||||
* Builds the `getNodeType` callback that the grouping validator needs to resolve
|
||||
* a node to its type description (used to detect trigger nodes). Returns `null`
|
||||
* for unknown node types so validation degrades gracefully rather than throwing.
|
||||
*/
|
||||
export function makeGetNodeTypeForGrouping(nodeTypes: INodeTypes): GetNodeTypeForGrouping {
|
||||
return (node: INode) => {
|
||||
try {
|
||||
return nodeTypes.getByNameAndVersion(node.type, node.typeVersion).description;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Maps a failed `validateNodeSelectionForGrouping` result to an actionable
|
||||
* `BadRequestError` that names the offending group and the rule it broke.
|
||||
*/
|
||||
function nodeGroupValidationError(
|
||||
group: IWorkflowGroup,
|
||||
result: Extract<NodeGroupValidationResult, { valid: false }>,
|
||||
): BadRequestError {
|
||||
const label = `Node group "${group.name}" (${group.id})`;
|
||||
switch (result.reason) {
|
||||
case 'trigger-selected':
|
||||
return new BadRequestError(
|
||||
`${label} cannot contain trigger nodes: ${result.triggers.join(', ')}.`,
|
||||
);
|
||||
case 'invalid-subgraph':
|
||||
return new BadRequestError(
|
||||
`${label} must form a single connected subgraph with a single entry and exit.`,
|
||||
);
|
||||
case 'multiple-input-branches':
|
||||
return new BadRequestError(`${label} has multiple input branches at node "${result.node}".`);
|
||||
case 'multiple-output-branches':
|
||||
return new BadRequestError(`${label} has multiple output branches at node "${result.node}".`);
|
||||
case 'node-already-grouped':
|
||||
return new BadRequestError(
|
||||
`${label} contains nodes that already belong to another group: ${result.nodeIds.join(', ')}.`,
|
||||
);
|
||||
case 'non-main-boundary':
|
||||
return new BadRequestError(
|
||||
`${label} cannot cross the "${result.connection.type}" connection between "${result.connection.source}" and "${result.connection.target}".`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates nodeGroups.
|
||||
*
|
||||
* Basic checks (always run): unique group IDs, unique group names, all referenced
|
||||
* node IDs exist, and each node belongs to at most one group.
|
||||
*
|
||||
* Full checks (run only when `getNodeType` is non-null): each group must satisfy
|
||||
* the same grouping rules the canvas enforces — no triggers, a single connected
|
||||
* subgraph, and no non-main connection crossing the group boundary — validated
|
||||
* against the other groups as existing groups. Pass the `getNodeType` callback to
|
||||
* run the full checks (on create, and on an update that changed the graph or the
|
||||
* groups); pass `null` to run basic checks only (e.g. a git import, so
|
||||
* legacy-invalid groups don't block the import).
|
||||
*
|
||||
* Note for frontend: Must be called after `addNodeIds` since nodes created via the API
|
||||
* may not have IDs until that step assigns them.
|
||||
*/
|
||||
export function validateWorkflowNodeGroups(workflow: Pick<IWorkflowBase, 'nodes' | 'nodeGroups'>) {
|
||||
export function validateWorkflowNodeGroups(
|
||||
workflow: Pick<IWorkflowBase, 'nodes' | 'nodeGroups'> & {
|
||||
connections?: IWorkflowBase['connections'];
|
||||
},
|
||||
getNodeType: GetNodeTypeForGrouping | null,
|
||||
) {
|
||||
const { nodeGroups, nodes } = workflow;
|
||||
if (!nodeGroups || nodeGroups.length === 0) return;
|
||||
|
||||
const nodeIds = new Set(nodes.map((n) => n.id).filter(Boolean));
|
||||
const seenGroupIds = new Set<string>();
|
||||
const seenGroupNames = new Set<string>();
|
||||
const nodeToGroup = new Map<string, string>();
|
||||
|
||||
for (const group of nodeGroups) {
|
||||
// Unique group IDs
|
||||
if (seenGroupIds.has(group.id)) {
|
||||
throw new BadRequestError(`Duplicate node group ID "${group.id}".`);
|
||||
}
|
||||
seenGroupIds.add(group.id);
|
||||
|
||||
// Unique group names
|
||||
if (seenGroupNames.has(group.name)) {
|
||||
throw new BadRequestError(`Duplicate node group name "${group.name}".`);
|
||||
|
|
@ -175,6 +257,24 @@ export function validateWorkflowNodeGroups(workflow: Pick<IWorkflowBase, 'nodes'
|
|||
nodeToGroup.set(nodeId, group.name);
|
||||
}
|
||||
}
|
||||
|
||||
if (!getNodeType) return;
|
||||
|
||||
const nodeById = new Map(nodes.map((node) => [node.id, node]));
|
||||
const connectionsBySourceNode = workflow.connections ?? {};
|
||||
|
||||
for (const group of nodeGroups) {
|
||||
const groupNodes = group.nodeIds.flatMap((id) => nodeById.get(id) ?? []);
|
||||
const result = validateNodeSelectionForGrouping({
|
||||
nodes: groupNodes,
|
||||
connectionsBySourceNode,
|
||||
getNodeType,
|
||||
existingNodeGroups: nodeGroups.filter((other) => other.id !== group.id),
|
||||
});
|
||||
if (!result.valid) {
|
||||
throw nodeGroupValidationError(group, result);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -302,6 +302,7 @@ describe('WorkflowService', () => {
|
|||
const userHasScopesMock = jest.mocked(userHasScopes);
|
||||
let workflowService: WorkflowService;
|
||||
let workflowFinderServiceMock: MockProxy<WorkflowFinderService>;
|
||||
let workflowHistoryServiceMock: MockProxy<WorkflowHistoryService>;
|
||||
let licenseStateMock: MockProxy<LicenseState>;
|
||||
let redactionEnforcementServiceMock: MockProxy<RedactionEnforcementService>;
|
||||
let workflowRepositoryMock: MockProxy<{
|
||||
|
|
@ -311,6 +312,7 @@ describe('WorkflowService', () => {
|
|||
|
||||
beforeEach(() => {
|
||||
workflowFinderServiceMock = mock<WorkflowFinderService>();
|
||||
workflowHistoryServiceMock = mock<WorkflowHistoryService>();
|
||||
workflowRepositoryMock = mock();
|
||||
licenseStateMock = mock<LicenseState>();
|
||||
licenseStateMock.isDataRedactionLicensed.mockReturnValue(true);
|
||||
|
|
@ -329,7 +331,7 @@ describe('WorkflowService', () => {
|
|||
mock(), // binaryDataService
|
||||
ownershipServiceMock, // ownershipService
|
||||
mock(), // tagService
|
||||
mock(), // workflowHistoryService
|
||||
workflowHistoryServiceMock, // workflowHistoryService
|
||||
mock(), // externalHooks
|
||||
mock(), // activeWorkflowManager
|
||||
mock(), // roleService
|
||||
|
|
@ -436,20 +438,76 @@ describe('WorkflowService', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('should validate nodeGroups against existing workflow when not in payload', async () => {
|
||||
test('validates the existing nodeGroups (full) when the graph changes but groups are omitted', async () => {
|
||||
const existingNodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }];
|
||||
const existingWorkflow = setupExistingWorkflow();
|
||||
existingWorkflow.nodeGroups = existingNodeGroups;
|
||||
|
||||
// The getNodeType callback being passed through is the signal that full checks ran.
|
||||
const getNodeTypeStub = jest.fn();
|
||||
jest.mocked(WorkflowHelpers.makeGetNodeTypeForGrouping).mockReturnValue(getNodeTypeStub);
|
||||
|
||||
// Change the nodes so validation runs; omit nodeGroups so they are backfilled.
|
||||
const changedNodes = [
|
||||
{ id: 'n1', name: 'N1', type: 't', typeVersion: 1, position: [0, 0], parameters: {} },
|
||||
];
|
||||
const user = mock<User>();
|
||||
await workflowService.update(
|
||||
user,
|
||||
{ nodes: changedNodes } as unknown as WorkflowEntity,
|
||||
'workflow-1',
|
||||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(WorkflowHelpers.validateWorkflowNodeGroups).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
nodes: changedNodes,
|
||||
nodeGroups: existingNodeGroups,
|
||||
}),
|
||||
getNodeTypeStub,
|
||||
);
|
||||
});
|
||||
|
||||
test('skips nodeGroup validation on a metadata-only edit (nodes/connections/groups unchanged)', async () => {
|
||||
const existingWorkflow = setupExistingWorkflow();
|
||||
existingWorkflow.nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }];
|
||||
|
||||
const user = mock<User>();
|
||||
await workflowService.update(
|
||||
user,
|
||||
{ name: 'Renamed workflow' } as unknown as WorkflowEntity,
|
||||
'workflow-1',
|
||||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(WorkflowHelpers.validateWorkflowNodeGroups).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('backfills existing nodeGroups into the saved history version when omitted', async () => {
|
||||
const existingNodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['n1'] }];
|
||||
const existingWorkflow = setupExistingWorkflow();
|
||||
existingWorkflow.nodeGroups = existingNodeGroups;
|
||||
|
||||
const user = mock<User>();
|
||||
await workflowService.update(user, { nodes: [] } as unknown as WorkflowEntity, 'workflow-1', {
|
||||
forceSave: true,
|
||||
});
|
||||
// Change nodes (forces a new version) while omitting nodeGroups.
|
||||
await workflowService.update(
|
||||
user,
|
||||
{
|
||||
nodes: [
|
||||
{ id: 'n1', name: 'N1', type: 't', typeVersion: 1, position: [0, 0], parameters: {} },
|
||||
],
|
||||
} as unknown as WorkflowEntity,
|
||||
'workflow-1',
|
||||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(WorkflowHelpers.validateWorkflowNodeGroups).toHaveBeenCalledWith({
|
||||
nodes: [],
|
||||
nodeGroups: existingNodeGroups,
|
||||
});
|
||||
// The history version must record the live (effective) groups, not empty.
|
||||
expect(workflowHistoryServiceMock.saveVersion).toHaveBeenCalledWith(
|
||||
user,
|
||||
expect.objectContaining({ nodeGroups: existingNodeGroups }),
|
||||
'workflow-1',
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test('should throw BadRequestError for invalid workflow structure', async () => {
|
||||
|
|
|
|||
|
|
@ -124,7 +124,10 @@ export class WorkflowCreationService {
|
|||
WorkflowHelpers.addNodeIds(newWorkflow);
|
||||
WorkflowHelpers.resolveNodeWebhookIds(newWorkflow, this.nodeTypes);
|
||||
WorkflowHelpers.validateWorkflowStructure(newWorkflow);
|
||||
WorkflowHelpers.validateWorkflowNodeGroups(newWorkflow);
|
||||
WorkflowHelpers.validateWorkflowNodeGroups(
|
||||
newWorkflow,
|
||||
WorkflowHelpers.makeGetNodeTypeForGrouping(this.nodeTypes),
|
||||
);
|
||||
|
||||
if (parentFolderId && parentFolderId !== PROJECT_ROOT) {
|
||||
await this.findParentFolderInProjectOrFail(parentFolderId, effectiveProjectId);
|
||||
|
|
|
|||
|
|
@ -503,6 +503,7 @@ describe('WorkflowHistoryService', () => {
|
|||
workflow.id = 'wf-1';
|
||||
workflow.versionId = 'wfv-fresh';
|
||||
workflow.connections = {};
|
||||
workflow.nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: ['uuid-1234'] }];
|
||||
workflowRepository.findOneBy.mockResolvedValueOnce(workflow);
|
||||
// First findOne: no existing history row → insert path.
|
||||
// Second findOne: post-save verification that the row now exists.
|
||||
|
|
@ -521,6 +522,7 @@ describe('WorkflowHistoryService', () => {
|
|||
workflowId: 'wf-1',
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -140,6 +140,7 @@ export class WorkflowHistoryService {
|
|||
versionId: workflow.versionId,
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
},
|
||||
workflowId,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -419,9 +419,11 @@ export class WorkflowService {
|
|||
},
|
||||
);
|
||||
|
||||
// To save a version, we need both nodes and connections
|
||||
// A saved version needs nodes, connections, and node groups; backfill any the update
|
||||
// omitted from the persisted workflow so the history row records the effective state.
|
||||
workflowUpdateData.nodes = workflowUpdateData.nodes ?? workflow.nodes;
|
||||
workflowUpdateData.connections = workflowUpdateData.connections ?? workflow.connections;
|
||||
workflowUpdateData.nodeGroups = workflowUpdateData.nodeGroups ?? workflow.nodeGroups;
|
||||
} else {
|
||||
// Do not let users change versionId directly
|
||||
workflowUpdateData.versionId = workflow.versionId;
|
||||
|
|
@ -433,10 +435,18 @@ export class WorkflowService {
|
|||
nodes: workflowUpdateData.nodes ?? workflow.nodes,
|
||||
connections: workflowUpdateData.connections ?? workflow.connections,
|
||||
});
|
||||
WorkflowHelpers.validateWorkflowNodeGroups({
|
||||
nodes: workflowUpdateData.nodes ?? workflow.nodes,
|
||||
nodeGroups: workflowUpdateData.nodeGroups ?? workflow.nodeGroups,
|
||||
});
|
||||
// Validate node groups only for structural changes; a metadata-only edit re-persists
|
||||
// already-validated groups, so re-checking is redundant and could block on legacy data.
|
||||
if (saveNewVersion) {
|
||||
WorkflowHelpers.validateWorkflowNodeGroups(
|
||||
{
|
||||
nodes: workflowUpdateData.nodes,
|
||||
nodeGroups: workflowUpdateData.nodeGroups,
|
||||
connections: workflowUpdateData.connections,
|
||||
},
|
||||
WorkflowHelpers.makeGetNodeTypeForGrouping(this.nodeTypes),
|
||||
);
|
||||
}
|
||||
|
||||
// Strip redactionPolicy if instance lacks data-redaction license
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -187,6 +187,7 @@ describe('GET /workflows', () => {
|
|||
activeVersionId,
|
||||
staticData,
|
||||
nodes,
|
||||
nodeGroups,
|
||||
settings,
|
||||
name,
|
||||
createdAt,
|
||||
|
|
@ -205,6 +206,7 @@ describe('GET /workflows', () => {
|
|||
expect(activeVersionId).toBeNull();
|
||||
expect(staticData).toBeDefined();
|
||||
expect(nodes).toBeDefined();
|
||||
expect(nodeGroups).toBeDefined();
|
||||
expect(tags).toBeDefined();
|
||||
expect(settings).toBeDefined();
|
||||
expect(createdAt).toBeDefined();
|
||||
|
|
@ -216,6 +218,34 @@ describe('GET /workflows', () => {
|
|||
}
|
||||
});
|
||||
|
||||
test('should include node groups when returning owned workflows', async () => {
|
||||
const workflow = await createWorkflowWithHistory(
|
||||
{
|
||||
nodes: [
|
||||
{
|
||||
id: 'uuid-1234',
|
||||
name: 'Schedule Trigger',
|
||||
parameters: {},
|
||||
position: [-20, 260],
|
||||
type: 'n8n-nodes-base.scheduleTrigger',
|
||||
typeVersion: 1,
|
||||
},
|
||||
],
|
||||
nodeGroups: [{ id: 'group-1', name: 'Processing', nodeIds: ['uuid-1234'] }],
|
||||
},
|
||||
member,
|
||||
);
|
||||
|
||||
const response = await authMemberAgent.get('/workflows');
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.body.data).toHaveLength(1);
|
||||
expect(response.body.data[0].id).toBe(workflow.id);
|
||||
expect(response.body.data[0].nodeGroups).toEqual([
|
||||
{ id: 'group-1', name: 'Processing', nodeIds: ['uuid-1234'] },
|
||||
]);
|
||||
});
|
||||
|
||||
test('should return all owned workflows with pagination', async () => {
|
||||
await Promise.all([
|
||||
createWorkflowWithHistory({}, member),
|
||||
|
|
@ -1581,6 +1611,54 @@ describe('POST /workflows', () => {
|
|||
expect(sharedWorkflow?.role).toEqual('workflow:owner');
|
||||
});
|
||||
|
||||
test('should create workflow with node groups', async () => {
|
||||
const payload = {
|
||||
name: 'grouped',
|
||||
nodes: [
|
||||
triggerNode,
|
||||
{
|
||||
id: 'uuid-5678',
|
||||
parameters: {},
|
||||
name: 'Step A',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [460, 300],
|
||||
},
|
||||
{
|
||||
id: 'uuid-9012',
|
||||
parameters: {},
|
||||
name: 'Step B',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [680, 300],
|
||||
},
|
||||
],
|
||||
connections: {
|
||||
Start: { main: [[{ node: 'Step A', type: 'main', index: 0 }]] },
|
||||
'Step A': { main: [[{ node: 'Step B', type: 'main', index: 0 }]] },
|
||||
},
|
||||
settings: { executionOrder: 'v1' },
|
||||
nodeGroups: [{ id: 'group-1', name: 'Processing', nodeIds: ['uuid-5678', 'uuid-9012'] }],
|
||||
};
|
||||
|
||||
const response = await authOwnerAgent.post('/workflows').send(payload);
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.body.nodeGroups).toEqual(payload.nodeGroups);
|
||||
});
|
||||
|
||||
test('should reject workflow with a node group referencing a missing node', async () => {
|
||||
const payload = {
|
||||
...mockPostWorkflowPayload('bad-group'),
|
||||
nodeGroups: [{ id: 'group-1', name: 'Ghost', nodeIds: ['does-not-exist'] }],
|
||||
};
|
||||
|
||||
const response = await authOwnerAgent.post('/workflows').send(payload);
|
||||
|
||||
expect(response.statusCode).toBe(400);
|
||||
expect(response.body.message).toContain('does not exist in the workflow');
|
||||
});
|
||||
|
||||
test('should assign webhookId to webhook nodes created via public API', async () => {
|
||||
const payload = {
|
||||
name: 'webhook-test',
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user