diff --git a/packages/@n8n/db/src/repositories/__tests__/shared-workflow.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/shared-workflow.repository.test.ts index 058e3288e1d..9d8920aa169 100644 --- a/packages/@n8n/db/src/repositories/__tests__/shared-workflow.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/shared-workflow.repository.test.ts @@ -1,15 +1,24 @@ import { Container } from '@n8n/di'; -import { In, type SelectQueryBuilder } from '@n8n/typeorm'; +import { In, type EntityManager, type SelectQueryBuilder } from '@n8n/typeorm'; import type { Mocked } from 'vitest'; import { mock } from 'vitest-mock-extended'; -import type { Folder, Project, WorkflowEntity } from '../../entities'; +import type { + Folder, + Project, + WorkflowEntity, + WorkflowHistory, + WorkflowPublishHistory, +} from '../../entities'; import { SharedWorkflow } from '../../entities'; import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager'; +import { mockInstance } from '../../utils/test-utils/mock-instance'; import { SharedWorkflowRepository } from '../shared-workflow.repository'; +import { WorkflowPublishHistoryRepository } from '../workflow-publish-history.repository'; describe('SharedWorkflowRepository', () => { const entityManager = mockEntityManager(SharedWorkflow); + const workflowPublishHistoryRepository = mockInstance(WorkflowPublishHistoryRepository); const sharedWorkflowRepository = Container.get(SharedWorkflowRepository); let queryBuilder: Mocked>; @@ -266,6 +275,66 @@ describe('SharedWorkflowRepository', () => { }); }); + describe('findWorkflowWithOptions', () => { + const sharedWorkflowWith = (activeVersion: WorkflowHistory | null) => + mock({ workflow: mock({ activeVersion }) }); + + it('loads the publish history of the active version in a separate query', async () => { + const activeVersion = mock({ versionId: 'version-1' }); + const events = [mock({ id: 1 })]; + entityManager.findOne.mockResolvedValueOnce(sharedWorkflowWith(activeVersion)); + workflowPublishHistoryRepository.findByVersion.mockResolvedValueOnce(events); + + const result = await sharedWorkflowRepository.findWorkflowWithOptions('workflow-1', { + includeActiveVersion: true, + }); + + expect(entityManager.findOne).toHaveBeenCalledWith( + SharedWorkflow, + expect.objectContaining({ + relations: expect.objectContaining({ + workflow: expect.objectContaining({ activeVersion: true }), + }), + }), + ); + expect(workflowPublishHistoryRepository.findByVersion).toHaveBeenCalledWith( + 'workflow-1', + 'version-1', + entityManager, + ); + expect(result?.workflow.activeVersion?.workflowPublishHistory).toBe(events); + }); + + it('loads the publish history with the entity manager of the caller', async () => { + const trx = mock(); + trx.findOne.mockResolvedValueOnce( + sharedWorkflowWith(mock({ versionId: 'version-1' })), + ); + workflowPublishHistoryRepository.findByVersion.mockResolvedValueOnce([]); + + await sharedWorkflowRepository.findWorkflowWithOptions('workflow-1', { + includeActiveVersion: true, + em: trx, + }); + + expect(workflowPublishHistoryRepository.findByVersion).toHaveBeenCalledWith( + 'workflow-1', + 'version-1', + trx, + ); + }); + + it('does not load publish history when the workflow has no active version', async () => { + entityManager.findOne.mockResolvedValueOnce(sharedWorkflowWith(null)); + + await sharedWorkflowRepository.findWorkflowWithOptions('workflow-1', { + includeActiveVersion: true, + }); + + expect(workflowPublishHistoryRepository.findByVersion).not.toHaveBeenCalled(); + }); + }); + describe('findOwnerProjectsByWorkflowIds', () => { it('should map each workflow id to its owner project', async () => { const projectA = mock({ id: 'project-a' }); diff --git a/packages/@n8n/db/src/repositories/shared-workflow.repository.ts b/packages/@n8n/db/src/repositories/shared-workflow.repository.ts index c35e0ca5252..4a9cd4d4121 100644 --- a/packages/@n8n/db/src/repositories/shared-workflow.repository.ts +++ b/packages/@n8n/db/src/repositories/shared-workflow.repository.ts @@ -14,6 +14,7 @@ import type { } from '@n8n/typeorm'; import { BaseRepository } from './base-repository'; +import { WorkflowPublishHistoryRepository } from './workflow-publish-history.repository'; import type { User } from '../entities'; import { Project, ProjectRelation, SharedWorkflow } from '../entities'; import { type OperationContext, TransactionRunner } from '../services/transaction'; @@ -21,7 +22,11 @@ import { chunkIds } from '../utils/chunk-ids'; @Service() export class SharedWorkflowRepository extends BaseRepository { - constructor(dataSource: DataSource, transactionRunner: TransactionRunner) { + constructor( + dataSource: DataSource, + transactionRunner: TransactionRunner, + private readonly workflowPublishHistoryRepository: WorkflowPublishHistoryRepository, + ) { super(SharedWorkflow, dataSource.manager, transactionRunner); } @@ -344,7 +349,7 @@ export class SharedWorkflowRepository extends BaseRepository { em = this.manager, } = options; - return await em.findOne(SharedWorkflow, { + const sharedWorkflow = await em.findOne(SharedWorkflow, { where: { workflowId, ...where, @@ -354,10 +359,22 @@ export class SharedWorkflowRepository extends BaseRepository { shared: { project: true }, tags: includeTags, parentFolder: includeParentFolder, - activeVersion: includeActiveVersion ? { workflowPublishHistory: true } : false, + activeVersion: includeActiveVersion, }, }, }); + + const activeVersion = sharedWorkflow?.workflow.activeVersion; + if (activeVersion) { + activeVersion.workflowPublishHistory = + await this.workflowPublishHistoryRepository.findByVersion( + workflowId, + activeVersion.versionId, + em, + ); + } + + return sharedWorkflow; } /** diff --git a/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts b/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts index 0eb459aef28..d814db10aec 100644 --- a/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-publish-history.repository.ts @@ -28,6 +28,16 @@ export class WorkflowPublishHistoryRepository extends Repository>> { diff --git a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts index 9aadaeab976..9ac90e89e2e 100644 --- a/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts +++ b/packages/cli/src/workflows/workflow-history/__tests__/workflow-history.service.test.ts @@ -51,6 +51,7 @@ describe('WorkflowHistoryService', () => { mockClear(workflowHistoryRepository.find); mockClear(workflowHistoryRepository.findOne); mockClear(workflowPublishHistoryRepository.find); + mockClear(workflowPublishHistoryRepository.findByVersion); mockClear(workflowFinderService.findWorkflowForUser); }); @@ -269,6 +270,26 @@ describe('WorkflowHistoryService', () => { }); }); + describe('getVersion', () => { + it('should not load publish history when includePublishHistory is false', async () => { + // Arrange + const workflow = getWorkflow({ addNodeWithoutCreds: true }); + workflow.id = '123'; + const version = getWorkflowHistory(workflow, { versionId: 'version1' }); + workflowFinderService.findWorkflowForUser.mockResolvedValueOnce(workflow); + workflowHistoryRepository.findOne.mockResolvedValueOnce(version); + + // Act + const result = await workflowHistoryService.getVersion(testUser, workflow.id, 'version1', { + includePublishHistory: false, + }); + + // Assert + expect(result).toBe(version); + expect(workflowPublishHistoryRepository.findByVersion).not.toHaveBeenCalled(); + }); + }); + describe('getVersionsByIds', () => { it('should return empty array when versionIds is empty', async () => { // Arrange diff --git a/packages/cli/src/workflows/workflow-history/workflow-history.service.ts b/packages/cli/src/workflows/workflow-history/workflow-history.service.ts index 350166bcc88..f75218c883a 100644 --- a/packages/cli/src/workflows/workflow-history/workflow-history.service.ts +++ b/packages/cli/src/workflows/workflow-history/workflow-history.service.ts @@ -81,19 +81,22 @@ export class WorkflowHistoryService { throw new SharedWorkflowNotFoundError(''); } - const includePublishHistory = settings?.includePublishHistory ?? true; - const relations = includePublishHistory ? ['workflowPublishHistory'] : []; - const hist = await this.workflowHistoryRepository.findOne({ where: { workflowId: workflow.id, versionId, }, - relations, }); if (!hist) { throw new WorkflowHistoryVersionNotFoundError(''); } + + if (settings?.includePublishHistory ?? true) { + hist.workflowPublishHistory = await this.workflowPublishHistoryRepository.findByVersion( + workflow.id, + versionId, + ); + } return hist; } diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 20e6215f21a..3797f0c0eb4 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -1242,17 +1242,21 @@ export class WorkflowService { // Fetch workflow again with workflowPublishHistory after activation to include the new entry const updatedWorkflow = await this.workflowRepository.findOne({ where: { id: workflowId }, - relations: { - activeVersion: { - workflowPublishHistory: true, - }, - }, + relations: { activeVersion: true }, }); if (!updatedWorkflow) { throw new NotFoundError(`Workflow with ID "${workflowId}" could not be found.`); } + if (updatedWorkflow.activeVersion) { + updatedWorkflow.activeVersion.workflowPublishHistory = + await this.workflowPublishHistoryRepository.findByVersion( + workflowId, + updatedWorkflow.activeVersion.versionId, + ); + } + return updatedWorkflow; } diff --git a/packages/cli/test/integration/workflow-history.api.test.ts b/packages/cli/test/integration/workflow-history.api.test.ts index 8c1b9d13381..b3642c52af6 100644 --- a/packages/cli/test/integration/workflow-history.api.test.ts +++ b/packages/cli/test/integration/workflow-history.api.test.ts @@ -303,13 +303,11 @@ describe('GET /workflow-history/workflow/:workflowId/version/:versionId', () => ...v1, createdAt: v1.createdAt.toISOString(), updatedAt: v1.updatedAt.toISOString(), - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - workflowPublishHistory: expect.arrayContaining([ + workflowPublishHistory: [ { ...wph1, createdAt: wph1.createdAt.toISOString() }, { ...wph2, createdAt: wph2.createdAt.toISOString() }, - ]), + ], }); - expect(resp.body.data.workflowPublishHistory).toHaveLength(2); }); }); diff --git a/packages/cli/test/integration/workflows/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts index f855c894ac3..bf1621466b5 100644 --- a/packages/cli/test/integration/workflows/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -63,6 +63,7 @@ import { createUser, } from '../shared/db/users'; import { createWorkflowHistoryItem } from '../shared/db/workflow-history'; +import { createWorkflowPublishHistoryItem } from '../shared/db/workflow-publish-history'; import type { SuperAgentTest } from '../shared/types'; import * as utils from '../shared/utils/'; import { makeWorkflow, MOCK_PINDATA } from '../shared/utils/'; @@ -1087,6 +1088,22 @@ describe('GET /workflows/:workflowId', () => { }); }); + test('should return every publish event of the active version in order', async () => { + const workflow = await createActiveWorkflow({}, owner); + const activeVersion = { workflowId: workflow.id, versionId: workflow.activeVersionId! }; + await createWorkflowPublishHistoryItem(activeVersion, { event: 'deactivated' }); + await createWorkflowPublishHistoryItem(activeVersion); + + const response = await authOwnerAgent.get(`/workflows/${workflow.id}`).expect(200); + + const { data } = response.body as { data: { activeVersion: WorkflowHistory } }; + expect(data.activeVersion.workflowPublishHistory.map(({ event }) => event)).toEqual([ + 'activated', + 'deactivated', + 'activated', + ]); + }); + test('should return parent folder', async () => { const personalProject = await projectRepository.getPersonalProjectForUserOrFail(owner.id);