diff --git a/packages/cli/src/commands/import/__tests__/workflow.test.ts b/packages/cli/src/commands/import/__tests__/workflow.test.ts new file mode 100644 index 00000000000..cc78237dc7a --- /dev/null +++ b/packages/cli/src/commands/import/__tests__/workflow.test.ts @@ -0,0 +1,77 @@ +import { mockInstance } from '@n8n/backend-test-utils'; +import { GlobalConfig } from '@n8n/config'; +import { Container } from '@n8n/di'; + +import '@/zod-alias-support'; +import { ImportService } from '@/services/import.service'; + +import { ImportWorkflowsCommand } from '../workflow'; + +jest.mock('@/services/import.service'); + +describe('ImportWorkflowsCommand', () => { + mockInstance(ImportService); + + const globalConfig = Container.get(GlobalConfig); + const originalMode = globalConfig.executions.mode; + + afterEach(() => { + globalConfig.executions.mode = originalMode; + }); + + const buildCommand = () => { + const command = new ImportWorkflowsCommand(); + // @ts-expect-error Protected property + command.logger = { + info: jest.fn(), + error: jest.fn(), + }; + return command; + }; + + describe('--activeState flag', () => { + it('throws when n8n is not running in queue mode and activeState is set to "fromJson"', async () => { + globalConfig.executions.mode = 'regular'; + + const command = buildCommand(); + // @ts-expect-error Protected property + command.flags = { + input: './workflows.json', + separate: false, + activeState: 'fromJson', + }; + + await expect(command.run()).rejects.toThrow( + 'The "--activeState=fromJson" flag can only be used when n8n is running in queue or multi-main mode. In regular deployment mode, workflow activation is not supported.', + ); + }); + + it('does not throw on the queue-mode guard when running in queue mode', async () => { + globalConfig.executions.mode = 'queue'; + + const command = buildCommand(); + // @ts-expect-error Protected property + command.flags = { + // `input` intentionally missing so `run` returns early after the guard + // without us needing to mock filesystem/repositories. + separate: false, + activeState: 'fromJson', + }; + + await expect(command.run()).resolves.toBeUndefined(); + }); + + it('does not throw when activeState is "false", regardless of mode', async () => { + globalConfig.executions.mode = 'regular'; + + const command = buildCommand(); + // @ts-expect-error Protected property + command.flags = { + separate: false, + activeState: 'false', + }; + + await expect(command.run()).resolves.toBeUndefined(); + }); + }); +}); diff --git a/packages/cli/src/commands/import/workflow.ts b/packages/cli/src/commands/import/workflow.ts index b77dc85b54e..13ad543afc3 100644 --- a/packages/cli/src/commands/import/workflow.ts +++ b/packages/cli/src/commands/import/workflow.ts @@ -51,6 +51,16 @@ const flagsSchema = z.object({ .string() .describe('The ID of the project to assign the imported workflows to') .optional(), + activeState: z + .enum(['false', 'fromJson'], { + errorMap: () => ({ + message: 'Valid values for flag "--activeState" are only "false" or "fromJson".', + }), + }) + .describe( + 'Whether to respect the JSON active field. "false" (default) deactivates all imported workflows. "fromJson" activates/deactivates each workflow based on its JSON active field.', + ) + .default('false'), }); @Command({ @@ -62,6 +72,7 @@ const flagsSchema = z.object({ '--input=file.json --userId=1d64c3d2-85fe-4a83-a649-e446b07b3aae', '--input=file.json --projectId=Ox8O54VQrmBrb4qL', '--separate --input=backups/latest/ --userId=1d64c3d2-85fe-4a83-a649-e446b07b3aae', + '--input=file.json --activeState=fromJson', ], flagsSchema, }) @@ -69,6 +80,12 @@ export class ImportWorkflowsCommand extends BaseCommand { const { flags } = this; + if (flags.activeState === 'fromJson' && this.globalConfig.executions.mode !== 'queue') { + throw new UserError( + 'The "--activeState=fromJson" flag can only be used when n8n is running in queue or multi-main mode. In regular deployment mode, workflow activation is not supported.', + ); + } + if (!flags.input) { this.logger.info('An input file or directory with --input must be provided'); return; @@ -101,7 +118,9 @@ export class ImportWorkflowsCommand extends BaseCommand { let mockActiveWorkflowManager: ActiveWorkflowManager; let mockWorkflowIndexService: WorkflowIndexService; let mockDatabaseConfig: DatabaseConfig; + let mockWorkflowRepository: WorkflowRepository; + let mockWorkflowPublishHistoryRepository: WorkflowPublishHistoryRepository; beforeEach(() => { jest.clearAllMocks(); @@ -55,6 +62,8 @@ describe('ImportService', () => { mockActiveWorkflowManager = mock(); mockWorkflowIndexService = mock(); mockDatabaseConfig = mock(); + mockWorkflowRepository = mock(); + mockWorkflowPublishHistoryRepository = mock(); // Set up cipher mock mockCipher.decrypt = jest.fn((data: string) => data.replace('encrypted:', '')); @@ -103,6 +112,8 @@ describe('ImportService', () => { mockActiveWorkflowManager, mockWorkflowIndexService, mockDatabaseConfig, + mockWorkflowRepository, + mockWorkflowPublishHistoryRepository, ); }); diff --git a/packages/cli/src/services/import.service.ts b/packages/cli/src/services/import.service.ts index 6b42adac571..ec613bbc571 100644 --- a/packages/cli/src/services/import.service.ts +++ b/packages/cli/src/services/import.service.ts @@ -9,11 +9,14 @@ import { TagRepository, WorkflowHistory, WorkflowPublishHistory, + WorkflowPublishHistoryRepository, + WorkflowRepository, } from '@n8n/db'; // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import import { DataSource, EntityManager, In } from '@n8n/typeorm'; import { Service } from '@n8n/di'; import { type INode, type INodeCredentialsDetails, type IWorkflowBase } from 'n8n-workflow'; +import { ensureError } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; import { readdir, readFile } from 'fs/promises'; @@ -58,6 +61,8 @@ export class ImportService { private readonly activeWorkflowManager: ActiveWorkflowManager, private readonly workflowIndexService: WorkflowIndexService, private readonly databaseConfig: DatabaseConfig, + private readonly workflowRepository: WorkflowRepository, + private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository, ) {} async initRecords() { @@ -65,7 +70,11 @@ export class ImportService { this.dbTags = await this.tagRepository.find(); } - async importWorkflows(workflows: IWorkflowDb[], projectId: string) { + async importWorkflows( + workflows: IWorkflowDb[], + projectId: string, + { activeState = 'false' }: { activeState?: 'false' | 'fromJson' } = {}, + ) { await this.initRecords(); const { manager: dbManager } = this.credentialsRepository; @@ -108,6 +117,7 @@ export class ImportService { } const insertedWorkflows: IWorkflowBase[] = []; + const workflowsToActivate: Array<{ workflowId: string; versionId: string }> = []; await dbManager.transaction(async (tx) => { const workflowsNeedingPublishHistory: Array<{ workflowId: string; versionId: string }> = []; @@ -116,11 +126,19 @@ export class ImportService { // Always generate a new versionId on import to ensure proper history ordering workflow.versionId = uuid(); - // Always deactivate workflows on import - they need to be manually activated later // Store the old activeVersionId to record the deactivation of the old version const oldActiveVersionId = workflow.id ? activeVersionIdByWorkflow.get(workflow.id) : null; - if (oldActiveVersionId || workflow.activeVersionId || workflow.active) { - this.logger.info(`Deactivating workflow "${workflow.name}". Remember to activate later.`); + const shouldActivate = activeState === 'fromJson' && workflow.active; + const versionIdToActivate = workflow.versionId; + + // Always upsert with active=false and activeVersionId=null. + // Activation happens post-transaction once the new workflow_history row exists + // (the activeVersionId FK references workflow_history.versionId). + if ( + !shouldActivate && + (oldActiveVersionId || workflow.activeVersionId || workflow.active) + ) { + this.logger.info(`Deactivating workflow "${workflow.name}".`); } workflow.active = false; workflow.activeVersionId = null; @@ -134,6 +152,10 @@ export class ImportService { workflowsNeedingPublishHistory.push({ workflowId, versionId: oldActiveVersionId }); } + if (shouldActivate) { + workflowsToActivate.push({ workflowId, versionId: versionIdToActivate }); + } + const personalProject = await tx.findOneByOrFail(Project, { id: projectId }); // Create relationship if the workflow was inserted instead of updated. @@ -180,6 +202,10 @@ export class ImportService { } }); + for (const { workflowId, versionId } of workflowsToActivate) { + await this.activateWorkflow(workflowId, versionId); + } + // Directly update the index for the important workflows, since they don't generate // workflow-update events during import. // Workflow indexing isn't supported on legacy SQLite. @@ -190,6 +216,31 @@ export class ImportService { } } + private async activateWorkflow(workflowId: string, versionIdToActivate: string): Promise { + 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, + }); + } + } + } + async replaceInvalidCreds(workflow: IWorkflowBase, projectId: string) { try { await replaceInvalidCredentials(workflow, projectId); diff --git a/packages/cli/test/integration/commands/import.cmd.test.ts b/packages/cli/test/integration/commands/import.cmd.test.ts index 8117381d7fb..a5cb587e235 100644 --- a/packages/cli/test/integration/commands/import.cmd.test.ts +++ b/packages/cli/test/integration/commands/import.cmd.test.ts @@ -5,7 +5,9 @@ import { getAllSharedWorkflows, getAllWorkflows, } from '@n8n/backend-test-utils'; +import { GlobalConfig } from '@n8n/config'; import { WorkflowPublishHistoryRepository } from '@n8n/db'; +import { Container } from '@n8n/di'; import { nanoid } from 'nanoid'; import '@/zod-alias-support'; @@ -340,3 +342,115 @@ test('`import:workflow --projectId ... --userId ...` fails explaining that only 'You cannot use `--userId` and `--projectId` together. Use one or the other.', ); }); + +describe('--activeState flag', () => { + const globalConfig = Container.get(GlobalConfig); + const originalMode = globalConfig.executions.mode; + + beforeAll(() => { + globalConfig.executions.mode = 'queue'; + }); + + afterAll(() => { + globalConfig.executions.mode = originalMode; + }); + + describe('fromJson', () => { + it('should activate a workflow that is marked as active in the imported json', async () => { + await createOwner(); + + await command.run([ + '--separate', + '--input=./test/integration/commands/import-workflows/separate', + '--activeState=fromJson', + ]); + + const workflowsInDB = await getAllWorkflows(); + const activeWorkflow = workflowsInDB.find((w) => w.name === 'active-workflow'); + const inactiveWorkflow = workflowsInDB.find((w) => w.name === 'inactive-workflow'); + + expect(workflowsInDB).toHaveLength(2); + expect(activeWorkflow).toMatchObject({ active: true }); + expect(activeWorkflow?.activeVersionId).toBe(activeWorkflow?.versionId); + expect(inactiveWorkflow).toMatchObject({ active: false, activeVersionId: null }); + + const activeWorkflowManager = Container.get(ActiveWorkflowManager); + expect(activeWorkflowManager.add).toHaveBeenCalledWith('998', 'activate'); + expect(activeWorkflowManager.add).not.toHaveBeenCalledWith('999', expect.anything()); + }); + + 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(); + + await command.run([ + '--input=./test/integration/commands/import-workflows/combined-with-update/original.json', + '--activeState=fromJson', + ]); + + const [first] = await getAllWorkflows(); + const v1VersionId = first.versionId; + expect(first).toMatchObject({ id: '998', active: true, name: 'active-workflow' }); + expect(first.activeVersionId).toBe(v1VersionId); + + await command.run([ + '--input=./test/integration/commands/import-workflows/combined-with-update/updated.json', + '--activeState=fromJson', + ]); + + const [second] = await getAllWorkflows(); + expect(second).toMatchObject({ + id: '998', + active: true, + name: 'active-workflow updated', + }); + expect(second.versionId).not.toBe(v1VersionId); + expect(second.activeVersionId).toBe(second.versionId); + + const activeWorkflowManager = Container.get(ActiveWorkflowManager); + expect(activeWorkflowManager.remove).toHaveBeenCalledWith('998'); // first removing previously active version + expect(activeWorkflowManager.add).toHaveBeenLastCalledWith('998', 'activate'); // added the new version + + const publishHistoryRepo = Container.get(WorkflowPublishHistoryRepository); + expect(publishHistoryRepo.addRecord).toHaveBeenCalledTimes(2); + expect(publishHistoryRepo.addRecord).toHaveBeenLastCalledWith({ + workflowId: '998', + versionId: second.versionId, + event: 'activated', + userId: null, + }); + }); + }); + + describe('false', () => { + it('should deactivate a workflow that is active in the workflow json to import', async () => { + await createOwner(); + const fixture = + './test/integration/commands/import-workflows/separate/001-activeWorkflow.json'; + + // Setup: activate the workflow first by importing it with --activeState=fromJson + await command.run([`--input=${fixture}`, '--activeState=fromJson']); + + const [active] = await getAllWorkflows(); + expect(active).toMatchObject({ id: '998', active: true }); + expect(active.activeVersionId).toBe(active.versionId); + + const activeWorkflowManager = Container.get(ActiveWorkflowManager); + jest.mocked(activeWorkflowManager.add).mockClear(); + jest.mocked(activeWorkflowManager.remove).mockClear(); + + // Action: re-import the same active workflow JSON with --activeState=false + await command.run([`--input=${fixture}`, '--activeState=false']); + + const [deactivated] = await getAllWorkflows(); + expect(deactivated).toMatchObject({ + id: '998', + name: 'active-workflow', + active: false, + activeVersionId: null, + }); + + expect(activeWorkflowManager.remove).toHaveBeenCalledWith('998'); + expect(activeWorkflowManager.add).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/cli/test/integration/import.service.test.ts b/packages/cli/test/integration/import.service.test.ts index d630fcfac9a..e5254605998 100644 --- a/packages/cli/test/integration/import.service.test.ts +++ b/packages/cli/test/integration/import.service.test.ts @@ -71,6 +71,8 @@ describe('ImportService', () => { mockActiveWorkflowManager, mockWorkflowIndexService, Container.get(DatabaseConfig), + workflowRepository, + workflowPublishHistoryRepository, ); }); @@ -331,4 +333,126 @@ describe('ImportService', () => { const updatedWorkflow = await getWorkflowById(initialWorkflow.id); expect(updatedWorkflow?.versionId).toBe(historyRecords[1].versionId); }); + + describe('activeState: fromJson', () => { + test('should activate imported workflow when JSON has active=true', async () => { + const workflowToImport = await createWorkflow(); + workflowToImport.active = true; + + await importService.importWorkflows([workflowToImport], ownerPersonalProject.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'); + }); + + test('should deactivate imported workflow that is updating existing one when JSON has active=false', async () => { + jest.mocked(mockActiveWorkflowManager.add).mockClear(); + + const existingWorkflow = await createActiveWorkflow(); + + const workflowToImport = await getWorkflowById(existingWorkflow.id); + if (!workflowToImport) fail('Expected to find workflow'); + workflowToImport.active = false; + + await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, { + activeState: 'fromJson', + }); + + const dbWorkflow = await getWorkflowById(workflowToImport.id); + if (!dbWorkflow) fail('Expected to find workflow'); + + expect(dbWorkflow.active).toBe(false); + expect(dbWorkflow.activeVersionId).toBeNull(); + expect(mockActiveWorkflowManager.add).not.toHaveBeenCalled(); + }); + + test('should leave imported workflow deactivated when JSON has active=false', async () => { + jest.mocked(mockActiveWorkflowManager.add).mockClear(); + + const workflowToImport = await createWorkflow(); + workflowToImport.active = false; + + await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, { + activeState: 'fromJson', + }); + + const dbWorkflow = await getWorkflowById(workflowToImport.id); + if (!dbWorkflow) fail('Expected to find workflow'); + + expect(dbWorkflow.active).toBe(false); + expect(dbWorkflow.activeVersionId).toBeNull(); + expect(mockActiveWorkflowManager.add).not.toHaveBeenCalled(); + }); + + test('should record both deactivated (old) and activated (new) publish history when re-importing an active workflow', async () => { + 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, { + 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 activatedRecords = await workflowPublishHistoryRepository.find({ + where: { workflowId: existingWorkflow.id, event: 'activated' }, + }); + + expect(deactivatedRecords).toHaveLength(1); + expect(deactivatedRecords[0].versionId).toBe(originalActiveVersionId); + expect(activatedRecords).toHaveLength(1); + expect(activatedRecords[0].versionId).toBe(dbWorkflow.versionId); + expect(activatedRecords[0].userId).toBeNull(); + }); + + test('should not call ActiveWorkflowManager.remove for a brand-new active workflow', async () => { + jest.mocked(mockActiveWorkflowManager.remove).mockClear(); + jest.mocked(mockActiveWorkflowManager.add).mockClear(); + + const workflowToImport = await createWorkflow(); + workflowToImport.active = true; + + await importService.importWorkflows([workflowToImport], ownerPersonalProject.id, { + activeState: 'fromJson', + }); + + expect(mockActiveWorkflowManager.remove).not.toHaveBeenCalled(); + expect(mockActiveWorkflowManager.add).toHaveBeenCalledTimes(1); + expect(mockActiveWorkflowManager.add).toHaveBeenCalledWith(workflowToImport.id, 'activate'); + }); + + test('should call ActiveWorkflowManager.remove exactly once when re-importing an active workflow', async () => { + jest.mocked(mockActiveWorkflowManager.remove).mockClear(); + jest.mocked(mockActiveWorkflowManager.add).mockClear(); + + const existingWorkflow = await createActiveWorkflow(); + + const workflowToImport = await getWorkflowById(existingWorkflow.id); + if (!workflowToImport) fail('Expected to find workflow'); + workflowToImport.active = true; + + await importService.importWorkflows([workflowToImport], ownerPersonalProject.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'); + }); + }); }); diff --git a/packages/cli/test/integration/shared/utils/test-command.ts b/packages/cli/test/integration/shared/utils/test-command.ts index 1a2c8dd5761..781ac4adca8 100644 --- a/packages/cli/test/integration/shared/utils/test-command.ts +++ b/packages/cli/test/integration/shared/utils/test-command.ts @@ -1,5 +1,6 @@ import { testDb, mockInstance } from '@n8n/backend-test-utils'; -import type { CommandClass } from '@n8n/decorators'; +import { CommandMetadata, type CommandClass } from '@n8n/decorators'; +import { Container } from '@n8n/di'; import argvParser from 'yargs-parser'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; @@ -29,7 +30,11 @@ export const setupTestCommand = (Command: T) => { const run = async (argv: string[] = []) => { const command = new Command(); - command.flags = argvParser(argv); + const rawFlags = argvParser(argv); + const entry = Container.get(CommandMetadata) + .getEntries() + .find(([, e]) => e.class === Command)?.[1]; + command.flags = entry?.flagsSchema ? entry.flagsSchema.parse(rawFlags) : rawFlags; await command.init?.(); await command.run(); return command;