From 9a72da03fd29d8a958dffeb96a167d9e0301a9f7 Mon Sep 17 00:00:00 2001 From: Daria Date: Thu, 4 Dec 2025 12:44:02 +0200 Subject: [PATCH] feat: Forbid creating active workflows (no-changelog) (#22718) --- .../__tests__/workflows.controller.test.ts | 3 + .../cli/src/workflows/workflows.controller.ts | 12 ++++ .../integration/workflow-history.api.test.ts | 55 +++++++++++++------ .../workflows/workflows.controller.test.ts | 17 ++---- 4 files changed, 57 insertions(+), 30 deletions(-) diff --git a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts index 0beaffb2164..8bf78ff203c 100644 --- a/packages/cli/src/workflows/__tests__/workflows.controller.test.ts +++ b/packages/cli/src/workflows/__tests__/workflows.controller.test.ts @@ -1,4 +1,5 @@ import type { ImportWorkflowFromUrlDto } from '@n8n/api-types'; +import type { Logger } from '@n8n/backend-common'; import type { AuthenticatedRequest, IExecutionResponse, CredentialsEntity, User } from '@n8n/db'; import { WorkflowEntity } from '@n8n/db'; import axios from 'axios'; @@ -25,9 +26,11 @@ describe('WorkflowsController', () => { const req = mock(); const res = mock(); const projectService = mock(); + const logger = mock(); beforeEach(() => { controller.projectService = projectService; + controller.logger = logger; jest.clearAllMocks(); }); diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index b8f26a610a3..82cde5ba59d 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -96,6 +96,18 @@ export class WorkflowsController { // mess with relations of other workflows delete req.body.shared; + // @ts-expect-error: We shouldn't accept this, this will be set when activating + if (req.body.activeVersionId || req.body.active) { + this.logger.warn( + 'Creating a workflow as active is not supported. The workflow will be created as inactive.', + { userId: req.user.id }, + ); + + // @ts-expect-error: We shouldn't accept this + req.body.activeVersionId = undefined; + req.body.active = false; + } + const newWorkflow = new WorkflowEntity(); Object.assign(newWorkflow, req.body); diff --git a/packages/cli/test/integration/workflow-history.api.test.ts b/packages/cli/test/integration/workflow-history.api.test.ts index ba2da10d1b5..fab10d5d014 100644 --- a/packages/cli/test/integration/workflow-history.api.test.ts +++ b/packages/cli/test/integration/workflow-history.api.test.ts @@ -1,5 +1,5 @@ import { createWorkflow, testDb } from '@n8n/backend-test-utils'; -import type { User } from '@n8n/db'; +import type { User, WorkflowHistory } from '@n8n/db'; import { createOwner, createUser } from './shared/db/users'; import { createWorkflowHistoryItem } from './shared/db/workflow-history'; @@ -149,31 +149,52 @@ describe('GET /workflow-history/:workflowId', () => { expect(resp.body.data).toHaveLength(5); expect(resp.body.data[0]).toEqual(last); }); + test('should include workflowPublishHistory records related to each history item', async () => { const workflow = await createWorkflow(undefined, owner); const v1 = await createWorkflowHistoryItem(workflow.id); const v2 = await createWorkflowHistoryItem(workflow.id); + const wph1 = await createWorkflowPublishHistoryItem(v1); const wph2 = await createWorkflowPublishHistoryItem(v1, { event: 'deactivated' }); const wph3 = await createWorkflowPublishHistoryItem(v2); - const resp = await authOwnerAgent.get(`/workflow-history/workflow/${workflow.id}`); + const response = await authOwnerAgent.get(`/workflow-history/workflow/${workflow.id}`); + expect(response.status).toBe(200); - expect(resp.status).toBe(200); - expect(resp.body.data[0].workflowPublishHistory).toHaveLength(1); - expect(resp.body.data[0].workflowPublishHistory).toContainEqual({ - ...wph3, - createdAt: wph3.createdAt.toISOString(), - }); - expect(resp.body.data[1].workflowPublishHistory).toHaveLength(2); - expect(resp.body.data[1].workflowPublishHistory).toContainEqual({ - ...wph1, - createdAt: wph1.createdAt.toISOString(), - }); - expect(resp.body.data[1].workflowPublishHistory).toContainEqual({ - ...wph2, - createdAt: wph2.createdAt.toISOString(), - }); + const body = response.body as { data: WorkflowHistory[] }; + + expect(body.data).toHaveLength(2); + + const publishHistories = body.data.map((history) => history.workflowPublishHistory); + expect(publishHistories).toEqual( + expect.arrayContaining([expect.any(Array), expect.any(Array)]), + ); + + const publishHistory1 = publishHistories.find((ph) => ph.length === 1)!; + const publishHistory2 = publishHistories.find((ph) => ph.length === 2)!; + + expect(publishHistory1).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + ...wph3, + createdAt: wph3.createdAt.toISOString(), + }), + ]), + ); + + expect(publishHistory2).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + ...wph1, + createdAt: wph1.createdAt.toISOString(), + }), + expect.objectContaining({ + ...wph2, + createdAt: wph2.createdAt.toISOString(), + }), + ]), + ); }); }); diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index 717dea8af19..fb95e7b3704 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -255,7 +255,7 @@ describe('POST /workflows', () => { expect(historyVersion!.nodes).toEqual(payload.nodes); }); - test('should create workflow as active when active: true is provided in POST body', async () => { + test('should create workflow as inactive even when active: true is provided in POST body', async () => { const payload = { name: 'active workflow', nodes: [ @@ -284,21 +284,12 @@ describe('POST /workflows', () => { expect(id).toBeDefined(); expect(versionId).toBeDefined(); - expect(activeVersionId).toBe(versionId); // Should be set to current version - expect(active).toBe(true); + expect(activeVersionId).toBeNull(); + expect(active).toBe(false); // Verify in database const workflow = await workflowRepository.findOneBy({ id }); - expect(workflow?.activeVersionId).toBe(versionId); - - // Verify history was created - const historyVersion = await workflowHistoryRepository.findOne({ - where: { - workflowId: id, - versionId, - }, - }); - expect(historyVersion).not.toBeNull(); + expect(workflow?.activeVersionId).toBeNull(); }); test('create workflow in personal project by default', async () => {