mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
fix(core): Load workflow publish history in a separate query (#40171)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
4939dbc787
commit
548bd57802
|
|
@ -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<SelectQueryBuilder<SharedWorkflow>>;
|
||||
|
|
@ -266,6 +275,66 @@ describe('SharedWorkflowRepository', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('findWorkflowWithOptions', () => {
|
||||
const sharedWorkflowWith = (activeVersion: WorkflowHistory | null) =>
|
||||
mock<SharedWorkflow>({ workflow: mock<WorkflowEntity>({ activeVersion }) });
|
||||
|
||||
it('loads the publish history of the active version in a separate query', async () => {
|
||||
const activeVersion = mock<WorkflowHistory>({ versionId: 'version-1' });
|
||||
const events = [mock<WorkflowPublishHistory>({ 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<EntityManager>();
|
||||
trx.findOne.mockResolvedValueOnce(
|
||||
sharedWorkflowWith(mock<WorkflowHistory>({ 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<Project>({ id: 'project-a' });
|
||||
|
|
|
|||
|
|
@ -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<SharedWorkflow> {
|
||||
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<SharedWorkflow> {
|
|||
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<SharedWorkflow> {
|
|||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -28,6 +28,16 @@ export class WorkflowPublishHistoryRepository extends Repository<WorkflowPublish
|
|||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the events of one version, oldest first. Use this method, not a
|
||||
* join on the `workflowPublishHistory` relation. A join repeats the nodes
|
||||
* JSON of the version for each event, and a version can have many events.
|
||||
*/
|
||||
async findByVersion(workflowId: string, versionId: string, trx?: EntityManager) {
|
||||
const repository = trx ? trx.getRepository(WorkflowPublishHistory) : this;
|
||||
return await repository.find({ where: { workflowId, versionId }, order: { id: 'ASC' } });
|
||||
}
|
||||
|
||||
async getPublishedVersions(
|
||||
workflowId: string,
|
||||
): Promise<Array<Pick<WorkflowPublishHistory, 'versionId'>>> {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user