mirror of
https://github.com/n8n-io/n8n.git
synced 2026-06-03 18:27:09 +02:00
fix(core): The n8n import:workflow --activeState=fromJson cli can fail for subworkflow dependencies (#31377)
This commit is contained in:
Konstantin Tieber
GitHub
parent
485c153ad6
commit
e785e4e7f3
|
|
@ -131,6 +131,12 @@ export class ImportWorkflowsCommand extends BaseCommand<z.infer<typeof flagsSche
|
|||
|
||||
const project = await this.getProject(flags.userId, flags.projectId);
|
||||
|
||||
const ownerUser = await Container.get(UserRepository).findOneByOrFail({
|
||||
role: { slug: GLOBAL_OWNER_ROLE.slug },
|
||||
});
|
||||
// This userId will be used as the actor for publish/unpublish workflow actions
|
||||
const userId = flags.userId ?? ownerUser.id;
|
||||
|
||||
const workflows = await this.readWorkflows(flags.input, flags.separate);
|
||||
|
||||
const result = await this.checkRelations(workflows, flags.projectId, flags.userId);
|
||||
|
|
@ -141,7 +147,7 @@ export class ImportWorkflowsCommand extends BaseCommand<z.infer<typeof flagsSche
|
|||
|
||||
this.logger.info(`Importing ${workflows.length} workflows...`);
|
||||
|
||||
await Container.get(ImportService).importWorkflows(workflows, project.id, {
|
||||
await Container.get(ImportService).importWorkflows(workflows, project.id, userId, {
|
||||
activeState: flags.activeState,
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,18 +1,13 @@
|
|||
import { safeJoinPath, type Logger } from '@n8n/backend-common';
|
||||
import type {
|
||||
CredentialsRepository,
|
||||
TagRepository,
|
||||
WorkflowPublishHistoryRepository,
|
||||
WorkflowRepository,
|
||||
} from '@n8n/db';
|
||||
import type { CredentialsRepository, TagRepository, UserRepository } from '@n8n/db';
|
||||
import { type DataSource, type EntityManager } from '@n8n/typeorm';
|
||||
import { readdir, readFile } from 'fs/promises';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { Cipher } from 'n8n-core';
|
||||
|
||||
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import type { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
|
||||
import type { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
import type { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { ImportService } from '../import.service';
|
||||
|
||||
|
|
@ -35,10 +30,6 @@ jest.mock('@n8n/db', () => ({
|
|||
WithTimestampsAndStringId: class {},
|
||||
}));
|
||||
|
||||
jest.mock('@/active-workflow-manager', () => ({
|
||||
ActiveWorkflowManager: mock<ActiveWorkflowManager>(),
|
||||
}));
|
||||
|
||||
describe('ImportService', () => {
|
||||
let importService: ImportService;
|
||||
let mockLogger: Logger;
|
||||
|
|
@ -47,11 +38,10 @@ describe('ImportService', () => {
|
|||
let mockTagRepository: TagRepository;
|
||||
let mockEntityManager: EntityManager;
|
||||
let mockCipher: Cipher;
|
||||
let mockActiveWorkflowManager: ActiveWorkflowManager;
|
||||
let mockWorkflowIndexService: WorkflowIndexService;
|
||||
let mockDataTableDDLService: DataTableDDLService;
|
||||
let mockWorkflowRepository: WorkflowRepository;
|
||||
let mockWorkflowPublishHistoryRepository: WorkflowPublishHistoryRepository;
|
||||
let mockUserRepository: UserRepository;
|
||||
let mockWorkflowService: WorkflowService;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
|
@ -62,11 +52,10 @@ describe('ImportService', () => {
|
|||
mockTagRepository = mock<TagRepository>();
|
||||
mockEntityManager = mock<EntityManager>();
|
||||
mockCipher = mock<Cipher>();
|
||||
mockActiveWorkflowManager = mock<ActiveWorkflowManager>();
|
||||
mockWorkflowIndexService = mock<WorkflowIndexService>();
|
||||
mockDataTableDDLService = mock<DataTableDDLService>();
|
||||
mockWorkflowRepository = mock<WorkflowRepository>();
|
||||
mockWorkflowPublishHistoryRepository = mock<WorkflowPublishHistoryRepository>();
|
||||
mockUserRepository = mock<UserRepository>();
|
||||
mockWorkflowService = mock<WorkflowService>();
|
||||
|
||||
// Set up cipher mock
|
||||
mockCipher.decryptV2 = jest.fn(async (data: string) =>
|
||||
|
|
@ -116,11 +105,10 @@ describe('ImportService', () => {
|
|||
mockTagRepository,
|
||||
mockDataSource,
|
||||
mockCipher,
|
||||
mockActiveWorkflowManager,
|
||||
mockWorkflowIndexService,
|
||||
mockDataTableDDLService,
|
||||
mockWorkflowRepository,
|
||||
mockWorkflowPublishHistoryRepository,
|
||||
mockUserRepository,
|
||||
mockWorkflowService,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -1252,6 +1240,234 @@ describe('ImportService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('extractSubworkflowId', () => {
|
||||
it('should extract workflow ID from legacy string format', () => {
|
||||
const node = {
|
||||
parameters: { workflowId: 'abc123' },
|
||||
};
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
expect(importService.extractSubworkflowId(node)).toBe('abc123');
|
||||
});
|
||||
|
||||
it('should extract workflow ID from resource-locator object format', () => {
|
||||
const node = {
|
||||
parameters: {
|
||||
workflowId: { __rl: true, value: 'LCEM9GnTcIVSy1D8', mode: 'list' },
|
||||
},
|
||||
};
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
expect(importService.extractSubworkflowId(node)).toBe('LCEM9GnTcIVSy1D8');
|
||||
});
|
||||
|
||||
it('should return undefined when workflowId is missing', () => {
|
||||
const node = {
|
||||
parameters: {},
|
||||
};
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
expect(importService.extractSubworkflowId(node)).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sortWorkflowsForActivation', () => {
|
||||
function makeNode(id: string, type: string, parameters: Record<string, unknown> = {}) {
|
||||
return { id, type, parameters, disabled: false } as any;
|
||||
}
|
||||
|
||||
function makeExecuteWorkflowNode(id: string, calleeId: string) {
|
||||
return makeNode(id, 'n8n-nodes-base.executeWorkflow', {
|
||||
workflowId: calleeId,
|
||||
});
|
||||
}
|
||||
|
||||
function makeWorkflow(id: string, nodes: any[] = []) {
|
||||
return { id, nodes } as any;
|
||||
}
|
||||
|
||||
function makeToActivate(ids: string[]) {
|
||||
return ids.map((id) => ({ workflowId: id, versionId: `v-${id}` }));
|
||||
}
|
||||
|
||||
it('should return single workflow unchanged', () => {
|
||||
const workflows = [makeWorkflow('A')];
|
||||
const toActivate = makeToActivate(['A']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result.map((w) => w.workflowId)).toEqual(['A']);
|
||||
});
|
||||
|
||||
it('should activate callee (B) before caller (A) — simple A→B case', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'B')]),
|
||||
makeWorkflow('B'),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result.map((w) => w.workflowId)).toEqual(['B', 'A']);
|
||||
});
|
||||
|
||||
it('should activate C → B → A for a three-level chain', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'B')]),
|
||||
makeWorkflow('B', [makeExecuteWorkflowNode('n2', 'C')]),
|
||||
makeWorkflow('C'),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B', 'C']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result.map((w) => w.workflowId)).toEqual(['C', 'B', 'A']);
|
||||
});
|
||||
|
||||
it('should activate both B and C before A when A calls both', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'B'), makeExecuteWorkflowNode('n2', 'C')]),
|
||||
makeWorkflow('B'),
|
||||
makeWorkflow('C'),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B', 'C']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
const ids = result.map((w) => w.workflowId);
|
||||
expect(ids.indexOf('B')).toBeLessThan(ids.indexOf('A'));
|
||||
expect(ids.indexOf('C')).toBeLessThan(ids.indexOf('A'));
|
||||
expect(ids).toHaveLength(3);
|
||||
});
|
||||
|
||||
it('should ignore referenced workflows not present in the activation batch', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'EXTERNAL')]),
|
||||
makeWorkflow('B'),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.map((w) => w.workflowId)).toContain('A');
|
||||
expect(result.map((w) => w.workflowId)).toContain('B');
|
||||
});
|
||||
|
||||
it('should skip disabled executeWorkflow nodes when building the dependency graph', () => {
|
||||
const disabledNode = { ...makeExecuteWorkflowNode('n1', 'B'), disabled: true };
|
||||
const workflows = [makeWorkflow('A', [disabledNode]), makeWorkflow('B')];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
// A has no active dependencies on B, so order can be anything — just verify both present
|
||||
expect(result).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('should handle resource-locator workflowId format in nodes', () => {
|
||||
const node = makeNode('n1', 'n8n-nodes-base.executeWorkflow', {
|
||||
workflowId: { __rl: true, value: 'B', mode: 'list' },
|
||||
});
|
||||
const workflows = [makeWorkflow('A', [node]), makeWorkflow('B')];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result.map((w) => w.workflowId)).toEqual(['B', 'A']);
|
||||
});
|
||||
|
||||
it('should return original order (fast path) when no workflow references another batch workflow', () => {
|
||||
// Both workflows have executeWorkflow nodes, but they point to external IDs not in batch
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'EXTERNAL_1')]),
|
||||
makeWorkflow('B', [makeExecuteWorkflowNode('n2', 'EXTERNAL_2')]),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
// Same reference → fast path returned the original array unchanged
|
||||
expect(result).toBe(toActivate);
|
||||
});
|
||||
|
||||
it('should return original order (fast path) when no workflows have executeWorkflow nodes', () => {
|
||||
const workflows = [makeWorkflow('A'), makeWorkflow('B'), makeWorkflow('C')];
|
||||
const toActivate = makeToActivate(['A', 'B', 'C']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result).toBe(toActivate);
|
||||
});
|
||||
|
||||
it('should append mutually-cyclic workflows (A↔B) and log a warning', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'B')]),
|
||||
makeWorkflow('B', [makeExecuteWorkflowNode('n2', 'A')]),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.map((w) => w.workflowId)).toContain('A');
|
||||
expect(result.map((w) => w.workflowId)).toContain('B');
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('circular'));
|
||||
});
|
||||
|
||||
it('should append all workflows in a three-way cycle (A→B→C→A) and log a warning', () => {
|
||||
const workflows = [
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n1', 'B')]),
|
||||
makeWorkflow('B', [makeExecuteWorkflowNode('n2', 'C')]),
|
||||
makeWorkflow('C', [makeExecuteWorkflowNode('n3', 'A')]),
|
||||
];
|
||||
const toActivate = makeToActivate(['A', 'B', 'C']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result).toHaveLength(3);
|
||||
expect(result.map((w) => w.workflowId)).toContain('A');
|
||||
expect(result.map((w) => w.workflowId)).toContain('B');
|
||||
expect(result.map((w) => w.workflowId)).toContain('C');
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('circular'));
|
||||
});
|
||||
|
||||
it('should sort non-cyclic workflows first and append cyclic workflows at the end', () => {
|
||||
// D→E: normal dependency, no cycle
|
||||
// A↔B: mutual cycle
|
||||
const workflows = [
|
||||
makeWorkflow('D', [makeExecuteWorkflowNode('n1', 'E')]),
|
||||
makeWorkflow('E'),
|
||||
makeWorkflow('A', [makeExecuteWorkflowNode('n2', 'B')]),
|
||||
makeWorkflow('B', [makeExecuteWorkflowNode('n3', 'A')]),
|
||||
];
|
||||
const toActivate = makeToActivate(['D', 'E', 'A', 'B']);
|
||||
|
||||
// @ts-expect-error accessing private method for testing
|
||||
const result = importService.sortWorkflowsForActivation(workflows, toActivate);
|
||||
|
||||
expect(result).toHaveLength(4);
|
||||
const ids = result.map((w) => w.workflowId);
|
||||
// E must come before D (normal dependency order)
|
||||
expect(ids.indexOf('E')).toBeLessThan(ids.indexOf('D'));
|
||||
// A and B are cyclic — they appear after the sorted pair
|
||||
expect(ids.indexOf('A')).toBeGreaterThan(ids.indexOf('D'));
|
||||
expect(ids.indexOf('B')).toBeGreaterThan(ids.indexOf('D'));
|
||||
expect(mockLogger.warn).toHaveBeenCalledWith(expect.stringContaining('circular'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('advanceIdentitySequences', () => {
|
||||
it('should run setval for each identity column on Postgres', async () => {
|
||||
// @ts-expect-error overriding for the test
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { Logger, safeJoinPath } from '@n8n/backend-common';
|
||||
import type { TagEntity, ICredentialsDb } from '@n8n/db';
|
||||
import type { TagEntity, ICredentialsDb, User } from '@n8n/db';
|
||||
import {
|
||||
Project,
|
||||
WorkflowEntity,
|
||||
|
|
@ -7,15 +7,15 @@ import {
|
|||
WorkflowTagMapping,
|
||||
CredentialsRepository,
|
||||
TagRepository,
|
||||
UserRepository,
|
||||
WorkflowHistory,
|
||||
WorkflowPublishHistory,
|
||||
WorkflowPublishHistoryRepository,
|
||||
WorkflowRepository,
|
||||
} from '@n8n/db';
|
||||
import { Service } from '@n8n/di';
|
||||
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
|
||||
import { DataSource, EntityManager, In, type EntityMetadata } from '@n8n/typeorm';
|
||||
import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity';
|
||||
import { Service } from '@n8n/di';
|
||||
import { readdir, readFile } from 'fs/promises';
|
||||
import { Cipher } from 'n8n-core';
|
||||
import {
|
||||
ensureError,
|
||||
type INode,
|
||||
|
|
@ -23,23 +23,21 @@ import {
|
|||
type IWorkflowBase,
|
||||
} from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
import { readdir, readFile } from 'fs/promises';
|
||||
|
||||
import { replaceInvalidCredentials, validateWorkflowStructure } from '@/workflow-helpers';
|
||||
import { validateDbTypeForImportEntities } from '@/utils/validate-database-type';
|
||||
import { Cipher } from 'n8n-core';
|
||||
import { decompressFolder } from '@/utils/compression.util';
|
||||
import { z } from 'zod';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
|
||||
import type { IWorkflowWithVersionMetadata } from '@/interfaces';
|
||||
import { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
import { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
|
||||
import type { DataTableColumn } from '@/modules/data-table/data-table-column.entity';
|
||||
import { DataTableDDLService } from '@/modules/data-table/data-table-ddl.service';
|
||||
import {
|
||||
normalizeUserRowValueForDatabase,
|
||||
quoteIdentifier,
|
||||
toTableName,
|
||||
} from '@/modules/data-table/utils/sql-utils';
|
||||
import { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
import { decompressFolder } from '@/utils/compression.util';
|
||||
import { validateDbTypeForImportEntities } from '@/utils/validate-database-type';
|
||||
import { replaceInvalidCredentials, validateWorkflowStructure } from '@/workflow-helpers';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
const DATA_TABLE_ROWS_FILE_PREFIX = 'data_table_user_';
|
||||
|
||||
|
|
@ -72,11 +70,10 @@ export class ImportService {
|
|||
private readonly tagRepository: TagRepository,
|
||||
private readonly dataSource: DataSource,
|
||||
private readonly cipher: Cipher,
|
||||
private readonly activeWorkflowManager: ActiveWorkflowManager,
|
||||
private readonly workflowIndexService: WorkflowIndexService,
|
||||
private readonly dataTableDDLService: DataTableDDLService,
|
||||
private readonly workflowRepository: WorkflowRepository,
|
||||
private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository,
|
||||
private readonly userRepository: UserRepository,
|
||||
private readonly workflowService: WorkflowService,
|
||||
) {}
|
||||
|
||||
async initRecords() {
|
||||
|
|
@ -87,10 +84,16 @@ export class ImportService {
|
|||
async importWorkflows(
|
||||
workflows: IWorkflowWithVersionMetadata[],
|
||||
projectId: string,
|
||||
{ activeState = 'false' }: { activeState?: 'false' | 'fromJson' } = {},
|
||||
userId: string,
|
||||
{ activeState = 'false' }: { activeState?: 'false' | 'fromJson' },
|
||||
) {
|
||||
await this.initRecords();
|
||||
|
||||
const user = await this.userRepository.findOneOrFail({
|
||||
where: { id: userId },
|
||||
relations: ['role'],
|
||||
});
|
||||
|
||||
const { manager: dbManager } = this.credentialsRepository;
|
||||
|
||||
// Check existence and active status of all workflows
|
||||
|
|
@ -124,18 +127,16 @@ export class ImportService {
|
|||
if (hasInvalidCreds) await this.replaceInvalidCreds(workflow, projectId);
|
||||
validateWorkflowStructure(workflow);
|
||||
|
||||
// Remove workflows from ActiveWorkflowManager BEFORE transaction to prevent orphaned trigger listeners
|
||||
// Only remove if the workflow already exists in the database and is active
|
||||
// Deactivate BEFORE the transaction to prevent orphaned trigger listeners.
|
||||
// Only applies to workflows that are currently active in the database.
|
||||
if (workflow.id && activeVersionIdByWorkflow.has(workflow.id)) {
|
||||
await this.activeWorkflowManager.remove(workflow.id);
|
||||
await this.workflowService.deactivateWorkflow(user, workflow.id, { source: 'import' });
|
||||
}
|
||||
}
|
||||
|
||||
const insertedWorkflows: IWorkflowWithVersionMetadata[] = [];
|
||||
const workflowsToActivate: Array<{ workflowId: string; versionId: string }> = [];
|
||||
await dbManager.transaction(async (tx) => {
|
||||
const workflowsNeedingPublishHistory: Array<{ workflowId: string; versionId: string }> = [];
|
||||
|
||||
// Upsert all workflows
|
||||
for (const workflow of workflows) {
|
||||
// Always generate a new versionId on import to ensure proper history ordering
|
||||
|
|
@ -163,11 +164,6 @@ export class ImportService {
|
|||
const workflowId = upsertResult.identifiers.at(0)?.id as string;
|
||||
insertedWorkflows.push({ ...workflow, id: workflowId }); // Collect inserted workflow with correct ID, for indexing later.
|
||||
|
||||
// Only add publish history if workflow was previously active
|
||||
if (oldActiveVersionId) {
|
||||
workflowsNeedingPublishHistory.push({ workflowId, versionId: oldActiveVersionId });
|
||||
}
|
||||
|
||||
if (shouldActivate) {
|
||||
workflowsToActivate.push({ workflowId, versionId: versionIdToActivate });
|
||||
}
|
||||
|
|
@ -210,20 +206,14 @@ export class ImportService {
|
|||
description: versionMetadata?.description ?? null,
|
||||
});
|
||||
}
|
||||
|
||||
// Add publish history records for workflows that were deactivated
|
||||
for (const { workflowId, versionId } of workflowsNeedingPublishHistory) {
|
||||
await tx.insert(WorkflowPublishHistory, {
|
||||
workflowId,
|
||||
versionId,
|
||||
event: 'deactivated',
|
||||
userId: null,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
for (const { workflowId, versionId } of workflowsToActivate) {
|
||||
await this.activateWorkflow(workflowId, versionId);
|
||||
const orderedWorkflowsToActivate = this.sortWorkflowsForActivation(
|
||||
insertedWorkflows,
|
||||
workflowsToActivate,
|
||||
);
|
||||
for (const { workflowId, versionId } of orderedWorkflowsToActivate) {
|
||||
await this.activateWorkflow(workflowId, versionId, user);
|
||||
}
|
||||
|
||||
// Directly update the index for the important workflows, since they don't generate
|
||||
|
|
@ -233,29 +223,101 @@ export class ImportService {
|
|||
}
|
||||
}
|
||||
|
||||
private async activateWorkflow(workflowId: string, versionIdToActivate: string): Promise<void> {
|
||||
let didActivate = false;
|
||||
try {
|
||||
await this.workflowRepository.update(
|
||||
{ id: workflowId },
|
||||
{ activeVersionId: versionIdToActivate },
|
||||
);
|
||||
await this.workflowRepository.updateActiveState(workflowId, true);
|
||||
await this.activeWorkflowManager.add(workflowId, 'activate');
|
||||
didActivate = true;
|
||||
} catch (e) {
|
||||
const error = ensureError(e);
|
||||
this.logger.error(`Failed to activate workflow ${workflowId}`, { error });
|
||||
} finally {
|
||||
if (didActivate) {
|
||||
await this.workflowPublishHistoryRepository.addRecord({
|
||||
workflowId,
|
||||
versionId: versionIdToActivate,
|
||||
event: 'activated',
|
||||
userId: null,
|
||||
});
|
||||
/**
|
||||
* Sorts workflows to activate in dependency order so that subworkflows are activated
|
||||
* before the workflows that call them. Uses Kahn's topological sort algorithm.
|
||||
*/
|
||||
private sortWorkflowsForActivation(
|
||||
allImportedWorkflows: IWorkflowWithVersionMetadata[],
|
||||
toActivate: Array<{ workflowId: string; versionId: string }>,
|
||||
): Array<{ workflowId: string; versionId: string }> {
|
||||
if (toActivate.length <= 1) return toActivate;
|
||||
|
||||
const nodesByWorkflowId = new Map(allImportedWorkflows.map((w) => [w.id, w.nodes]));
|
||||
const activateIds = new Set(toActivate.map((w) => w.workflowId));
|
||||
|
||||
// Fast path: skip the full graph build if no workflow in the batch references
|
||||
// another batch workflow via an active executeWorkflow node.
|
||||
const hasCrossReference = toActivate.some(({ workflowId }) =>
|
||||
(nodesByWorkflowId.get(workflowId) ?? []).some(
|
||||
(node) =>
|
||||
!node.disabled &&
|
||||
node.type === 'n8n-nodes-base.executeWorkflow' &&
|
||||
activateIds.has(this.extractSubworkflowId(node) ?? ''),
|
||||
),
|
||||
);
|
||||
if (!hasCrossReference) return toActivate;
|
||||
|
||||
const toActivateByWorkflowId = new Map(toActivate.map((w) => [w.workflowId, w]));
|
||||
// callee id → set of caller ids that depend on it being activated first
|
||||
const dependents = new Map<string, Set<string>>(
|
||||
toActivate.map(({ workflowId }) => [workflowId, new Set()]),
|
||||
);
|
||||
// caller id → how many of its subworkflow dependencies in this batch are not yet activated
|
||||
const unresolvedDepsCount = new Map<string, number>(
|
||||
toActivate.map(({ workflowId }) => [workflowId, 0]),
|
||||
);
|
||||
|
||||
for (const { workflowId } of toActivate) {
|
||||
for (const node of nodesByWorkflowId.get(workflowId) ?? []) {
|
||||
if (node.disabled || node.type !== 'n8n-nodes-base.executeWorkflow') continue;
|
||||
const calleeId = this.extractSubworkflowId(node);
|
||||
if (!calleeId || !activateIds.has(calleeId) || calleeId === workflowId) continue;
|
||||
dependents.get(calleeId)!.add(workflowId);
|
||||
unresolvedDepsCount.set(workflowId, unresolvedDepsCount.get(workflowId)! + 1);
|
||||
}
|
||||
}
|
||||
|
||||
const queue = toActivate.filter((w) => unresolvedDepsCount.get(w.workflowId) === 0);
|
||||
const result: Array<{ workflowId: string; versionId: string }> = [];
|
||||
|
||||
while (queue.length > 0) {
|
||||
const item = queue.shift()!;
|
||||
result.push(item);
|
||||
for (const callerId of dependents.get(item.workflowId)!) {
|
||||
const remaining = unresolvedDepsCount.get(callerId)! - 1;
|
||||
unresolvedDepsCount.set(callerId, remaining);
|
||||
if (remaining === 0) queue.push(toActivateByWorkflowId.get(callerId)!);
|
||||
}
|
||||
}
|
||||
|
||||
if (result.length < toActivate.length) {
|
||||
// Any workflow still with unresolvedDepsCount > 0 was never enqueued by the
|
||||
// sort — it's part of a cycle (its dependency also waits on it). Append these
|
||||
// so they still get activated rather than being silently dropped.
|
||||
const cycleWorkflows = toActivate.filter(
|
||||
(w) => Number(unresolvedDepsCount.get(w.workflowId)) > 0,
|
||||
);
|
||||
this.logger.warn(
|
||||
`Detected circular subworkflow references among workflows: [${cycleWorkflows.map((w) => w.workflowId).join(', ')}]. Activating them in original order.`,
|
||||
);
|
||||
result.push(...cycleWorkflows);
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private extractSubworkflowId(node: INode): string | undefined {
|
||||
const source = node.parameters?.['source'];
|
||||
if (source === 'parameter' || source === 'localFile' || source === 'url') return undefined;
|
||||
const wfId = node.parameters?.['workflowId'];
|
||||
const rawId = typeof wfId === 'string' ? wfId : (wfId as { value?: unknown } | null)?.value;
|
||||
return typeof rawId === 'string' && !rawId.startsWith('=') ? rawId : undefined;
|
||||
}
|
||||
|
||||
private async activateWorkflow(
|
||||
workflowId: string,
|
||||
versionIdToActivate: string,
|
||||
user: User,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await this.workflowService.activateWorkflow(user, workflowId, {
|
||||
versionId: versionIdToActivate,
|
||||
source: 'import',
|
||||
});
|
||||
} catch (e) {
|
||||
this.logger.error(`Failed to activate workflow ${workflowId}`, { error: ensureError(e) });
|
||||
}
|
||||
}
|
||||
|
||||
async replaceInvalidCreds(workflow: IWorkflowBase, projectId: string) {
|
||||
|
|
|
|||
|
|
@ -8,12 +8,15 @@ import {
|
|||
import { GlobalConfig } from '@n8n/config';
|
||||
import { WorkflowPublishHistoryRepository, WorkflowHistoryRepository } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import type { INodeType } from 'n8n-workflow';
|
||||
import { nanoid } from 'nanoid';
|
||||
|
||||
import '@/zod-alias-support';
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import { ImportWorkflowsCommand } from '@/commands/import/workflow';
|
||||
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
|
||||
import { NodeTypes } from '@/node-types';
|
||||
import { WorkflowService } from '@/workflows/workflow.service';
|
||||
import { setupTestCommand } from '@test-integration/utils/test-command';
|
||||
|
||||
import { createMember, createOwner } from '../shared/db/users';
|
||||
|
|
@ -21,6 +24,7 @@ import { createMember, createOwner } from '../shared/db/users';
|
|||
mockInstance(LoadNodesAndCredentials);
|
||||
mockInstance(ActiveWorkflowManager);
|
||||
mockInstance(WorkflowPublishHistoryRepository);
|
||||
const mockNodeTypes = mockInstance(NodeTypes);
|
||||
|
||||
const command = setupTestCommand(ImportWorkflowsCommand);
|
||||
|
||||
|
|
@ -385,6 +389,27 @@ describe('--activeState flag', () => {
|
|||
globalConfig.executions.mode = originalMode;
|
||||
});
|
||||
|
||||
// TODO: fix this workaround being needed for these tests to run.
|
||||
// It was introduced after refactoring the ImportService used by the import command
|
||||
// from using the ActiveWorkflowManager to activate/deactivate workflows to the WorkflowService.
|
||||
beforeEach(() => {
|
||||
// Bypass webhook conflict detection to avoid infrastructure dependencies
|
||||
// (getWorkflowExecutionData → VariablesService.getAllCached → CacheService/Redis)
|
||||
jest
|
||||
.spyOn(Container.get(WorkflowService) as any, '_findConflictingWebhooks')
|
||||
.mockResolvedValue([]);
|
||||
|
||||
mockNodeTypes.getByNameAndVersion.mockImplementation((nodeType) => {
|
||||
if (nodeType === 'n8n-nodes-base.webhook') {
|
||||
return {
|
||||
description: { webhooks: undefined, properties: [] },
|
||||
webhook: jest.fn(),
|
||||
} as unknown as INodeType;
|
||||
}
|
||||
return { description: { properties: [] } } as unknown as INodeType;
|
||||
});
|
||||
});
|
||||
|
||||
describe('fromJson', () => {
|
||||
it('should activate a workflow that is marked as active in the imported json', async () => {
|
||||
await createOwner();
|
||||
|
|
@ -410,7 +435,7 @@ describe('--activeState flag', () => {
|
|||
});
|
||||
|
||||
it('should deactivate the previously active version and activate the new version when importing a workflow json with an ID that already exists for an active workflow', async () => {
|
||||
await createOwner();
|
||||
const owner = await createOwner();
|
||||
|
||||
await command.run([
|
||||
'--input=./test/integration/commands/import-workflows/combined-with-update/original.json',
|
||||
|
|
@ -441,12 +466,12 @@ describe('--activeState flag', () => {
|
|||
expect(activeWorkflowManager.add).toHaveBeenLastCalledWith('998', 'activate');
|
||||
|
||||
const publishHistoryRepo = Container.get(WorkflowPublishHistoryRepository);
|
||||
expect(publishHistoryRepo.addRecord).toHaveBeenCalledTimes(2);
|
||||
expect(publishHistoryRepo.addRecord).toHaveBeenCalledTimes(3);
|
||||
expect(publishHistoryRepo.addRecord).toHaveBeenLastCalledWith({
|
||||
workflowId: '998',
|
||||
versionId: second.versionId,
|
||||
event: 'activated',
|
||||
userId: null,
|
||||
userId: owner.id,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -16,16 +16,16 @@ import {
|
|||
SharedWorkflowRepository,
|
||||
WorkflowRepository,
|
||||
WorkflowHistoryRepository,
|
||||
WorkflowPublishHistoryRepository,
|
||||
UserRepository,
|
||||
} from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { mock } from 'jest-mock-extended';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
import type { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
import type { WorkflowIndexService } from '@/modules/workflow-index/workflow-index.service';
|
||||
import { ImportService } from '@/services/import.service';
|
||||
import type { WorkflowService } from '@/workflows/workflow.service';
|
||||
|
||||
import { createMember, createOwner } from './shared/db/users';
|
||||
|
||||
|
|
@ -34,13 +34,12 @@ describe('ImportService', () => {
|
|||
let tagRepository: TagRepository;
|
||||
let owner: User;
|
||||
let ownerPersonalProject: Project;
|
||||
let mockActiveWorkflowManager: ActiveWorkflowManager;
|
||||
let mockWorkflowService: jest.Mocked<WorkflowService>;
|
||||
let mockWorkflowIndexService: WorkflowIndexService;
|
||||
|
||||
let workflowRepository: WorkflowRepository;
|
||||
let sharedWorkflowRepository: SharedWorkflowRepository;
|
||||
let workflowHistoryRepository: WorkflowHistoryRepository;
|
||||
let workflowPublishHistoryRepository: WorkflowPublishHistoryRepository;
|
||||
|
||||
beforeAll(async () => {
|
||||
await testDb.init();
|
||||
|
|
@ -48,7 +47,6 @@ describe('ImportService', () => {
|
|||
workflowRepository = Container.get(WorkflowRepository);
|
||||
sharedWorkflowRepository = Container.get(SharedWorkflowRepository);
|
||||
workflowHistoryRepository = Container.get(WorkflowHistoryRepository);
|
||||
workflowPublishHistoryRepository = Container.get(WorkflowPublishHistoryRepository);
|
||||
|
||||
owner = await createOwner();
|
||||
ownerPersonalProject = await getPersonalProject(owner);
|
||||
|
|
@ -56,9 +54,9 @@ describe('ImportService', () => {
|
|||
tagRepository = Container.get(TagRepository);
|
||||
|
||||
const credentialsRepository = Container.get(CredentialsRepository);
|
||||
const userRepository = Container.get(UserRepository);
|
||||
|
||||
mockActiveWorkflowManager = mock<ActiveWorkflowManager>();
|
||||
|
||||
mockWorkflowService = mock<WorkflowService>();
|
||||
mockWorkflowIndexService = mock<WorkflowIndexService>();
|
||||
|
||||
importService = new ImportService(
|
||||
|
|
@ -67,11 +65,10 @@ describe('ImportService', () => {
|
|||
tagRepository,
|
||||
mock(),
|
||||
mock(),
|
||||
mockActiveWorkflowManager,
|
||||
mockWorkflowIndexService,
|
||||
mock(),
|
||||
workflowRepository,
|
||||
workflowPublishHistoryRepository,
|
||||
userRepository,
|
||||
mockWorkflowService,
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -93,7 +90,7 @@ describe('ImportService', () => {
|
|||
test('should import credless and tagless workflow', async () => {
|
||||
const workflowToImport = await createWorkflow();
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await getWorkflowById(workflowToImport.id);
|
||||
|
||||
|
|
@ -106,7 +103,7 @@ describe('ImportService', () => {
|
|||
test('should make user owner of imported workflow', async () => {
|
||||
const workflowToImport = newWorkflow();
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbSharing = await sharedWorkflowRepository.findOneOrFail({
|
||||
where: {
|
||||
|
|
@ -124,7 +121,7 @@ describe('ImportService', () => {
|
|||
const memberPersonalProject = await getPersonalProject(member);
|
||||
const workflowToImport = await createWorkflow(undefined, owner);
|
||||
|
||||
await importService.importWorkflows([workflowToImport], memberPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], memberPersonalProject.id, owner.id, {});
|
||||
|
||||
const sharings = await getAllSharedWorkflows();
|
||||
|
||||
|
|
@ -140,7 +137,7 @@ describe('ImportService', () => {
|
|||
test('should deactivate imported workflow if active', async () => {
|
||||
const workflowToImport = await createActiveWorkflow();
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await getWorkflowById(workflowToImport.id);
|
||||
|
||||
|
|
@ -169,7 +166,7 @@ describe('ImportService', () => {
|
|||
|
||||
const workflowToImport = await createWorkflow({ nodes });
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await getWorkflowById(workflowToImport.id);
|
||||
|
||||
|
|
@ -189,7 +186,7 @@ describe('ImportService', () => {
|
|||
|
||||
const workflowToImport = await createWorkflow({ tags: [tag] });
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await workflowRepository.findOneOrFail({
|
||||
where: { id: workflowToImport.id },
|
||||
|
|
@ -210,7 +207,7 @@ describe('ImportService', () => {
|
|||
|
||||
const workflowToImport = await createWorkflow({ tags: [tag] });
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await workflowRepository.findOneOrFail({
|
||||
where: { id: workflowToImport.id },
|
||||
|
|
@ -229,7 +226,7 @@ describe('ImportService', () => {
|
|||
|
||||
const workflowToImport = await createWorkflow({ tags: [tag] });
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const dbWorkflow = await workflowRepository.findOneOrFail({
|
||||
where: { id: workflowToImport.id },
|
||||
|
|
@ -245,17 +242,21 @@ describe('ImportService', () => {
|
|||
expect(dbTag.name).toBe(tag.name); // tag created
|
||||
});
|
||||
|
||||
test('should remove workflow from ActiveWorkflowManager when workflow has ID', async () => {
|
||||
test('should call WorkflowService.deactivateWorkflow when workflow has ID and is active', async () => {
|
||||
const workflowWithId = await createActiveWorkflow();
|
||||
await importService.importWorkflows([workflowWithId], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowWithId], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
expect(mockActiveWorkflowManager.remove).toHaveBeenCalledWith(workflowWithId.id);
|
||||
expect(mockWorkflowService.deactivateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
workflowWithId.id,
|
||||
{ source: 'import' },
|
||||
);
|
||||
});
|
||||
|
||||
test('should always create a record in workflow history', async () => {
|
||||
const workflowToImport = newWorkflow();
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const workflowHistoryRecords = await workflowHistoryRepository.find({
|
||||
where: {
|
||||
|
|
@ -277,7 +278,7 @@ describe('ImportService', () => {
|
|||
description: 'Historical workflow description',
|
||||
};
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const workflowHistoryRecords = await workflowHistoryRepository.find({
|
||||
where: {
|
||||
|
|
@ -290,46 +291,31 @@ describe('ImportService', () => {
|
|||
expect(workflowHistoryRecords[0].description).toBe('Historical workflow description');
|
||||
});
|
||||
|
||||
test('should create a record in workflow publish history if active version exists', async () => {
|
||||
// Create an existing active workflow in the database first
|
||||
test('should call WorkflowService.deactivateWorkflow when re-importing an existing active workflow', async () => {
|
||||
const existingWorkflow = await createActiveWorkflow();
|
||||
const originalActiveVersionId = existingWorkflow.activeVersionId!;
|
||||
|
||||
// Now import it again (simulating re-import of an active workflow)
|
||||
const workflowToImport = await getWorkflowById(existingWorkflow.id);
|
||||
if (!workflowToImport) fail('Expected to find workflow');
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
const publishHistoryRecords = await workflowPublishHistoryRepository.find({
|
||||
where: {
|
||||
workflowId: existingWorkflow.id,
|
||||
event: 'deactivated',
|
||||
},
|
||||
});
|
||||
|
||||
// Should have publish history for deactivating the original active version
|
||||
expect(publishHistoryRecords).toHaveLength(1);
|
||||
expect(publishHistoryRecords[0].versionId).toBe(originalActiveVersionId);
|
||||
expect(mockWorkflowService.deactivateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
existingWorkflow.id,
|
||||
{ source: 'import' },
|
||||
);
|
||||
});
|
||||
|
||||
test('should not create a record in workflow publish history for new workflows', async () => {
|
||||
test('should not call WorkflowService.deactivateWorkflow for new (non-existing) workflows', async () => {
|
||||
mockWorkflowService.deactivateWorkflow.mockClear();
|
||||
|
||||
const workflowToImport = newWorkflow();
|
||||
workflowToImport.active = true;
|
||||
workflowToImport.activeVersionId = 'some-version';
|
||||
|
||||
if (!workflowToImport) fail('Expected to find workflow');
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {});
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id);
|
||||
|
||||
const publishHistoryRecords = await workflowPublishHistoryRepository.find({
|
||||
where: {
|
||||
workflowId: workflowToImport.id,
|
||||
event: 'deactivated',
|
||||
},
|
||||
});
|
||||
|
||||
expect(publishHistoryRecords).toHaveLength(0);
|
||||
expect(mockWorkflowService.deactivateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should always generate a new versionId when importing, ensuring proper history ordering', async () => {
|
||||
|
|
@ -340,7 +326,12 @@ describe('ImportService', () => {
|
|||
const workflowToReimport = await getWorkflowById(initialWorkflow.id);
|
||||
if (!workflowToReimport) fail('Expected to find workflow');
|
||||
|
||||
await importService.importWorkflows([workflowToReimport], ownerPersonalProject.id);
|
||||
await importService.importWorkflows(
|
||||
[workflowToReimport],
|
||||
ownerPersonalProject.id,
|
||||
owner.id,
|
||||
{},
|
||||
);
|
||||
|
||||
const historyRecords = await workflowHistoryRepository.find({
|
||||
where: { workflowId: initialWorkflow.id },
|
||||
|
|
@ -361,20 +352,19 @@ describe('ImportService', () => {
|
|||
const workflowToImport = await createWorkflow();
|
||||
workflowToImport.active = true;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
const dbWorkflow = await getWorkflowById(workflowToImport.id);
|
||||
if (!dbWorkflow) fail('Expected to find workflow');
|
||||
|
||||
expect(dbWorkflow.active).toBe(true);
|
||||
expect(dbWorkflow.activeVersionId).toBe(dbWorkflow.versionId);
|
||||
expect(mockActiveWorkflowManager.add).toHaveBeenCalledWith(workflowToImport.id, 'activate');
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
workflowToImport.id,
|
||||
expect.objectContaining({ source: 'import' }),
|
||||
);
|
||||
});
|
||||
|
||||
test('should deactivate imported workflow that is updating existing one when JSON has active=false', async () => {
|
||||
jest.mocked(mockActiveWorkflowManager.add).mockClear();
|
||||
mockWorkflowService.activateWorkflow.mockClear();
|
||||
|
||||
const existingWorkflow = await createActiveWorkflow();
|
||||
|
||||
|
|
@ -382,7 +372,7 @@ describe('ImportService', () => {
|
|||
if (!workflowToImport) fail('Expected to find workflow');
|
||||
workflowToImport.active = false;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
|
|
@ -391,16 +381,16 @@ describe('ImportService', () => {
|
|||
|
||||
expect(dbWorkflow.active).toBe(false);
|
||||
expect(dbWorkflow.activeVersionId).toBeNull();
|
||||
expect(mockActiveWorkflowManager.add).not.toHaveBeenCalled();
|
||||
expect(mockWorkflowService.activateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should leave imported workflow deactivated when JSON has active=false', async () => {
|
||||
jest.mocked(mockActiveWorkflowManager.add).mockClear();
|
||||
mockWorkflowService.activateWorkflow.mockClear();
|
||||
|
||||
const workflowToImport = await createWorkflow();
|
||||
workflowToImport.active = false;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
|
|
@ -409,60 +399,58 @@ describe('ImportService', () => {
|
|||
|
||||
expect(dbWorkflow.active).toBe(false);
|
||||
expect(dbWorkflow.activeVersionId).toBeNull();
|
||||
expect(mockActiveWorkflowManager.add).not.toHaveBeenCalled();
|
||||
expect(mockWorkflowService.activateWorkflow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should record both deactivated (old) and activated (new) publish history when re-importing an active workflow', async () => {
|
||||
test('should call both deactivateWorkflow and activateWorkflow when re-importing an active workflow', async () => {
|
||||
mockWorkflowService.deactivateWorkflow.mockClear();
|
||||
mockWorkflowService.activateWorkflow.mockClear();
|
||||
|
||||
const existingWorkflow = await createActiveWorkflow();
|
||||
const originalActiveVersionId = existingWorkflow.activeVersionId!;
|
||||
|
||||
const workflowToImport = await getWorkflowById(existingWorkflow.id);
|
||||
if (!workflowToImport) fail('Expected to find workflow');
|
||||
workflowToImport.active = true;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
const dbWorkflow = await getWorkflowById(existingWorkflow.id);
|
||||
if (!dbWorkflow) fail('Expected to find workflow');
|
||||
|
||||
const deactivatedRecords = await workflowPublishHistoryRepository.find({
|
||||
where: { workflowId: existingWorkflow.id, event: 'deactivated' },
|
||||
});
|
||||
const activatedForNewVersion = await workflowPublishHistoryRepository.find({
|
||||
where: {
|
||||
workflowId: existingWorkflow.id,
|
||||
event: 'activated',
|
||||
versionId: dbWorkflow.versionId,
|
||||
},
|
||||
});
|
||||
|
||||
expect(deactivatedRecords).toHaveLength(1);
|
||||
expect(deactivatedRecords[0].versionId).toBe(originalActiveVersionId);
|
||||
expect(activatedForNewVersion).toHaveLength(1);
|
||||
expect(activatedForNewVersion[0].userId).toBeNull();
|
||||
expect(mockWorkflowService.deactivateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
existingWorkflow.id,
|
||||
{ source: 'import' },
|
||||
);
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
existingWorkflow.id,
|
||||
expect.objectContaining({ source: 'import' }),
|
||||
);
|
||||
});
|
||||
|
||||
test('should not call ActiveWorkflowManager.remove for a brand-new active workflow', async () => {
|
||||
jest.mocked(mockActiveWorkflowManager.remove).mockClear();
|
||||
jest.mocked(mockActiveWorkflowManager.add).mockClear();
|
||||
test('should not call WorkflowService.deactivateWorkflow for a brand-new active workflow', async () => {
|
||||
mockWorkflowService.deactivateWorkflow.mockClear();
|
||||
mockWorkflowService.activateWorkflow.mockClear();
|
||||
|
||||
const workflowToImport = await createWorkflow();
|
||||
workflowToImport.active = true;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
expect(mockActiveWorkflowManager.remove).not.toHaveBeenCalled();
|
||||
expect(mockActiveWorkflowManager.add).toHaveBeenCalledTimes(1);
|
||||
expect(mockActiveWorkflowManager.add).toHaveBeenCalledWith(workflowToImport.id, 'activate');
|
||||
expect(mockWorkflowService.deactivateWorkflow).not.toHaveBeenCalled();
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledTimes(1);
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
workflowToImport.id,
|
||||
expect.objectContaining({ source: 'import' }),
|
||||
);
|
||||
});
|
||||
|
||||
test('should call ActiveWorkflowManager.remove exactly once when re-importing an active workflow', async () => {
|
||||
jest.mocked(mockActiveWorkflowManager.remove).mockClear();
|
||||
jest.mocked(mockActiveWorkflowManager.add).mockClear();
|
||||
test('should call WorkflowService.deactivateWorkflow exactly once when re-importing an active workflow', async () => {
|
||||
mockWorkflowService.deactivateWorkflow.mockClear();
|
||||
mockWorkflowService.activateWorkflow.mockClear();
|
||||
|
||||
const existingWorkflow = await createActiveWorkflow();
|
||||
|
||||
|
|
@ -470,14 +458,22 @@ describe('ImportService', () => {
|
|||
if (!workflowToImport) fail('Expected to find workflow');
|
||||
workflowToImport.active = true;
|
||||
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, {
|
||||
await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, owner.id, {
|
||||
activeState: 'fromJson',
|
||||
});
|
||||
|
||||
expect(mockActiveWorkflowManager.remove).toHaveBeenCalledTimes(1);
|
||||
expect(mockActiveWorkflowManager.remove).toHaveBeenCalledWith(existingWorkflow.id);
|
||||
expect(mockActiveWorkflowManager.add).toHaveBeenCalledTimes(1);
|
||||
expect(mockActiveWorkflowManager.add).toHaveBeenCalledWith(existingWorkflow.id, 'activate');
|
||||
expect(mockWorkflowService.deactivateWorkflow).toHaveBeenCalledTimes(1);
|
||||
expect(mockWorkflowService.deactivateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
existingWorkflow.id,
|
||||
{ source: 'import' },
|
||||
);
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledTimes(1);
|
||||
expect(mockWorkflowService.activateWorkflow).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: owner.id }),
|
||||
existingWorkflow.id,
|
||||
expect.objectContaining({ source: 'import' }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user