mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-12 16:10:30 +02:00
Merge cc200e2dbb into ab0c1b715f
This commit is contained in:
commit
469b417617
|
|
@ -58,6 +58,24 @@ describe('CreateWorkflowDto', () => {
|
|||
settings: { redactionPolicy: 'non-manual' },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'with nodeGroups',
|
||||
request: {
|
||||
name: 'Grouped Workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'group1', name: 'Data Fetching', nodeIds: ['node1', 'node2'] }],
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'with empty nodeGroups array',
|
||||
request: {
|
||||
name: 'Empty Groups Workflow',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [],
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'with tags as objects (backward compatibility)',
|
||||
request: {
|
||||
|
|
@ -138,6 +156,71 @@ describe('CreateWorkflowDto', () => {
|
|||
request: { name: 'Test', nodes: [], connections: {}, pinData: [] },
|
||||
expectedErrorPath: ['pinData'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups as string',
|
||||
request: { name: 'Test', nodes: [], connections: {}, nodeGroups: 'not-an-array' },
|
||||
expectedErrorPath: ['nodeGroups'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing id',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ name: 'Group', nodeIds: [] }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'id'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing name',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', nodeIds: [] }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'name'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing nodeIds',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Group' }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'nodeIds'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty id',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: '', name: 'Group', nodeIds: [] }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'id'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty name',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: '', nodeIds: [] }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'name'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty nodeId string',
|
||||
request: {
|
||||
name: 'Test',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: [''] }],
|
||||
},
|
||||
expectedErrorPath: ['nodeGroups', 0, 'nodeIds', 0],
|
||||
},
|
||||
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
|
||||
const result = CreateWorkflowDto.safeParse(request);
|
||||
expect(result.success).toBe(false);
|
||||
|
|
|
|||
|
|
@ -49,6 +49,16 @@ describe('UpdateWorkflowDto', () => {
|
|||
settings: { redactionPolicy: 'non-manual' },
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'update nodeGroups',
|
||||
request: {
|
||||
nodeGroups: [{ id: 'group1', name: 'Data Fetching', nodeIds: ['node1', 'node2'] }],
|
||||
},
|
||||
},
|
||||
{
|
||||
name: 'set nodeGroups to empty array',
|
||||
request: { nodeGroups: [] },
|
||||
},
|
||||
{
|
||||
name: 'update multiple fields',
|
||||
request: {
|
||||
|
|
@ -118,6 +128,41 @@ describe('UpdateWorkflowDto', () => {
|
|||
request: { pinData: [] },
|
||||
expectedErrorPath: ['pinData'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups as string',
|
||||
request: { nodeGroups: 'not-an-array' },
|
||||
expectedErrorPath: ['nodeGroups'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing id',
|
||||
request: { nodeGroups: [{ name: 'Group', nodeIds: [] }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'id'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing name',
|
||||
request: { nodeGroups: [{ id: 'g1', nodeIds: [] }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'name'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with missing nodeIds',
|
||||
request: { nodeGroups: [{ id: 'g1', name: 'Group' }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'nodeIds'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty id',
|
||||
request: { nodeGroups: [{ id: '', name: 'Group', nodeIds: [] }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'id'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty name',
|
||||
request: { nodeGroups: [{ id: 'g1', name: '', nodeIds: [] }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'name'],
|
||||
},
|
||||
{
|
||||
name: 'nodeGroups with empty nodeId string',
|
||||
request: { nodeGroups: [{ id: 'g1', name: 'Group', nodeIds: [''] }] },
|
||||
expectedErrorPath: ['nodeGroups', 0, 'nodeIds', 0],
|
||||
},
|
||||
])('should fail validation for $name', ({ request, expectedErrorPath }) => {
|
||||
const result = UpdateWorkflowDto.safeParse(request);
|
||||
expect(result.success).toBe(false);
|
||||
|
|
|
|||
|
|
@ -70,6 +70,14 @@ export const workflowPinDataSchema = z.custom<IPinData | null>(
|
|||
|
||||
export const workflowMetaSchema = z.record(z.string(), z.unknown()).nullable();
|
||||
|
||||
const workflowGroupSchema = z.object({
|
||||
id: z.string().min(1),
|
||||
name: z.string().min(1),
|
||||
nodeIds: z.array(z.string().min(1)),
|
||||
});
|
||||
|
||||
export const workflowNodeGroupsSchema = z.array(workflowGroupSchema);
|
||||
|
||||
/**
|
||||
* Base workflow shape containing fields shared between Create and Update DTOs.
|
||||
*/
|
||||
|
|
@ -85,6 +93,7 @@ export const baseWorkflowShape = {
|
|||
staticData: workflowStaticDataSchema.optional(),
|
||||
meta: workflowMetaSchema.optional(),
|
||||
pinData: workflowPinDataSchema.optional(),
|
||||
nodeGroups: workflowNodeGroupsSchema.optional(),
|
||||
hash: z.string().optional(),
|
||||
|
||||
// Folder organization
|
||||
|
|
|
|||
|
|
@ -16,7 +16,8 @@ import { NodeConnectionTypes } from 'n8n-workflow';
|
|||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
export function newWorkflow(attributes: Partial<IWorkflowDb> = {}): IWorkflowDb {
|
||||
const { active, isArchived, name, nodes, connections, versionId, settings } = attributes;
|
||||
const { active, isArchived, name, nodes, connections, versionId, settings, nodeGroups } =
|
||||
attributes;
|
||||
|
||||
const workflowEntity = Container.get(WorkflowRepository).create({
|
||||
active: active ?? false,
|
||||
|
|
@ -33,6 +34,7 @@ export function newWorkflow(attributes: Partial<IWorkflowDb> = {}): IWorkflowDb
|
|||
},
|
||||
],
|
||||
connections: connections ?? {},
|
||||
nodeGroups: nodeGroups ?? [],
|
||||
versionId: versionId ?? uuid(),
|
||||
settings: settings ?? {},
|
||||
...attributes,
|
||||
|
|
@ -256,15 +258,19 @@ export async function createWorkflowHistory(
|
|||
: 'Test User'
|
||||
: 'Test User';
|
||||
|
||||
await Container.get(WorkflowHistoryRepository).insert({
|
||||
workflowId: workflow.id,
|
||||
versionId: workflow.versionId,
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
authors,
|
||||
autosaved: false,
|
||||
...overrides,
|
||||
});
|
||||
const repo = Container.get(WorkflowHistoryRepository);
|
||||
await repo.insert(
|
||||
repo.create({
|
||||
workflowId: workflow.id,
|
||||
versionId: workflow.versionId,
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
nodeGroups: workflow.nodeGroups ?? [],
|
||||
authors,
|
||||
autosaved: false,
|
||||
...overrides,
|
||||
}),
|
||||
);
|
||||
|
||||
if (withPublishHistory) {
|
||||
// We wait a millisecond as createdAt order is often relevant for the publishing history
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ import {
|
|||
} from '@n8n/typeorm';
|
||||
import { Length } from 'class-validator';
|
||||
import { IConnections, IDataObject, IWorkflowSettings, WorkflowFEMeta } from 'n8n-workflow';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import type { INode, IWorkflowGroup } from 'n8n-workflow';
|
||||
|
||||
import { JsonColumn, WithTimestampsAndStringId, dbType } from './abstract-entity';
|
||||
import { type Folder } from './folder';
|
||||
|
|
@ -70,6 +70,9 @@ export class WorkflowEntity extends WithTimestampsAndStringId implements IWorkfl
|
|||
})
|
||||
meta?: WorkflowFEMeta;
|
||||
|
||||
@JsonColumn()
|
||||
nodeGroups: IWorkflowGroup[];
|
||||
|
||||
@ManyToMany('TagEntity', 'workflows')
|
||||
@JoinTable({
|
||||
name: 'workflows_tags', // table name for the junction table of this relation
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { Column, Entity, ManyToOne, OneToMany, PrimaryColumn, Relation } from '@n8n/typeorm';
|
||||
import { IConnections } from 'n8n-workflow';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import type { INode, IWorkflowGroup } from 'n8n-workflow';
|
||||
|
||||
import { JsonColumn, WithTimestamps } from './abstract-entity';
|
||||
import { WorkflowEntity } from './workflow-entity';
|
||||
|
|
@ -20,6 +20,9 @@ export class WorkflowHistory extends WithTimestamps {
|
|||
@JsonColumn()
|
||||
connections: IConnections;
|
||||
|
||||
@JsonColumn()
|
||||
nodeGroups: IWorkflowGroup[];
|
||||
|
||||
@Column()
|
||||
authors: string;
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import {
|
|||
replaceInvalidCredentials,
|
||||
shouldRestartParentExecution,
|
||||
validatePinDataSize,
|
||||
validateWorkflowNodeGroups,
|
||||
} from '@/workflow-helpers';
|
||||
|
||||
describe('workflow-helpers', () => {
|
||||
|
|
@ -380,6 +381,62 @@ describe('removeDefaultValues', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('validateWorkflowNodeGroups', () => {
|
||||
const makeNode = (id: string) =>
|
||||
({ id, name: `Node ${id}`, type: 'test', position: [0, 0], parameters: {} }) as never;
|
||||
|
||||
it('should pass when nodeGroups is undefined', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: undefined }),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('should pass when nodeGroups is empty', () => {
|
||||
expect(() =>
|
||||
validateWorkflowNodeGroups({ nodes: [makeNode('n1')], nodeGroups: [] }),
|
||||
).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'] }],
|
||||
}),
|
||||
).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'] }],
|
||||
}),
|
||||
).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'] }],
|
||||
}),
|
||||
).toThrow('Group "Empty Group" references node ID "bad1"');
|
||||
});
|
||||
|
||||
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: [] },
|
||||
],
|
||||
}),
|
||||
).toThrow('Duplicate node group name "Duplicate".');
|
||||
});
|
||||
});
|
||||
|
||||
describe('validatePinDataSize', () => {
|
||||
const baseWorkflow: IWorkflowBase = {
|
||||
id: '1',
|
||||
|
|
|
|||
|
|
@ -726,7 +726,9 @@ export class SourceControlImportService {
|
|||
|
||||
const importedWorkflow = await this.parseWorkflowFromFile(candidate.file);
|
||||
|
||||
const { versionId, nodes, connections, id, owner } = importedWorkflow;
|
||||
importedWorkflow.nodeGroups ??= [];
|
||||
|
||||
const { versionId, nodes, connections, id, owner, nodeGroups } = importedWorkflow;
|
||||
|
||||
if (!id || !versionId || !nodes || !connections) {
|
||||
this.logger.error(
|
||||
|
|
@ -764,7 +766,10 @@ export class SourceControlImportService {
|
|||
}
|
||||
|
||||
try {
|
||||
await this.saveOrUpdateWorkflowHistory({ id, versionId, nodes, connections }, userId);
|
||||
await this.saveOrUpdateWorkflowHistory(
|
||||
{ id, versionId, nodes, connections, nodeGroups },
|
||||
userId,
|
||||
);
|
||||
} catch (error) {
|
||||
const e = ensureError(error);
|
||||
this.logger.error(`Failed to save or update workflow history for workflow ${id}`, {
|
||||
|
|
@ -1663,11 +1668,12 @@ export class SourceControlImportService {
|
|||
versionId: string;
|
||||
nodes: IWorkflowToImport['nodes'];
|
||||
connections: IWorkflowToImport['connections'];
|
||||
nodeGroups: IWorkflowToImport['nodeGroups'];
|
||||
id: string;
|
||||
},
|
||||
userId: string,
|
||||
): Promise<void> {
|
||||
const { versionId, nodes, connections, id } = importedWorkflow;
|
||||
const { versionId, nodes, connections, nodeGroups, id } = importedWorkflow;
|
||||
|
||||
// Fetch user for author info
|
||||
const user = await this.userRepository.findOne({ where: { id: userId } });
|
||||
|
|
@ -1676,20 +1682,26 @@ export class SourceControlImportService {
|
|||
const existingVersion = await this.workflowHistoryService.findVersion(id, versionId);
|
||||
|
||||
if (existingVersion) {
|
||||
// Check if nodes or connections changed
|
||||
// Check if workflow content changed
|
||||
const nodesChanged = !isEqual(existingVersion.nodes, nodes);
|
||||
const connectionsChanged = !isEqual(existingVersion.connections, connections);
|
||||
const nodeGroupsChanged = !isEqual(existingVersion.nodeGroups, nodeGroups);
|
||||
|
||||
if (nodesChanged || connectionsChanged) {
|
||||
if (nodesChanged || connectionsChanged || nodeGroupsChanged) {
|
||||
await this.workflowHistoryService.updateVersion(id, versionId, {
|
||||
nodes,
|
||||
connections,
|
||||
nodeGroups,
|
||||
authors,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
// Create new version history record
|
||||
await this.workflowHistoryService.saveVersion(authors, { versionId, nodes, connections }, id);
|
||||
await this.workflowHistoryService.saveVersion(
|
||||
authors,
|
||||
{ versionId, nodes, connections, nodeGroups },
|
||||
id,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -269,7 +269,7 @@ export declare namespace VariablesRequest {
|
|||
export declare namespace WorkflowHistoryRequest {
|
||||
type GetList = AuthenticatedRequest<
|
||||
{ workflowId: string },
|
||||
Array<Omit<WorkflowHistory, 'nodes' | 'connections'>>,
|
||||
Array<Omit<WorkflowHistory, 'nodes' | 'connections' | 'nodeGroups'>>,
|
||||
{},
|
||||
ListQuery.Options
|
||||
>;
|
||||
|
|
|
|||
|
|
@ -195,6 +195,7 @@ export class ImportService {
|
|||
workflowId: workflow.id,
|
||||
nodes: workflow.nodes,
|
||||
connections: workflow.connections,
|
||||
nodeGroups: workflow.nodeGroups ?? [],
|
||||
authors: 'import',
|
||||
name: versionMetadata?.name ?? null,
|
||||
description: versionMetadata?.description ?? null,
|
||||
|
|
|
|||
|
|
@ -125,6 +125,35 @@ export function resolveNodeWebhookIds(workflow: IWorkflowBase, nodeTypes: INodeT
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates that all nodeIds in nodeGroups reference existing node IDs
|
||||
* and that group names are unique.
|
||||
* 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'>) {
|
||||
const { nodeGroups, nodes } = workflow;
|
||||
if (!nodeGroups || nodeGroups.length === 0) return;
|
||||
|
||||
const nodeIds = new Set(nodes.map((n) => n.id).filter(Boolean));
|
||||
const seenGroupNames = new Set<string>();
|
||||
|
||||
for (const group of nodeGroups) {
|
||||
if (seenGroupNames.has(group.name)) {
|
||||
throw new BadRequestError(`Duplicate node group name "${group.name}".`);
|
||||
}
|
||||
seenGroupNames.add(group.name);
|
||||
|
||||
for (const nodeId of group.nodeIds) {
|
||||
if (!nodeIds.has(nodeId)) {
|
||||
throw new BadRequestError(
|
||||
`Group "${group.name}" references node ID "${nodeId}" that does not exist in the workflow.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function validateWorkflowStructure(workflow: Pick<IWorkflowBase, 'nodes' | 'connections'>) {
|
||||
const result = safeParseWorkflowStructure(workflow);
|
||||
|
||||
|
|
|
|||
|
|
@ -161,6 +161,25 @@ describe('WorkflowCreationService', () => {
|
|||
'The workflow you are trying to save contains credentials that are not shared with you',
|
||||
);
|
||||
});
|
||||
|
||||
it('should default missing nodeGroups before saving', async () => {
|
||||
projectServiceMock.getProjectWithScope.mockResolvedValue({ id: 'project-1' } as never);
|
||||
licenseStateMock.isSharingLicensed.mockReturnValue(false);
|
||||
const { transactionManager } = setupTransactionMocks();
|
||||
|
||||
const user = mock<User>();
|
||||
const newWorkflow = new WorkflowEntity();
|
||||
newWorkflow.name = 'Test';
|
||||
newWorkflow.nodes = [];
|
||||
newWorkflow.connections = {};
|
||||
|
||||
await expect(
|
||||
workflowCreationService.createWorkflow(user, newWorkflow, { projectId: 'project-1' }),
|
||||
).rejects.toThrow('Stopping for test');
|
||||
|
||||
const savedEntity = transactionManager.save.mock.calls[0][0] as WorkflowEntity;
|
||||
expect(savedEntity.nodeGroups).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('redaction policy scope enforcement on create', () => {
|
||||
|
|
|
|||
|
|
@ -237,6 +237,81 @@ describe('WorkflowService', () => {
|
|||
return { settings } as unknown as WorkflowEntity;
|
||||
}
|
||||
|
||||
test('should save new version when nodeGroups change', async () => {
|
||||
setupExistingWorkflow();
|
||||
|
||||
const user = mock<User>();
|
||||
await workflowService.update(
|
||||
user,
|
||||
{
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: [] }],
|
||||
} as unknown as WorkflowEntity,
|
||||
'workflow-1',
|
||||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
versionId: expect.not.stringMatching('v1'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test('should not save new version when nodeGroups are unchanged', async () => {
|
||||
const nodeGroups = [{ id: 'g1', name: 'Group 1', nodeIds: [] }];
|
||||
const existingWorkflow = {
|
||||
id: 'workflow-1',
|
||||
isArchived: false,
|
||||
versionId: 'v1',
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups,
|
||||
settings: {},
|
||||
activeVersionId: undefined,
|
||||
tags: [],
|
||||
} as unknown as WorkflowEntity;
|
||||
workflowFinderServiceMock.findWorkflowForUser.mockResolvedValue(existingWorkflow);
|
||||
workflowRepositoryMock.findOne.mockResolvedValue(existingWorkflow);
|
||||
|
||||
const user = mock<User>();
|
||||
await workflowService.update(
|
||||
user,
|
||||
{
|
||||
nodes: [],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'g1', name: 'Group 1', nodeIds: [] }],
|
||||
} as unknown as WorkflowEntity,
|
||||
'workflow-1',
|
||||
{ forceSave: true },
|
||||
);
|
||||
|
||||
expect(workflowRepositoryMock.update).toHaveBeenCalledWith(
|
||||
'workflow-1',
|
||||
expect.objectContaining({
|
||||
versionId: 'v1',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
test('should validate nodeGroups against existing workflow when not in payload', 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,
|
||||
});
|
||||
|
||||
expect(WorkflowHelpers.validateWorkflowNodeGroups).toHaveBeenCalledWith({
|
||||
nodes: [],
|
||||
nodeGroups: existingNodeGroups,
|
||||
});
|
||||
});
|
||||
|
||||
test('should throw BadRequestError for invalid workflow structure', async () => {
|
||||
setupExistingWorkflow();
|
||||
jest.mocked(WorkflowHelpers.validateWorkflowStructure).mockImplementationOnce(() => {
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ export class WorkflowCreationService {
|
|||
// Ensure workflow is created as inactive
|
||||
newWorkflow.active = false;
|
||||
newWorkflow.versionId = uuid();
|
||||
newWorkflow.nodeGroups ??= [];
|
||||
|
||||
await validateEntity(newWorkflow);
|
||||
|
||||
|
|
@ -110,6 +111,7 @@ export class WorkflowCreationService {
|
|||
WorkflowHelpers.addNodeIds(newWorkflow);
|
||||
WorkflowHelpers.resolveNodeWebhookIds(newWorkflow, this.nodeTypes);
|
||||
WorkflowHelpers.validateWorkflowStructure(newWorkflow);
|
||||
WorkflowHelpers.validateWorkflowNodeGroups(newWorkflow);
|
||||
|
||||
if ('pinData' in newWorkflow) {
|
||||
WorkflowHelpers.validatePinDataSize(newWorkflow);
|
||||
|
|
|
|||
|
|
@ -60,6 +60,31 @@ describe('WorkflowHistoryService', () => {
|
|||
authors: 'John Doe',
|
||||
connections: {},
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
versionId: workflow.versionId,
|
||||
workflowId,
|
||||
autosaved: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('should save a new version with nodeGroups when provided', async () => {
|
||||
// Arrange
|
||||
const workflow = getWorkflow({ addNodeWithoutCreds: true });
|
||||
const workflowId = '123';
|
||||
workflow.connections = {};
|
||||
workflow.id = workflowId;
|
||||
workflow.versionId = '456';
|
||||
const nodeGroups = [{ id: 'group1', name: 'Data Fetching', nodeIds: ['node1', 'node2'] }];
|
||||
|
||||
// Act
|
||||
await workflowHistoryService.saveVersion(testUser, { ...workflow, nodeGroups }, workflowId);
|
||||
|
||||
// Assert
|
||||
expect(workflowHistoryRepository.insert).toHaveBeenCalledWith({
|
||||
authors: 'John Doe',
|
||||
connections: {},
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups,
|
||||
versionId: workflow.versionId,
|
||||
workflowId,
|
||||
autosaved: false,
|
||||
|
|
@ -82,6 +107,7 @@ describe('WorkflowHistoryService', () => {
|
|||
authors: 'John Doe',
|
||||
connections: {},
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
versionId: workflow.versionId,
|
||||
workflowId,
|
||||
autosaved: true,
|
||||
|
|
@ -104,6 +130,7 @@ describe('WorkflowHistoryService', () => {
|
|||
authors: 'John Doe',
|
||||
connections: {},
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups: workflow.nodeGroups,
|
||||
versionId: workflow.versionId,
|
||||
workflowId,
|
||||
autosaved: false,
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ export class WorkflowHistoryService {
|
|||
workflowId: string,
|
||||
take: number,
|
||||
skip: number,
|
||||
): Promise<Array<Omit<WorkflowHistory, 'nodes' | 'connections'>>> {
|
||||
): Promise<Array<Omit<WorkflowHistory, 'nodes' | 'connections' | 'nodeGroups'>>> {
|
||||
const workflow = await this.workflowFinderService.findWorkflowForUser(workflowId, user, [
|
||||
'workflow:read',
|
||||
]);
|
||||
|
|
@ -108,6 +108,7 @@ export class WorkflowHistoryService {
|
|||
versionId: string;
|
||||
nodes: IWorkflowBase['nodes'];
|
||||
connections: IWorkflowBase['connections'];
|
||||
nodeGroups?: IWorkflowBase['nodeGroups'];
|
||||
},
|
||||
workflowId: string,
|
||||
autosaved = false,
|
||||
|
|
@ -130,6 +131,7 @@ export class WorkflowHistoryService {
|
|||
authors,
|
||||
connections: workflow.connections,
|
||||
nodes: workflow.nodes,
|
||||
nodeGroups: workflow.nodeGroups ?? [],
|
||||
versionId: workflow.versionId,
|
||||
workflowId,
|
||||
autosaved,
|
||||
|
|
|
|||
|
|
@ -342,13 +342,16 @@ export class WorkflowService {
|
|||
await this._detectConflicts(workflow, expectedChecksum);
|
||||
}
|
||||
|
||||
// Update the workflow's version when changing nodes or connections
|
||||
// Update the workflow's version when changing nodes, connections, or nodeGroups
|
||||
const hasNodesKey = 'nodes' in workflowUpdateData;
|
||||
const hasConnectionsKey = 'connections' in workflowUpdateData;
|
||||
const hasNodeGroupsKey = 'nodeGroups' in workflowUpdateData;
|
||||
const nodesChanged = hasNodesKey && !isEqual(workflowUpdateData.nodes, workflow.nodes);
|
||||
const connectionsChanged =
|
||||
hasConnectionsKey && !isEqual(workflowUpdateData.connections, workflow.connections);
|
||||
const saveNewVersion = nodesChanged || connectionsChanged;
|
||||
const nodeGroupsChanged =
|
||||
hasNodeGroupsKey && !isEqual(workflowUpdateData.nodeGroups, workflow.nodeGroups);
|
||||
const saveNewVersion = nodesChanged || connectionsChanged || nodeGroupsChanged;
|
||||
|
||||
if (saveNewVersion) {
|
||||
workflowUpdateData.versionId = uuid();
|
||||
|
|
@ -378,6 +381,10 @@ 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,
|
||||
});
|
||||
|
||||
// Strip redactionPolicy if instance lacks data-redaction license
|
||||
if (
|
||||
|
|
@ -440,6 +447,7 @@ export class WorkflowService {
|
|||
'name',
|
||||
'nodes',
|
||||
'connections',
|
||||
'nodeGroups',
|
||||
'meta',
|
||||
'settings',
|
||||
'staticData',
|
||||
|
|
|
|||
|
|
@ -765,6 +765,7 @@ describe('GET /workflows/:id/:versionId', () => {
|
|||
description: 'Version Description',
|
||||
nodes: versionData.nodes,
|
||||
connections: versionData.connections,
|
||||
nodeGroups: [],
|
||||
authors: 'Test User',
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
createdAt: expect.any(String),
|
||||
|
|
|
|||
|
|
@ -7,25 +7,29 @@ export async function createWorkflowHistoryItem(
|
|||
workflowId: string,
|
||||
data?: Partial<WorkflowHistory>,
|
||||
) {
|
||||
return await Container.get(WorkflowHistoryRepository).save({
|
||||
authors: 'John Smith',
|
||||
connections: {},
|
||||
nodes: [
|
||||
{
|
||||
id: 'uuid-1234',
|
||||
name: 'Start',
|
||||
parameters: {},
|
||||
position: [-20, 260],
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
typeVersion: 1,
|
||||
},
|
||||
],
|
||||
versionId: uuid(),
|
||||
workflowPublishHistory: [],
|
||||
autosaved: false,
|
||||
...(data ?? {}),
|
||||
workflowId,
|
||||
});
|
||||
const repo = Container.get(WorkflowHistoryRepository);
|
||||
return await repo.save(
|
||||
repo.create({
|
||||
authors: 'John Smith',
|
||||
connections: {},
|
||||
nodes: [
|
||||
{
|
||||
id: 'uuid-1234',
|
||||
name: 'Start',
|
||||
parameters: {},
|
||||
position: [-20, 260],
|
||||
type: 'n8n-nodes-base.manualTrigger',
|
||||
typeVersion: 1,
|
||||
},
|
||||
],
|
||||
versionId: uuid(),
|
||||
workflowPublishHistory: [],
|
||||
autosaved: false,
|
||||
...(data ?? {}),
|
||||
nodeGroups: data?.nodeGroups ?? [],
|
||||
workflowId,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
export async function createManyWorkflowHistoryItems(
|
||||
|
|
|
|||
|
|
@ -1,4 +1,10 @@
|
|||
import type { IWorkflowSettings, IConnections, INode, IPinData } from 'n8n-workflow';
|
||||
import type {
|
||||
IWorkflowSettings,
|
||||
IConnections,
|
||||
INode,
|
||||
IPinData,
|
||||
IWorkflowGroup,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import type { ITag } from './tags';
|
||||
|
||||
|
|
@ -36,6 +42,7 @@ export interface WorkflowDataUpdate {
|
|||
pinData?: IPinData;
|
||||
versionId?: string;
|
||||
meta?: WorkflowMetadata;
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
parentFolderId?: string;
|
||||
uiContext?: string;
|
||||
// checksum of workflow snapshot for conflict detection
|
||||
|
|
|
|||
|
|
@ -4800,6 +4800,58 @@ describe('useCanvasOperations', () => {
|
|||
expect((workflow.nodes![1].parameters.options as { path: string }).path).toBe('some-path');
|
||||
},
|
||||
);
|
||||
|
||||
it('should remap nodeGroups nodeIds when regenerating IDs', async () => {
|
||||
// This mock is needed for addImportedNodesToWorkflow to work
|
||||
vi.mocked(workflowDocumentStoreInstance.createWorkflowObject).mockReturnValue({
|
||||
nodes: {},
|
||||
connections: {},
|
||||
connectionsBySourceNode: {},
|
||||
renameNode: vi.fn(),
|
||||
} as unknown as Workflow);
|
||||
|
||||
const oldId1 = 'old-node-id-1';
|
||||
const oldId2 = 'old-node-id-2';
|
||||
|
||||
const workflowDataWithGroups = {
|
||||
nodes: [
|
||||
{
|
||||
id: oldId1,
|
||||
name: 'Node 1',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [0, 0] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
{
|
||||
id: oldId2,
|
||||
name: 'Node 2',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [200, 0] as [number, number],
|
||||
parameters: {},
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'group-1', name: 'My Group', nodeIds: [oldId1, oldId2] }],
|
||||
};
|
||||
|
||||
const canvasOperations = useCanvasOperations();
|
||||
const result = await canvasOperations.importWorkflowData(workflowDataWithGroups, 'paste', {
|
||||
regenerateIds: true,
|
||||
});
|
||||
|
||||
// Node IDs should have been reassigned
|
||||
const newId1 = result.nodes![0].id;
|
||||
const newId2 = result.nodes![1].id;
|
||||
expect(newId1).not.toBe(oldId1);
|
||||
expect(newId2).not.toBe(oldId2);
|
||||
|
||||
// nodeGroups should reference the new IDs
|
||||
expect(result.nodeGroups).toEqual([
|
||||
{ id: 'group-1', name: 'My Group', nodeIds: [newId1, newId2] },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('duplicateNodes', () => {
|
||||
|
|
|
|||
|
|
@ -2695,6 +2695,18 @@ export function useCanvasOperations() {
|
|||
});
|
||||
}
|
||||
|
||||
// Remap nodeGroup nodeIds to new node IDs
|
||||
if (regenerateIds && workflowData.nodeGroups?.length) {
|
||||
const oldToNewIdMap = new Map(
|
||||
Object.entries(nodeIdMap).map(([newId, previousId]) => [previousId, newId]),
|
||||
);
|
||||
|
||||
workflowData.nodeGroups = workflowData.nodeGroups.map((group) => ({
|
||||
...group,
|
||||
nodeIds: group.nodeIds.map((id) => oldToNewIdMap.get(id) ?? id),
|
||||
}));
|
||||
}
|
||||
|
||||
removeUnknownCredentials(workflowData);
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -447,6 +447,54 @@ describe('useWorkflowSaving', () => {
|
|||
expect(workflow.nodes![1].parameters.path).not.toBe(staticPath);
|
||||
expect(workflow.nodes![1].parameters.path).toBe(workflow.nodes![1].webhookId);
|
||||
});
|
||||
|
||||
it('should remap nodeGroups nodeIds when resetNodeIds is true', async () => {
|
||||
const oldId1 = 'old-id-1';
|
||||
const oldId2 = 'old-id-2';
|
||||
const workflow: WorkflowDataUpdate = {
|
||||
name: 'Grouped workflow',
|
||||
active: false,
|
||||
nodes: [
|
||||
{
|
||||
parameters: {},
|
||||
id: oldId1,
|
||||
name: 'Node 1',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
},
|
||||
{
|
||||
parameters: {},
|
||||
id: oldId2,
|
||||
name: 'Node 2',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [200, 0],
|
||||
},
|
||||
],
|
||||
connections: {},
|
||||
nodeGroups: [{ id: 'group-1', name: 'My Group', nodeIds: [oldId1, oldId2] }],
|
||||
};
|
||||
|
||||
const { saveAsNewWorkflow } = useWorkflowSaving({ router });
|
||||
|
||||
await saveAsNewWorkflow({
|
||||
name: workflow.name,
|
||||
resetNodeIds: true,
|
||||
data: workflow,
|
||||
});
|
||||
|
||||
// Node IDs should have been reassigned
|
||||
const newId1 = workflow.nodes![0].id;
|
||||
const newId2 = workflow.nodes![1].id;
|
||||
expect(newId1).not.toBe(oldId1);
|
||||
expect(newId2).not.toBe(oldId2);
|
||||
|
||||
// nodeGroups should reference the new IDs
|
||||
expect(workflow.nodeGroups).toEqual([
|
||||
{ id: 'group-1', name: 'My Group', nodeIds: [newId1, newId2] },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('saveCurrentWorkflow', () => {
|
||||
|
|
|
|||
|
|
@ -397,11 +397,20 @@ export function useWorkflowSaving({
|
|||
}
|
||||
|
||||
if (resetNodeIds) {
|
||||
const nodeIdMap = new Map<string, string>();
|
||||
workflowDataRequest.nodes = workflowDataRequest.nodes!.map((node) => {
|
||||
const oldId = node.id;
|
||||
nodeHelpers.assignNodeId(node);
|
||||
|
||||
if (oldId) nodeIdMap.set(oldId, node.id);
|
||||
return node;
|
||||
});
|
||||
|
||||
if (workflowDataRequest.nodeGroups?.length) {
|
||||
workflowDataRequest.nodeGroups = workflowDataRequest.nodeGroups.map((group) => ({
|
||||
...group,
|
||||
nodeIds: group.nodeIds.map((id) => nodeIdMap.get(id) ?? id),
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
if (resetWebhookUrls) {
|
||||
|
|
|
|||
|
|
@ -3025,6 +3025,12 @@ export interface IWaitingForExecutionSource {
|
|||
|
||||
export type WorkflowId = IWorkflowBase['id'];
|
||||
|
||||
export interface IWorkflowGroup {
|
||||
id: string;
|
||||
name: string;
|
||||
nodeIds: string[];
|
||||
}
|
||||
|
||||
export interface IWorkflowBase {
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
@ -3044,6 +3050,7 @@ export interface IWorkflowBase {
|
|||
activeVersion?: IWorkflowHistory | null;
|
||||
versionCounter?: number;
|
||||
meta?: WorkflowFEMeta;
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
}
|
||||
|
||||
interface IWorkflowHistory {
|
||||
|
|
@ -3051,6 +3058,7 @@ interface IWorkflowHistory {
|
|||
workflowId: string;
|
||||
nodes: INode[];
|
||||
connections: IConnections;
|
||||
nodeGroups: IWorkflowGroup[];
|
||||
authors: string;
|
||||
name: string | null;
|
||||
description: string | null;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,12 @@
|
|||
import jsSHA from 'jssha';
|
||||
|
||||
import type { IConnections, INode, IPinData, IWorkflowSettings } from './interfaces';
|
||||
import type {
|
||||
IConnections,
|
||||
INode,
|
||||
IPinData,
|
||||
IWorkflowGroup,
|
||||
IWorkflowSettings,
|
||||
} from './interfaces';
|
||||
import { isObject } from './utils';
|
||||
|
||||
/**
|
||||
|
|
@ -17,6 +23,7 @@ export interface WorkflowSnapshot {
|
|||
pinData?: IPinData;
|
||||
isArchived?: boolean;
|
||||
activeVersionId?: string | null;
|
||||
nodeGroups?: IWorkflowGroup[];
|
||||
}
|
||||
|
||||
export const WORKFLOW_CHECKSUM_FIELDS = [
|
||||
|
|
@ -29,6 +36,7 @@ export const WORKFLOW_CHECKSUM_FIELDS = [
|
|||
'pinData',
|
||||
'isArchived',
|
||||
'activeVersionId',
|
||||
'nodeGroups',
|
||||
] as const satisfies ReadonlyArray<keyof WorkflowSnapshot>;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -143,4 +143,38 @@ describe('calculateWorkflowChecksum', () => {
|
|||
|
||||
expect(checksum1).not.toBe(checksum2);
|
||||
});
|
||||
|
||||
it('should generate different checksums when nodeGroups change', async () => {
|
||||
const workflow1: WorkflowSnapshot = {
|
||||
...baseWorkflow,
|
||||
nodeGroups: [{ id: 'group1', name: 'Group A', nodeIds: ['node1'] }],
|
||||
};
|
||||
|
||||
const workflow2: WorkflowSnapshot = {
|
||||
...baseWorkflow,
|
||||
nodeGroups: [{ id: 'group1', name: 'Group B', nodeIds: ['node1'] }],
|
||||
};
|
||||
|
||||
const checksum1 = await calculateWorkflowChecksum(workflow1);
|
||||
const checksum2 = await calculateWorkflowChecksum(workflow2);
|
||||
|
||||
expect(checksum1).not.toBe(checksum2);
|
||||
});
|
||||
|
||||
it('should generate different checksums when nodeGroups are added', async () => {
|
||||
const workflow1: WorkflowSnapshot = {
|
||||
...baseWorkflow,
|
||||
nodeGroups: [],
|
||||
};
|
||||
|
||||
const workflow2: WorkflowSnapshot = {
|
||||
...baseWorkflow,
|
||||
nodeGroups: [{ id: 'group1', name: 'Group A', nodeIds: ['node1'] }],
|
||||
};
|
||||
|
||||
const checksum1 = await calculateWorkflowChecksum(workflow1);
|
||||
const checksum2 = await calculateWorkflowChecksum(workflow2);
|
||||
|
||||
expect(checksum1).not.toBe(checksum2);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user