opensource/n8n
3
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-08-28 17:22:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(core): Split workflow review repositories by use case (no-changelog) (#37188)

Browse Source
This commit is contained in:
Kai
2026-08-27 12:12:02 +00:00
committed by GitHub
parent 5b4c65be39
commit 51ac8f6896
11 changed files with 557 additions and 573 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/@n8n/db/src/repositories/__tests__/workflow-review-request.repository.test.ts → packages/@n8n/db/src/repositories/__tests__/workflow-review.repositories.test.ts
+65 -55
View File
@@ -11,9 +11,11 @@ import { WorkflowReviewRequest } from '../../entities/workflow-review-request.ee
import { TypeOrmTransaction } from '../../services/typeorm-transaction';
import { mockEntityManager } from '../../utils/test-utils/mock-entity-manager';
import {
WorkflowReviewRequestRepository,
WorkflowReviewInboxRepository,
type InboxVisibility,
} from '../workflow-review-request.repository';
} from '../workflow-review-inbox.repository';
import { WorkflowReviewLifecycleRepository } from '../workflow-review-lifecycle.repository';
import { WorkflowReviewRequestRepository } from '../workflow-review-request.repository';
/** Stand-ins for the SQL TypeORM would render for the correlated junction subqueries. */
const AUTHOR_SUBQUERY_SQL = '(SELECT 1 FROM workflow_review_request_authors author WHERE ...)';
@@ -50,9 +52,11 @@ function involvedVisibility(
};
}
describe('WorkflowReviewRequestRepository', () => {
describe('workflow review repositories', () => {
const entityManager = mockEntityManager(WorkflowReviewRequest);
const repo = Container.get(WorkflowReviewRequestRepository);
const requestRepository = Container.get(WorkflowReviewRequestRepository);
const inboxRepository = Container.get(WorkflowReviewInboxRepository);
const lifecycleRepository = Container.get(WorkflowReviewLifecycleRepository);
let queryBuilder: Mocked<SelectQueryBuilder<WorkflowReviewRequest>>;
let subQueryBuilders: Array<Mocked<SelectQueryBuilder<WorkflowReviewRequestAuthor>>>;
@@ -92,17 +96,17 @@ describe('WorkflowReviewRequestRepository', () => {
queryBuilder.getMany.mockResolvedValue([]);
queryBuilder.getRawMany.mockResolvedValue([]);
vi.spyOn(repo, 'createQueryBuilder').mockReturnValue(queryBuilder);
vi.spyOn(inboxRepository, 'createQueryBuilder').mockReturnValue(queryBuilder);
});
describe('createRequest', () => {
describe('WorkflowReviewRequestRepository.createRequest', () => {
it('persists an open pending request with audit fields initialised', async () => {
(entityManager.create as Mock).mockImplementation(
(_target: unknown, entityLike: unknown) => entityLike as WorkflowReviewRequest,
);
entityManager.save.mockImplementationOnce(async (_target, entity) => entity);
await repo.createRequest(
await requestRepository.createRequest(
{
id: 'req-1',
projectId: 'proj-1',
@@ -134,7 +138,7 @@ describe('WorkflowReviewRequestRepository', () => {
);
entityManager.save.mockImplementationOnce(async (_target, entity) => entity);
await repo.createRequest(
await requestRepository.createRequest(
{
projectId: 'proj-1',
title: 'Review title',
@@ -151,7 +155,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
});
describe('findRequestsForWorkflow', () => {
describe('WorkflowReviewRequestRepository.findRequestsForWorkflow', () => {
let queryBuilder: Mocked<SelectQueryBuilder<WorkflowReviewRequest>>;
beforeEach(() => {
@@ -171,7 +175,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('scopes to the requested workflow and orders newest first, ties broken by id', async () => {
await repo.findRequestsForWorkflow('workflow-1');
await requestRepository.findRequestsForWorkflow('workflow-1');
expect(queryBuilder.where).toHaveBeenCalledWith('requestWorkflow.workflowId = :workflowId', {
workflowId: 'workflow-1',
@@ -184,7 +188,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it.each(['open', 'closed'] as const)('narrows to state %s when given', async (state) => {
await repo.findRequestsForWorkflow('workflow-1', { state });
await requestRepository.findRequestsForWorkflow('workflow-1', { state });
expect(queryBuilder.andWhere).toHaveBeenCalledWith('request.state = :state', { state });
});
@@ -197,7 +201,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
queryBuilder.getCount.mockResolvedValue(5);
const [data, count] = await repo.findRequestsForWorkflow('workflow-1', {
const [data, count] = await requestRepository.findRequestsForWorkflow('workflow-1', {
skip: 1,
take: 1,
});
@@ -232,7 +236,7 @@ describe('WorkflowReviewRequestRepository', () => {
],
});
const [data] = await repo.findRequestsForWorkflow('workflow-1');
const [data] = await requestRepository.findRequestsForWorkflow('workflow-1');
expect(data[0]).toEqual({
id: 'req-1',
@@ -247,7 +251,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('applies skip and take when they are zero', async () => {
await repo.findRequestsForWorkflow('workflow-1', { skip: 0, take: 0 });
await requestRepository.findRequestsForWorkflow('workflow-1', { skip: 0, take: 0 });
expect(queryBuilder.skip).toHaveBeenCalledWith(0);
expect(queryBuilder.take).toHaveBeenCalledWith(0);
@@ -270,7 +274,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
queryBuilder.getCount.mockResolvedValue(2);
const [data] = await repo.findRequestsForWorkflow('workflow-1');
const [data] = await requestRepository.findRequestsForWorkflow('workflow-1');
expect(queryBuilder.addSelect).toHaveBeenCalledWith(
'requestWorkflow.workflowVersionId',
@@ -293,7 +297,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
});
describe('findOpenRequestsForWorkflows', () => {
describe('WorkflowReviewLifecycleRepository.findOpenRequestsAffectedByWorkflows', () => {
let queryBuilder: Mocked<SelectQueryBuilder<WorkflowReviewRequest>>;
beforeEach(() => {
@@ -307,14 +311,17 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('returns an empty list without querying when no workflow ids are given', async () => {
const result = await repo.findOpenRequestsForWorkflows([], {});
const result = await lifecycleRepository.findOpenRequestsAffectedByWorkflows([], {});
expect(result).toEqual([]);
expect(entityManager.createQueryBuilder).not.toHaveBeenCalled();
});
it('scopes to the given workflows and to open requests only', async () => {
await repo.findOpenRequestsForWorkflows(['workflow-1', 'workflow-2'], {});
await lifecycleRepository.findOpenRequestsAffectedByWorkflows(
['workflow-1', 'workflow-2'],
{},
);
expect(queryBuilder.where).toHaveBeenCalledWith(
'requestWorkflow.workflowId IN (:...workflowIds)',
@@ -338,7 +345,10 @@ describe('WorkflowReviewRequestRepository', () => {
],
});
const result = await repo.findOpenRequestsForWorkflows(['workflow-1', 'workflow-2'], {});
const result = await lifecycleRepository.findOpenRequestsAffectedByWorkflows(
['workflow-1', 'workflow-2'],
{},
);
expect(queryBuilder.addSelect).toHaveBeenCalledWith(
'requestWorkflow.workflowVersionId',
@@ -362,7 +372,7 @@ describe('WorkflowReviewRequestRepository', () => {
const transactionManager = mock<EntityManager>();
(transactionManager.createQueryBuilder as Mock).mockReturnValue(queryBuilder);
await repo.findOpenRequestsForWorkflows(['workflow-1'], {
await lifecycleRepository.findOpenRequestsAffectedByWorkflows(['workflow-1'], {
trx: new TypeOrmTransaction(transactionManager),
});
@@ -371,7 +381,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
});
describe('findUnreviewableOpenRequestIds', () => {
describe('WorkflowReviewLifecycleRepository.findUnreviewableOpenRequestIds', () => {
let queryBuilder: Mocked<SelectQueryBuilder<WorkflowReviewRequest>>;
beforeEach(() => {
@@ -418,23 +428,23 @@ describe('WorkflowReviewRequestRepository', () => {
},
]);
expect(await repo.findUnreviewableOpenRequestIds({})).toEqual(['req-orphan']);
expect(await lifecycleRepository.findUnreviewableOpenRequestIds({})).toEqual(['req-orphan']);
expect(queryBuilder.andWhere).not.toHaveBeenCalled();
await repo.findUnreviewableOpenRequestIds({}, ['req-1', 'req-2']);
await lifecycleRepository.findUnreviewableOpenRequestIds({}, ['req-1', 'req-2']);
expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.id IN (:...candidateRequestIds)', {
candidateRequestIds: ['req-1', 'req-2'],
});
(entityManager.createQueryBuilder as Mock).mockClear();
expect(await repo.findUnreviewableOpenRequestIds({}, [])).toEqual([]);
expect(await lifecycleRepository.findUnreviewableOpenRequestIds({}, [])).toEqual([]);
expect(entityManager.createQueryBuilder).not.toHaveBeenCalled();
});
});
describe('closeRequests', () => {
describe('WorkflowReviewRequestRepository.closeRequests', () => {
it('bulk-closes the given requests, clearing the closing user and bumping updatedAt', async () => {
await repo.closeRequests(['req-1', 'req-2'], {});
await requestRepository.closeRequests(['req-1', 'req-2'], {});
expect(entityManager.update).toHaveBeenCalledWith(WorkflowReviewRequest, ['req-1', 'req-2'], {
state: 'closed',
@@ -443,18 +453,18 @@ describe('WorkflowReviewRequestRepository', () => {
});
entityManager.update.mockClear();
await repo.closeRequests([], {});
await requestRepository.closeRequests([], {});
expect(entityManager.update).not.toHaveBeenCalled();
});
});
describe('findById', () => {
describe('WorkflowReviewRequestRepository.findById', () => {
it("reads through the context's transaction manager", async () => {
const transactionManager = mock<EntityManager>();
const request = mock<WorkflowReviewRequest>({ id: 'req-1' });
transactionManager.findOne.mockResolvedValue(request);
const result = await repo.findById('req-1', {
const result = await requestRepository.findById('req-1', {
trx: new TypeOrmTransaction(transactionManager),
});
@@ -466,19 +476,19 @@ describe('WorkflowReviewRequestRepository', () => {
});
});
describe('findManyForInbox', () => {
describe('WorkflowReviewInboxRepository.findRequests', () => {
it('skips visibility filtering entirely for the whole-inbox scope', async () => {
const rows = [mock<WorkflowReviewRequest>({ id: 'req-1' })];
queryBuilder.getMany.mockResolvedValueOnce(rows);
const result = await repo.findManyForInbox({
const result = await inboxRepository.findRequests({
visibility: allVisibility,
state: 'open',
limit: 15,
});
expect(result).toBe(rows);
expect(repo.createQueryBuilder).toHaveBeenCalledWith('review');
expect(inboxRepository.createQueryBuilder).toHaveBeenCalledWith('review');
expect(queryBuilder.andWhere).not.toHaveBeenCalledWith(
expect.stringContaining('review.projectId'),
expect.anything(),
@@ -495,7 +505,7 @@ describe('WorkflowReviewRequestRepository', () => {
const rows = [mock<WorkflowReviewRequest>({ id: 'req-1' })];
queryBuilder.getMany.mockResolvedValueOnce(rows);
const result = await repo.findManyForInbox({
const result = await inboxRepository.findRequests({
visibility: involvedVisibility({
adminProjectIds: ['admin-proj'],
readableProjectIds: ['read-1', 'read-2'],
@@ -516,7 +526,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('resolves workflow readability through shared_workflow, not the stored project', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: involvedVisibility({ readableProjectIds: ['read-1'] }),
limit: 15,
});
@@ -539,7 +549,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('drops the readability conjunct entirely when the caller is unrestricted', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: involvedVisibility({ readableProjectIds: null }),
limit: 15,
});
@@ -555,7 +565,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('correlates both involvement subqueries through their entities, never literal table names', async () => {
await repo.findManyForInbox({ visibility: involvedVisibility(), limit: 15 });
await inboxRepository.findRequests({ visibility: involvedVisibility(), limit: 15 });
const [authorSubQuery, reviewerSubQuery] = subQueryBuilders;
expect(authorSubQuery.from).toHaveBeenCalledWith(
@@ -577,7 +587,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('returns nothing when no project is readable, admin projects included', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: involvedVisibility({ adminProjectIds: ['admin-proj'], readableProjectIds: [] }),
limit: 15,
});
@@ -587,7 +597,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('matches nothing when the caller administers no project and can read none', async () => {
const result = await repo.findManyForInbox({
const result = await inboxRepository.findRequests({
visibility: involvedVisibility({ adminProjectIds: [], readableProjectIds: [] }),
limit: 15,
});
@@ -597,11 +607,11 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('applies the keyset boundary carried in the cursor without an anchor lookup', async () => {
const findOneSpy = vi.spyOn(repo, 'findOne');
const findOneSpy = vi.spyOn(inboxRepository, 'findOne');
queryBuilder.getMany.mockResolvedValueOnce([]);
const createdAt = new Date('2024-01-02T00:00:00.000Z');
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: involvedVisibility(),
limit: 10,
cursor: { createdAt, id: 'req-cursor' },
@@ -616,7 +626,7 @@ describe('WorkflowReviewRequestRepository', () => {
describe('category filter', () => {
it('leaves the query untouched when no category is requested', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: allVisibility,
limit: 15,
});
@@ -625,7 +635,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('correlates both junction subqueries through their entities, never literal table names', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: allVisibility,
category: { userId: 'user-1', category: 'authored' },
limit: 15,
@@ -647,7 +657,7 @@ describe('WorkflowReviewRequestRepository', () => {
// The requester always has an author row, so neither predicate needs
// the nullable `createdById`.
it('matches a non-reviewing author for the authored section', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: allVisibility,
category: { userId: 'user-1', category: 'authored' },
limit: 15,
@@ -660,7 +670,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('matches assigned reviewers first for the waiting section', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: allVisibility,
category: { userId: 'user-1', category: 'waiting' },
limit: 15,
@@ -673,7 +683,7 @@ describe('WorkflowReviewRequestRepository', () => {
});
it('narrows the visibility predicate instead of replacing it', async () => {
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: involvedVisibility(),
category: { userId: 'user-1', category: 'waiting' },
limit: 15,
@@ -689,7 +699,7 @@ describe('WorkflowReviewRequestRepository', () => {
it('applies the category filter before the limit and the cursor boundary', async () => {
const createdAt = new Date('2024-01-02T00:00:00.000Z');
await repo.findManyForInbox({
await inboxRepository.findRequests({
visibility: allVisibility,
category: { userId: 'user-1', category: 'authored' },
state: 'open',
@@ -706,16 +716,16 @@ describe('WorkflowReviewRequestRepository', () => {
});
});
describe('countByStateForInbox', () => {
describe('WorkflowReviewInboxRepository.countRequestsByState', () => {
it('groups by state under the involvement visibility predicate', async () => {
queryBuilder.getRawMany.mockResolvedValueOnce([{ state: 'open', count: '2' }]);
const result = await repo.countByStateForInbox({
visibility: involvedVisibility({ readableProjectIds: ['proj-1', 'proj-2'] }),
});
const result = await inboxRepository.countRequestsByState(
involvedVisibility({ readableProjectIds: ['proj-1', 'proj-2'] }),
);
expect(result).toEqual({ open: 2, closed: 0 });
expect(repo.createQueryBuilder).toHaveBeenCalledWith('review');
expect(inboxRepository.createQueryBuilder).toHaveBeenCalledWith('review');
expect(queryBuilder.select).toHaveBeenCalledWith('review.state', 'state');
expect(queryBuilder.addSelect).toHaveBeenCalledWith('COUNT(*)', 'count');
expect(queryBuilder.groupBy).toHaveBeenCalledWith('review.state');
@@ -735,7 +745,7 @@ describe('WorkflowReviewRequestRepository', () => {
{ state: 'closed', count: '12' },
]);
const result = await repo.countByStateForInbox({ visibility: allVisibility });
const result = await inboxRepository.countRequestsByState(allVisibility);
expect(result).toEqual({ open: 3, closed: 12 });
expect(queryBuilder.andWhere).not.toHaveBeenCalled();
@@ -745,9 +755,9 @@ describe('WorkflowReviewRequestRepository', () => {
it('counts nothing when the caller administers no project and can read none', async () => {
queryBuilder.getRawMany.mockResolvedValueOnce([]);
const result = await repo.countByStateForInbox({
visibility: involvedVisibility({ adminProjectIds: [], readableProjectIds: [] }),
});
const result = await inboxRepository.countRequestsByState(
involvedVisibility({ adminProjectIds: [], readableProjectIds: [] }),
);
expect(result).toEqual({ open: 0, closed: 0 });
expect(queryBuilder.andWhere).toHaveBeenCalledWith('1 = 0');
packages/@n8n/db/src/repositories/index.ts
+6 -2
View File
@@ -85,10 +85,14 @@ export {
export { WorkflowPublishHistoryRepository } from './workflow-publish-history.repository';
export {
WorkflowReviewRequestRepository,
type InboxCursor,
type InboxVisibility,
type WorkflowReviewRequestForWorkflowRow,
} from './workflow-review-request.repository';
export { WorkflowReviewLifecycleRepository } from './workflow-review-lifecycle.repository';
export {
WorkflowReviewInboxRepository,
type InboxCursor,
type InboxVisibility,
} from './workflow-review-inbox.repository';
export {
WorkflowReviewRequestWorkflowRepository,
type WorkflowReviewRequestLinkedWorkflow,
packages/@n8n/db/src/repositories/workflow-review-inbox.repository.ts
+224
View File
@@ -0,0 +1,224 @@
import { Service } from '@n8n/di';
import { DataSource, type SelectQueryBuilder } from '@n8n/typeorm';
import { BaseRepository } from './base-repository';
import { SharedWorkflow } from '../entities/shared-workflow';
import { WorkflowReviewRequestAuthor } from '../entities/workflow-review-request-author.ee';
import { WorkflowReviewRequestReviewer } from '../entities/workflow-review-request-reviewer.ee';
import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee';
import {
WorkflowReviewRequest,
type WorkflowReviewRequestState,
} from '../entities/workflow-review-request.ee';
import { TransactionRunner } from '../services/transaction';
/** The cursor carries its boundary values so deleting the previous page's last row is safe. */
export type InboxCursor = {
createdAt: Date;
id: string;
};
/**
* Reviewers belong in `waiting`, even when they are also authors. All other authors belong in
* `authored`. The requester always has an author row, so this does not use `createdById`.
*/
type InboxCategoryFilter = {
userId: string;
category: 'waiting' | 'authored';
};
export type InboxVisibility =
| { scope: 'all' }
| {
scope: 'involved';
userId: string;
adminProjectIds: string[];
/** Null means the user can read workflows in every project. */
readableProjectIds: string[] | null;
readableWorkflowRoles: string[];
};
type FindInboxRequestsOptions = {
visibility: InboxVisibility;
state?: WorkflowReviewRequestState;
category?: InboxCategoryFilter;
limit: number;
cursor?: InboxCursor;
};
export type InboxStateCounts = {
open: number;
closed: number;
};
function participantExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
junctionEntity: typeof WorkflowReviewRequestAuthor | typeof WorkflowReviewRequestReviewer,
alias: string,
userParameter: string,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(junctionEntity, alias)
.where(`${alias}.workflowReviewRequestId = review.id`)
.andWhere(`${alias}.userId = :${userParameter}`)
.getQuery();
}
function linkedWorkflowExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
alias: string,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(WorkflowReviewRequestWorkflow, alias)
.where(`${alias}.workflowReviewRequestId = review.id`)
.getQuery();
}
function readableLinkedWorkflowExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(SharedWorkflow, 'visibilityShared')
.innerJoin(
WorkflowReviewRequestWorkflow,
'visibilityLink',
'visibilityLink.workflowId = visibilityShared.workflowId',
)
.where('visibilityLink.workflowReviewRequestId = review.id')
.andWhere('visibilityShared.role IN (:...readableWorkflowRoles)')
.andWhere('visibilityShared.projectId IN (:...readableProjectIds)')
.getQuery();
}
function applyInboxVisibility(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
visibility: InboxVisibility,
): void {
if (visibility.scope === 'all') return;
const { userId, adminProjectIds, readableProjectIds, readableWorkflowRoles } = visibility;
const parameters: Record<string, unknown> = { involvedUserId: userId };
const clauses: string[] = [];
if (adminProjectIds.length > 0) {
clauses.push('review.projectId IN (:...adminProjectIds)');
parameters.adminProjectIds = adminProjectIds;
}
const authorExists = participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestAuthor,
'visibilityAuthor',
'involvedUserId',
);
const reviewerExists = participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestReviewer,
'visibilityReviewer',
'involvedUserId',
);
clauses.push(`(EXISTS ${authorExists} OR EXISTS ${reviewerExists})`);
if (readableProjectIds === null) {
queryBuilder.andWhere(`(${clauses.join(' OR ')})`, parameters);
return;
}
if (readableProjectIds.length === 0 || readableWorkflowRoles.length === 0) {
queryBuilder.andWhere('1 = 0');
return;
}
parameters.readableProjectIds = readableProjectIds;
parameters.readableWorkflowRoles = readableWorkflowRoles;
// Use current workflow access because the request's stored project can become stale.
const anyLinkExists = linkedWorkflowExistsSubquery(queryBuilder, 'visibilityAnyLink');
const readableLinkExists = readableLinkedWorkflowExistsSubquery(queryBuilder);
queryBuilder.andWhere(
`(${clauses.join(' OR ')}) AND (NOT EXISTS ${anyLinkExists} OR EXISTS ${readableLinkExists})`,
parameters,
);
}
function applyCategoryFilter(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
{ userId, category }: InboxCategoryFilter,
): void {
const authorExists = participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestAuthor,
'author',
'categoryUserId',
);
const reviewerExists = participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestReviewer,
'reviewer',
'categoryUserId',
);
if (category === 'authored') {
queryBuilder.andWhere(`(EXISTS ${authorExists} AND NOT EXISTS ${reviewerExists})`, {
categoryUserId: userId,
});
return;
}
queryBuilder.andWhere(`(EXISTS ${reviewerExists} OR NOT EXISTS ${authorExists})`, {
categoryUserId: userId,
});
}
@Service()
export class WorkflowReviewInboxRepository extends BaseRepository<WorkflowReviewRequest> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(WorkflowReviewRequest, dataSource.manager, transactionRunner);
}
async findRequests(options: FindInboxRequestsOptions): Promise<WorkflowReviewRequest[]> {
const { visibility, state, category, limit, cursor } = options;
const queryBuilder = this.createQueryBuilder('review')
.orderBy('review.createdAt', 'DESC')
.addOrderBy('review.id', 'ASC');
applyInboxVisibility(queryBuilder, visibility);
if (category) applyCategoryFilter(queryBuilder, category);
if (state !== undefined) {
queryBuilder.andWhere('review.state = :state', { state });
}
if (cursor) {
queryBuilder.andWhere(
'(review.createdAt < :createdAt OR (review.createdAt = :createdAt AND review.id > :id))',
{ createdAt: cursor.createdAt, id: cursor.id },
);
}
queryBuilder.take(limit);
return await queryBuilder.getMany();
}
async countRequestsByState(visibility: InboxVisibility): Promise<InboxStateCounts> {
const queryBuilder = this.createQueryBuilder('review')
.select('review.state', 'state')
.addSelect('COUNT(*)', 'count')
.groupBy('review.state');
applyInboxVisibility(queryBuilder, visibility);
const rows = await queryBuilder.getRawMany<{
state: WorkflowReviewRequestState;
count: string | number;
}>();
return {
open: Number(rows.find((row) => row.state === 'open')?.count ?? 0),
closed: Number(rows.find((row) => row.state === 'closed')?.count ?? 0),
};
}
}
packages/@n8n/db/src/repositories/workflow-review-lifecycle.repository.ts
+147
View File
@@ -0,0 +1,147 @@
import { Service } from '@n8n/di';
import type { WorkflowSharingRole } from '@n8n/permissions';
import { DataSource } from '@n8n/typeorm';
import { BaseRepository } from './base-repository';
import { SharedWorkflow } from '../entities/shared-workflow';
import { WorkflowEntity } from '../entities/workflow-entity';
import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee';
import {
WorkflowReviewRequest,
type WorkflowReviewRequestState,
} from '../entities/workflow-review-request.ee';
import { type OperationContext, TransactionRunner } from '../services/transaction';
type OpenRequestWorkflowRow = {
requestId: string;
requestProjectId: string;
linkedWorkflowId: string | null;
/** Raw database boolean: `1` on SQLite, `true` on Postgres. */
isArchived: boolean | number | null;
owningProjectId: string | null;
};
type OpenRequestAffectedByWorkflows = {
request: WorkflowReviewRequest;
links: Array<{ workflowId: string; workflowVersionId: string | null }>;
};
function isReviewable(row: OpenRequestWorkflowRow): boolean {
if (row.linkedWorkflowId === null) return false;
if (row.isArchived) return false;
// A missing owner row is broken data, not evidence that the workflow moved.
return row.owningProjectId === null || row.owningProjectId === row.requestProjectId;
}
@Service()
export class WorkflowReviewLifecycleRepository extends BaseRepository<WorkflowReviewRequest> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(WorkflowReviewRequest, dataSource.manager, transactionRunner);
}
/**
* Finds open requests linked to the workflows and returns only the matching links.
* Lifecycle handlers use the links to record one activity for each affected workflow.
*/
async findOpenRequestsAffectedByWorkflows(
workflowIds: string[],
ctx: OperationContext,
): Promise<OpenRequestAffectedByWorkflows[]> {
if (workflowIds.length === 0) return [];
const state: WorkflowReviewRequestState = 'open';
const { entities, raw } = await this.managerFor(ctx)
.createQueryBuilder(WorkflowReviewRequest, 'request')
.innerJoin(
WorkflowReviewRequestWorkflow,
'requestWorkflow',
'requestWorkflow.workflowReviewRequestId = request.id',
)
.addSelect('requestWorkflow.workflowId', 'linkedWorkflowId')
.addSelect('requestWorkflow.workflowVersionId', 'linkedWorkflowVersionId')
.where('requestWorkflow.workflowId IN (:...workflowIds)', { workflowIds })
.andWhere('request.state = :state', { state })
.getRawAndEntities<{
request_id: string;
linkedWorkflowId: string;
linkedWorkflowVersionId: string | null;
}>();
const linksByRequestId = new Map<
string,
Array<{ workflowId: string; workflowVersionId: string | null }>
>();
for (const row of raw) {
const links = linksByRequestId.get(row.request_id) ?? [];
links.push({
workflowId: row.linkedWorkflowId,
workflowVersionId: row.linkedWorkflowVersionId ?? null,
});
linksByRequestId.set(row.request_id, links);
}
return entities.map((request) => ({
request,
links: linksByRequestId.get(request.id) ?? [],
}));
}
/**
* Finds open requests that have no workflow left to review. A workflow is reviewable when it
* exists, is not archived, and still belongs to the request's project.
*
* This checks current workflow state instead of the mutation that changed it. It catches links
* removed by delete cascades, mutations that bypass lifecycle hooks, and close operations that
* rolled back after the workflow mutation committed.
*
* This method only reads. The caller must hold the review mutation lock until it closes the
* returned requests with the same transaction context.
*/
async findUnreviewableOpenRequestIds(
ctx: OperationContext,
candidateRequestIds?: string[],
): Promise<string[]> {
if (candidateRequestIds?.length === 0) return [];
const openState: WorkflowReviewRequestState = 'open';
const ownerRole: WorkflowSharingRole = 'workflow:owner';
const qb = this.managerFor(ctx)
.createQueryBuilder(WorkflowReviewRequest, 'review')
.select('review.id', 'requestId')
.addSelect('review.projectId', 'requestProjectId')
.addSelect('workflow.id', 'linkedWorkflowId')
.addSelect('workflow.isArchived', 'isArchived')
.addSelect('shared.projectId', 'owningProjectId')
// Keep requests whose link or workflow was removed so they can be closed.
.leftJoin(WorkflowReviewRequestWorkflow, 'link', 'link.workflowReviewRequestId = review.id')
.leftJoin(WorkflowEntity, 'workflow', 'workflow.id = link.workflowId')
.leftJoin(
SharedWorkflow,
'shared',
'shared.workflowId = link.workflowId AND shared.role = :ownerRole',
{ ownerRole },
)
.where('review.state = :openState', { openState });
if (candidateRequestIds !== undefined) {
qb.andWhere('review.id IN (:...candidateRequestIds)', { candidateRequestIds });
}
const rows = await qb.getRawMany<OpenRequestWorkflowRow>();
const closableRequestIds = new Set<string>();
const requestIdsWithReviewableWorkflow = new Set<string>();
for (const row of rows) {
if (isReviewable(row)) {
requestIdsWithReviewableWorkflow.add(row.requestId);
} else {
closableRequestIds.add(row.requestId);
}
}
for (const requestId of requestIdsWithReviewableWorkflow) {
closableRequestIds.delete(requestId);
}
return [...closableRequestIds];
}
}
packages/@n8n/db/src/repositories/workflow-review-request.repository.ts
+8 -446
View File
@@ -1,14 +1,8 @@
import { Service } from '@n8n/di';
import type { WorkflowSharingRole } from '@n8n/permissions';
import type { SelectQueryBuilder } from '@n8n/typeorm';
import { DataSource } from '@n8n/typeorm';
import { BaseRepository } from './base-repository';
import { SharedWorkflow } from '../entities/shared-workflow';
import { WorkflowEntity } from '../entities/workflow-entity';
import { WorkflowHistory } from '../entities/workflow-history';
import { WorkflowReviewRequestAuthor } from '../entities/workflow-review-request-author.ee';
import { WorkflowReviewRequestReviewer } from '../entities/workflow-review-request-reviewer.ee';
import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee';
import {
WorkflowReviewRequest,
@@ -17,69 +11,6 @@ import {
} from '../entities/workflow-review-request.ee';
import { type OperationContext, TransactionRunner } from '../services/transaction';
/**
* Keyset pagination boundary. The caller carries `createdAt`/`id` in the cursor
* itself so pagination never depends on the anchor row still existing.
*/
export type InboxCursor = {
createdAt: Date;
id: string;
};
/**
* Splits the visible reviews into the two groups a caller can ask for:
* `waiting` = assigned reviewer, or not an author; `authored` = an author who is
* not assigned to review it. Being a reviewer wins, so a review sits where the
* caller's pending action is. Only narrows what visibility already allowed.
*
* Authorship alone decides the split. The requester always has an author row, so
* neither group needs the nullable `createdById`.
*/
export type InboxCategoryFilter = {
userId: string;
category: 'waiting' | 'authored';
};
/**
* Who may see which reviews in the inbox.
*
* `all` — every review (global admins/owners). `involved` — reviews in projects the
* caller administers, plus reviews they participate in (author or assigned
* reviewer; the requester is always an author) — in both cases only while they can
* still read one of the workflows the review covers.
*/
export type InboxVisibility =
| { scope: 'all' }
| {
scope: 'involved';
userId: string;
/** Projects the caller administers: every review in them is visible. */
adminProjectIds: string[];
/**
* Projects the caller reads workflows through: their personal project,
* where directly shared workflows land, plus team projects granting
* `workflow:read`. `null` means every project — listing them all would
* bind one parameter per project on every inbox query.
*/
readableProjectIds: string[] | null;
/** Workflow sharing roles that grant `workflow:read`. */
readableWorkflowRoles: string[];
};
export type FindManyForInboxOptions = {
visibility: InboxVisibility;
state?: WorkflowReviewRequestState;
/** Omitted means no category filter. */
category?: InboxCategoryFilter;
limit: number;
cursor?: InboxCursor;
};
/**
* Projection for the workflow-scoped list: the request fields the use case
* needs plus the version pinned for the workflow the query was scoped to, and
* the name that version was given.
*/
export type WorkflowReviewRequestForWorkflowRow = Pick<
WorkflowReviewRequest,
| 'id'
@@ -95,37 +26,6 @@ export type WorkflowReviewRequestForWorkflowRow = Pick<
workflowVersionName: string | null;
};
export type CountByStateForInboxOptions = {
visibility: InboxVisibility;
};
export type InboxStateCounts = {
open: number;
closed: number;
};
/** One row per (open request, linked workflow); a request with no link left yields one empty row. */
type OpenRequestWorkflowRow = {
requestId: string;
requestProjectId: string;
linkedWorkflowId: string | null;
/** Raw, so the driver's own boolean: `1` on sqlite and mysql, `true` on postgres. */
isArchived: boolean | number | null;
owningProjectId: string | null;
};
/** Reviewable: the linked workflow exists, is not archived, and still belongs to the request's project. */
function isReviewable(row: OpenRequestWorkflowRow): boolean {
// Nothing behind the link: either the request has no link row left, or it points at a
// workflow that is gone. Both mean the delete cascade got there first.
if (row.linkedWorkflowId === null) return false;
if (row.isArchived) return false;
// A workflow with no owning project at all is a broken row, not a move — leave it alone.
return row.owningProjectId === null || row.owningProjectId === row.requestProjectId;
}
@Service()
export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowReviewRequest> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
@@ -161,10 +61,7 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
return await this.managerFor(ctx).save(WorkflowReviewRequest, entity);
}
/**
* Persists an already-loaded request. Deliberately `save` and not `update`, so the
* entity's `@BeforeUpdate` hook bumps `updatedAt`.
*/
/** Uses save so the entity's BeforeUpdate hook updates updatedAt. */
async saveRequest(
request: WorkflowReviewRequest,
ctx: OperationContext,
@@ -172,79 +69,7 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
return await this.managerFor(ctx).save(WorkflowReviewRequest, request);
}
/**
* Ids of the open requests with no reviewable workflow left — every linked workflow deleted,
* archived, or moved out of the request's project. `candidateRequestIds` narrows the scan to
* those requests (the targeted lifecycle paths); omitting it evaluates every open request (the
* reconciliation sweep).
*
* Matches on the workflows' current state rather than on the mutation that changed it, so it
* catches what the per-mutation hooks cannot: reviews a delete cascade unlinked before a hook
* could find them by workflow id, mutations that skip the hooks entirely, and hooks whose
* close rolled back after their mutation had already committed.
*
* Read-only; the caller closes the returned ids with {@link closeRequests} under the
* review-request lock, so selecting and closing cannot race a concurrent decision.
*/
async findUnreviewableOpenRequestIds(
ctx: OperationContext,
candidateRequestIds?: string[],
): Promise<string[]> {
if (candidateRequestIds?.length === 0) return [];
const openState: WorkflowReviewRequestState = 'open';
const ownerRole: WorkflowSharingRole = 'workflow:owner';
const qb = this.managerFor(ctx)
.createQueryBuilder(WorkflowReviewRequest, 'review')
.select('review.id', 'requestId')
.addSelect('review.projectId', 'requestProjectId')
.addSelect('workflow.id', 'linkedWorkflowId')
.addSelect('workflow.isArchived', 'isArchived')
.addSelect('shared.projectId', 'owningProjectId')
// Left joins throughout: a request with no link, or a link with no workflow, is
// precisely the orphan case, and dropping those rows would hide it.
.leftJoin(WorkflowReviewRequestWorkflow, 'link', 'link.workflowReviewRequestId = review.id')
.leftJoin(WorkflowEntity, 'workflow', 'workflow.id = link.workflowId')
.leftJoin(
SharedWorkflow,
'shared',
'shared.workflowId = link.workflowId AND shared.role = :ownerRole',
{ ownerRole },
)
.where('review.state = :openState', { openState });
if (candidateRequestIds !== undefined) {
qb.andWhere('review.id IN (:...candidateRequestIds)', { candidateRequestIds });
}
const rows = await qb.getRawMany<OpenRequestWorkflowRow>();
// One reviewable workflow keeps the request open, however many of its siblings are gone.
const closableRequestIds = new Set<string>();
const requestIdsWithReviewableWorkflow = new Set<string>();
for (const row of rows) {
if (isReviewable(row)) {
requestIdsWithReviewableWorkflow.add(row.requestId);
} else {
closableRequestIds.add(row.requestId);
}
}
for (const requestId of requestIdsWithReviewableWorkflow) {
closableRequestIds.delete(requestId);
}
return [...closableRequestIds];
}
/**
* Bulk-closes the given requests. A system close has no closing user, and the decision stays
* as-is. `updatedAt` is set explicitly because `manager.update` skips `@BeforeUpdate`.
*
* Keys off the given ids rather than a state predicate, so the caller must hold the
* review-request lock across selecting them ({@link findUnreviewableOpenRequestIds}) and
* closing them here.
*/
/** The caller selects the request IDs and holds the review mutation lock. */
async closeRequests(requestIds: string[], ctx: OperationContext): Promise<void> {
if (requestIds.length === 0) return;
@@ -252,6 +77,7 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
await this.managerFor(ctx).update(WorkflowReviewRequest, requestIds, {
state: closedState,
closedById: null,
// update() does not run the entity's BeforeUpdate hook.
updatedAt: new Date(),
});
}
@@ -280,23 +106,17 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
.addSelect('pinnedVersion.name', 'pinnedWorkflowVersionName')
.where('requestWorkflow.workflowId = :workflowId', { workflowId })
.orderBy('request.createdAt', 'DESC')
// Ids are random, so this only breaks ties deterministically: callers ask
// for the newest review to decide the publish gate, and that answer must
// not flip between requests when two reviews share a timestamp.
// IDs only break timestamp ties. Stable ordering keeps offset pagination consistent
// when requests have the same creation time.
.addOrderBy('request.id', 'DESC');
if (options.state) {
qb.andWhere('request.state = :state', { state: options.state });
}
if (options.skip !== undefined) {
qb.skip(options.skip);
}
if (options.take !== undefined) {
qb.take(options.take);
}
if (options.skip !== undefined) qb.skip(options.skip);
if (options.take !== undefined) qb.take(options.take);
// Sequential, not concurrent: both calls run off the same builder, and each
// mutates its shared state while executing.
// These calls share a mutable query builder, so run them in order.
const { entities, raw } = await qb.getRawAndEntities<{
request_id: string;
pinnedWorkflowVersionId: string | null;
@@ -304,8 +124,6 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
}>();
const count = await qb.getCount();
// Raw rows are 1:1 with entities — the (requestId, workflowId) pair is unique —
// but key by id instead of index to stay independent of entity deduplication.
const pinnedByRequestId = new Map<
string,
{ workflowVersionId: string | null; workflowVersionName: string | null }
@@ -350,260 +168,4 @@ export class WorkflowReviewRequestRepository extends BaseRepository<WorkflowRevi
.orderBy('request.createdAt', 'DESC')
.getOne();
}
/**
* All open requests linked to any of the given workflows, each with the
* subset of those workflows it is linked to and the version pinned per link —
* so a lifecycle cleanup can close a request once while knowing which
* workflows were affected, and a status read can report the pin.
*/
async findOpenRequestsForWorkflows(
workflowIds: string[],
ctx: OperationContext,
): Promise<
Array<{
request: WorkflowReviewRequest;
links: Array<{ workflowId: string; workflowVersionId: string | null }>;
}>
> {
if (workflowIds.length === 0) return [];
const state: WorkflowReviewRequestState = 'open';
const { entities, raw } = await this.managerFor(ctx)
.createQueryBuilder(WorkflowReviewRequest, 'request')
.innerJoin(
WorkflowReviewRequestWorkflow,
'requestWorkflow',
'requestWorkflow.workflowReviewRequestId = request.id',
)
.addSelect('requestWorkflow.workflowId', 'linkedWorkflowId')
.addSelect('requestWorkflow.workflowVersionId', 'linkedWorkflowVersionId')
.where('requestWorkflow.workflowId IN (:...workflowIds)', { workflowIds })
.andWhere('request.state = :state', { state })
.getRawAndEntities<{
request_id: string;
linkedWorkflowId: string;
linkedWorkflowVersionId: string | null;
}>();
// Raw rows are per (request, workflow) pair; entities are deduplicated.
const linksByRequestId = new Map<
string,
Array<{ workflowId: string; workflowVersionId: string | null }>
>();
for (const row of raw) {
const links = linksByRequestId.get(row.request_id) ?? [];
links.push({
workflowId: row.linkedWorkflowId,
workflowVersionId: row.linkedWorkflowVersionId ?? null,
});
linksByRequestId.set(row.request_id, links);
}
return entities.map((request) => ({
request,
links: linksByRequestId.get(request.id) ?? [],
}));
}
async findManyForInbox(options: FindManyForInboxOptions): Promise<WorkflowReviewRequest[]> {
const { visibility, state, category, limit, cursor } = options;
const queryBuilder = this.createQueryBuilder('review')
.orderBy('review.createdAt', 'DESC')
.addOrderBy('review.id', 'ASC');
this.applyInboxVisibility(queryBuilder, visibility);
if (category) {
this.applyCategoryFilter(queryBuilder, category);
}
if (state !== undefined) {
queryBuilder.andWhere('review.state = :state', { state });
}
if (cursor) {
queryBuilder.andWhere(
'(review.createdAt < :createdAt OR (review.createdAt = :createdAt AND review.id > :id))',
{ createdAt: cursor.createdAt, id: cursor.id },
);
}
queryBuilder.take(limit);
return await queryBuilder.getMany();
}
async countByStateForInbox(options: CountByStateForInboxOptions): Promise<InboxStateCounts> {
const queryBuilder = this.createQueryBuilder('review')
.select('review.state', 'state')
.addSelect('COUNT(*)', 'count')
.groupBy('review.state');
this.applyInboxVisibility(queryBuilder, options.visibility);
const rows = await queryBuilder.getRawMany<{
state: WorkflowReviewRequestState;
count: string | number;
}>();
return {
open: Number(rows.find((row) => row.state === 'open')?.count ?? 0),
closed: Number(rows.find((row) => row.state === 'closed')?.count ?? 0),
};
}
/**
* Inbox visibility — see {@link InboxVisibility}. A review is visible when the
* caller administers its project or takes part in it, and can still read one of
* the workflows it covers. Neither means no rows.
*/
private applyInboxVisibility(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
visibility: InboxVisibility,
): void {
if (visibility.scope === 'all') {
return;
}
const { userId, adminProjectIds, readableProjectIds, readableWorkflowRoles } = visibility;
const parameters: Record<string, unknown> = { involvedUserId: userId };
const clauses: string[] = [];
if (adminProjectIds.length > 0) {
clauses.push('review.projectId IN (:...adminProjectIds)');
parameters.adminProjectIds = adminProjectIds;
}
const authorExists = this.participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestAuthor,
'visibilityAuthor',
'involvedUserId',
);
const reviewerExists = this.participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestReviewer,
'visibilityReviewer',
'involvedUserId',
);
// No separate term for the requester: they always have an author row.
clauses.push(`(EXISTS ${authorExists} OR EXISTS ${reviewerExists})`);
// This caller reads every workflow, so the check below is always true. Skip it.
if (readableProjectIds === null) {
queryBuilder.andWhere(`(${clauses.join(' OR ')})`, parameters);
return;
}
// A caller who can read no workflow sees no reviews, admins included.
if (readableProjectIds.length === 0 || readableWorkflowRoles.length === 0) {
queryBuilder.andWhere('1 = 0');
return;
}
parameters.readableProjectIds = readableProjectIds;
parameters.readableWorkflowRoles = readableWorkflowRoles;
// Check the workflows the caller can read now, not the review's stored project,
// which goes stale. Same rule as the detail gate, so a listed row always opens.
const anyLinkExists = this.linkedWorkflowExistsSubquery(queryBuilder, 'visibilityAnyLink');
const readableLinkExists = this.readableLinkedWorkflowExistsSubquery(queryBuilder);
queryBuilder.andWhere(
`(${clauses.join(' OR ')}) AND (NOT EXISTS ${anyLinkExists} OR EXISTS ${readableLinkExists})`,
parameters,
);
}
/** `EXISTS`-ready subquery: the current `review` row covers at least one workflow. */
private linkedWorkflowExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
alias: string,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(WorkflowReviewRequestWorkflow, alias)
.where(`${alias}.workflowReviewRequestId = review.id`)
.getQuery();
}
/**
* `EXISTS`-ready subquery: the caller can read one of the workflows the `review`
* row covers, looked up through current `shared_workflow` rows.
*/
private readableLinkedWorkflowExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(SharedWorkflow, 'visibilityShared')
.innerJoin(
WorkflowReviewRequestWorkflow,
'visibilityLink',
'visibilityLink.workflowId = visibilityShared.workflowId',
)
.where('visibilityLink.workflowReviewRequestId = review.id')
.andWhere('visibilityShared.role IN (:...readableWorkflowRoles)')
.andWhere('visibilityShared.projectId IN (:...readableProjectIds)')
.getQuery();
}
/**
* `EXISTS`-ready subquery probing a participant junction table (authors or
* reviewers) for the current `review` row and the user bound to `userParameter`.
*/
private participantExistsSubquery(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
junctionEntity: typeof WorkflowReviewRequestAuthor | typeof WorkflowReviewRequestReviewer,
alias: string,
userParameter: string,
): string {
return queryBuilder
.subQuery()
.select('1')
.from(junctionEntity, alias)
.where(`${alias}.workflowReviewRequestId = review.id`)
.andWhere(`${alias}.userId = :${userParameter}`)
.getQuery();
}
/**
* Narrows the visible rows to one category — see {@link InboxCategoryFilter}.
* The two predicates are opposites, so every review lands in exactly one.
* Always `andWhere`: {@link applyInboxVisibility} runs first.
*/
private applyCategoryFilter(
queryBuilder: SelectQueryBuilder<WorkflowReviewRequest>,
{ userId, category }: InboxCategoryFilter,
): void {
const authorExists = this.participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestAuthor,
'author',
'categoryUserId',
);
const reviewerExists = this.participantExistsSubquery(
queryBuilder,
WorkflowReviewRequestReviewer,
'reviewer',
'categoryUserId',
);
if (category === 'authored') {
queryBuilder.andWhere(`(EXISTS ${authorExists} AND NOT EXISTS ${reviewerExists})`, {
categoryUserId: userId,
});
return;
}
queryBuilder.andWhere(`(EXISTS ${reviewerExists} OR NOT EXISTS ${authorExists})`, {
categoryUserId: userId,
});
}
}
packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-inbox.detail.service.test.ts
+11 -6
View File
@@ -2,8 +2,8 @@ import type { LicenseState } from '@n8n/backend-common';
import type {
User,
WorkflowHistory,
WorkflowReviewInboxRepository,
WorkflowReviewRequest,
WorkflowReviewRequestRepository,
WorkflowReviewRequestState,
WorkflowReviewRequestWorkflowDetailRow,
WorkflowReviewRequestWorkflowRepository,
@@ -62,7 +62,7 @@ describe('WorkflowReviewInboxService.getDetail', () => {
const workflowReviewPolicyService = mock<WorkflowReviewPolicyService>();
const authorizationService = mock<WorkflowReviewAuthorizationService>();
const workflowHistoryService = mock<WorkflowHistoryService>();
const requestRepository = mock<WorkflowReviewRequestRepository>();
const inboxRepository = mock<WorkflowReviewInboxRepository>();
const workflowRepository = mock<WorkflowReviewRequestWorkflowRepository>();
const participantResolver = mock<WorkflowReviewParticipantResolver>();
const licenseState = mock<LicenseState>();
@@ -71,7 +71,7 @@ describe('WorkflowReviewInboxService.getDetail', () => {
new WorkflowReviewFeatureGate(licenseState, workflowReviewPolicyService),
authorizationService,
workflowHistoryService,
requestRepository,
inboxRepository,
workflowRepository,
participantResolver,
);
@@ -173,11 +173,12 @@ describe('WorkflowReviewInboxService.getDetail', () => {
// A covered workflow is removed along with the workflow itself, so a closed
// review — history of a deleted workflow — can legitimately cover none
it('returns a closed review with no workflows when its workflow was deleted', async () => {
requestRepository.findById.mockResolvedValue(reviewRequest({ state: 'closed' }));
mockGate([], reviewRequest({ state: 'closed' }));
workflowRepository.findLinkedWorkflowDetailsByRequestId.mockResolvedValue([]);
const detail = await service.getDetail(requester, requestId);
expect(detail.state).toBe('closed');
expect(detail.workflows).toEqual([]);
});
@@ -264,14 +265,18 @@ describe('WorkflowReviewInboxService.getDetail', () => {
});
it('passes empty coverage when a closed review no longer covers any workflow', async () => {
requestRepository.findById.mockResolvedValue(reviewRequest({ state: 'closed' }));
mockGate([], reviewRequest({ state: 'closed' }));
workflowRepository.findLinkedWorkflowDetailsByRequestId.mockResolvedValue([]);
await service.getDetail(requester, requestId);
expect(authorizationService.resolveViewerEligibility).toHaveBeenCalledWith(
requester,
expect.objectContaining({ workflowRows: [], readableWorkflowRows: [] }),
expect.objectContaining({
request: expect.objectContaining({ state: 'closed' }),
workflowRows: [],
readableWorkflowRows: [],
}),
);
});
});
packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-inbox.list.service.test.ts
+12 -12
View File
@@ -1,7 +1,7 @@
import { LicenseState } from '@n8n/backend-common';
import { mockInstance } from '@n8n/backend-test-utils';
import type { InboxVisibility, User, WorkflowReviewRequest } from '@n8n/db';
import { WorkflowReviewRequestRepository, WorkflowReviewRequestWorkflowRepository } from '@n8n/db';
import { WorkflowReviewInboxRepository, WorkflowReviewRequestWorkflowRepository } from '@n8n/db';
import { mock } from 'vitest-mock-extended';
import type { WorkflowReviewAuthorizationService } from '../workflow-review-authorization.service';
@@ -19,7 +19,7 @@ describe('WorkflowReviewInboxService', () => {
const workflowReviewPolicyService = mockInstance(WorkflowReviewPolicyService);
const authorizationService = mock<WorkflowReviewAuthorizationService>();
const workflowHistoryService = mock<WorkflowHistoryService>();
const workflowReviewRequestRepository = mockInstance(WorkflowReviewRequestRepository);
const workflowReviewInboxRepository = mockInstance(WorkflowReviewInboxRepository);
const workflowReviewRequestWorkflowRepository = mockInstance(
WorkflowReviewRequestWorkflowRepository,
);
@@ -47,7 +47,7 @@ describe('WorkflowReviewInboxService', () => {
new WorkflowReviewFeatureGate(licenseState, workflowReviewPolicyService),
authorizationService,
workflowHistoryService,
workflowReviewRequestRepository,
workflowReviewInboxRepository,
workflowReviewRequestWorkflowRepository,
participantResolver,
);
@@ -88,14 +88,14 @@ describe('WorkflowReviewInboxService', () => {
updatedAt: new Date('2024-01-01T00:00:00.000Z'),
}),
];
workflowReviewRequestRepository.findManyForInbox.mockResolvedValue(rows);
workflowReviewInboxRepository.findRequests.mockResolvedValue(rows);
workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
new Map([['req-2', { workflowName: 'Linked workflow', workflowVersionId: 'ver-2' }]]),
);
const result = await service.listForInbox(user, { limit: 1 });
expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith({
expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith({
visibility: involvedVisibility,
state: 'open',
limit: 2,
@@ -116,7 +116,7 @@ describe('WorkflowReviewInboxService', () => {
it('decodes the incoming cursor into a keyset boundary', async () => {
mockVisibility();
workflowReviewRequestRepository.findManyForInbox.mockResolvedValue([]);
workflowReviewInboxRepository.findRequests.mockResolvedValue([]);
workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
new Map(),
);
@@ -124,7 +124,7 @@ describe('WorkflowReviewInboxService', () => {
await service.listForInbox(user, { limit: 15, cursor });
expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith(
expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
expect.objectContaining({
cursor: { createdAt: new Date('2024-01-02T00:00:00.000Z'), id: 'req-2' },
}),
@@ -142,7 +142,7 @@ describe('WorkflowReviewInboxService', () => {
describe('category', () => {
beforeEach(() => {
workflowReviewRequestRepository.findManyForInbox.mockResolvedValue([]);
workflowReviewInboxRepository.findRequests.mockResolvedValue([]);
workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
new Map(),
);
@@ -155,7 +155,7 @@ describe('WorkflowReviewInboxService', () => {
await service.listForInbox(user, { limit: 15, category });
expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith(
expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
expect.objectContaining({ category: { userId: 'user-1', category } }),
);
},
@@ -167,7 +167,7 @@ describe('WorkflowReviewInboxService', () => {
await service.listForInbox(otherUser, { limit: 15, category: 'authored' });
expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith(
expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
expect.objectContaining({ category: { userId: 'user-2', category: 'authored' } }),
);
});
@@ -177,7 +177,7 @@ describe('WorkflowReviewInboxService', () => {
await service.listForInbox(user, { limit: 15 });
expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith(
expect(workflowReviewInboxRepository.findRequests).toHaveBeenCalledWith(
expect.objectContaining({ category: undefined }),
);
});
@@ -197,7 +197,7 @@ describe('WorkflowReviewInboxService', () => {
beforeEach(() => {
authorizationService.resolveInboxVisibility.mockResolvedValue(involvedVisibility);
workflowReviewRequestRepository.findManyForInbox.mockResolvedValue([inboxRow]);
workflowReviewInboxRepository.findRequests.mockResolvedValue([inboxRow]);
workflowReviewRequestWorkflowRepository.findLinkedWorkflowsByRequestIds.mockResolvedValue(
new Map(),
);
packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-lifecycle.integration.test.ts
+4 -1
View File
@@ -14,6 +14,7 @@ import {
UserRepository,
WorkflowRepository,
WorkflowReviewActivityRepository,
WorkflowReviewLifecycleRepository,
WorkflowReviewRequestAuthorRepository,
WorkflowReviewRequestRepository,
WorkflowReviewRequestWorkflowRepository,
@@ -65,6 +66,7 @@ let ownerProject: Project;
let ownerAgent: SuperAgentTest;
let requestRepository: WorkflowReviewRequestRepository;
let lifecycleRepository: WorkflowReviewLifecycleRepository;
let linkRepository: WorkflowReviewRequestWorkflowRepository;
let authorRepository: WorkflowReviewRequestAuthorRepository;
let activityRepository: WorkflowReviewActivityRepository;
@@ -72,6 +74,7 @@ let activityRepository: WorkflowReviewActivityRepository;
beforeAll(async () => {
await utils.initNodeTypes();
requestRepository = Container.get(WorkflowReviewRequestRepository);
lifecycleRepository = Container.get(WorkflowReviewLifecycleRepository);
linkRepository = Container.get(WorkflowReviewRequestWorkflowRepository);
authorRepository = Container.get(WorkflowReviewRequestAuthorRepository);
activityRepository = Container.get(WorkflowReviewActivityRepository);
@@ -363,7 +366,7 @@ describe('auto-close on workflow hard delete', () => {
test('a failed capture degrades to a sweep close without a cause entry', async () => {
const { workflow, versionId } = await createReviewableWorkflow({ isArchived: true });
const request = await createOpenReview(workflow.id, versionId);
vi.spyOn(requestRepository, 'findOpenRequestsForWorkflows').mockRejectedValueOnce(
vi.spyOn(lifecycleRepository, 'findOpenRequestsAffectedByWorkflows').mockRejectedValueOnce(
new Error('read failed'),
);
packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-lifecycle.service.test.ts
+63 -38
View File
@@ -6,6 +6,7 @@ import type {
Transaction,
WorkflowReviewActivity,
WorkflowReviewActivityRepository,
WorkflowReviewLifecycleRepository,
WorkflowReviewRequest,
WorkflowReviewRequestRepository,
WorkflowReviewRequestWorkflowRepository,
@@ -21,6 +22,7 @@ import { WorkflowReviewStateNotifier } from '../workflow-review-state-notifier.s
describe('WorkflowReviewLifecycleService', () => {
const logger = mock<Logger>();
const requestRepository = mock<WorkflowReviewRequestRepository>();
const lifecycleRepository = mock<WorkflowReviewLifecycleRepository>();
const requestWorkflowRepository = mock<WorkflowReviewRequestWorkflowRepository>();
const activityRepository = mock<WorkflowReviewActivityRepository>();
const dbLockService = mock<DbLockService>();
@@ -46,6 +48,7 @@ describe('WorkflowReviewLifecycleService', () => {
service = new WorkflowReviewLifecycleService(
logger,
requestRepository,
lifecycleRepository,
requestWorkflowRepository,
activityRepository,
dbLockService,
@@ -54,7 +57,7 @@ describe('WorkflowReviewLifecycleService', () => {
);
dbLockService.withLockContext.mockImplementation(async (_id, fn) => await fn(ctx));
requestRepository.closeRequests.mockResolvedValue(undefined);
requestRepository.findUnreviewableOpenRequestIds.mockImplementation(
lifecycleRepository.findUnreviewableOpenRequestIds.mockImplementation(
async (_ctx, candidateRequestIds) => candidateRequestIds ?? [],
);
activityRepository.createActivity.mockResolvedValue(mock<WorkflowReviewActivity>());
@@ -64,7 +67,7 @@ describe('WorkflowReviewLifecycleService', () => {
describe('archive', () => {
it('records the cause entry and the close entry together, in the lock transaction', async () => {
const request = openRequest();
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request, links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
@@ -74,7 +77,10 @@ describe('WorkflowReviewLifecycleService', () => {
DbLock.WORKFLOW_REVIEW_MUTATION,
expect.any(Function),
);
expect(requestRepository.findOpenRequestsForWorkflows).toHaveBeenCalledWith(['wf-1'], ctx);
expect(lifecycleRepository.findOpenRequestsAffectedByWorkflows).toHaveBeenCalledWith(
['wf-1'],
ctx,
);
expect(activityRepository.createActivity).toHaveBeenCalledWith(
{
workflowReviewRequestId: 'req-1',
@@ -94,7 +100,9 @@ describe('WorkflowReviewLifecycleService', () => {
ctx,
);
// Evaluate the linked request, then close it by ID.
expect(requestRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, ['req-1']);
expect(lifecycleRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, [
'req-1',
]);
expect(requestRepository.closeRequests).toHaveBeenCalledWith(['req-1'], ctx);
expect(
collaborationService.broadcastWorkflowReviewStateChanged,
@@ -106,7 +114,7 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('attributes a system archive (no user) as a system actor', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{
request: openRequest(),
links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }],
@@ -130,15 +138,17 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('leaves the request open while a reviewable workflow remains outside the affected set', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request: openRequest(), links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
// Keep the request open while it has a reviewable workflow.
requestRepository.findUnreviewableOpenRequestIds.mockResolvedValue([]);
lifecycleRepository.findUnreviewableOpenRequestIds.mockResolvedValue([]);
await service.afterWorkflowArchived('wf-1', 'user-9');
expect(requestRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, ['req-1']);
expect(lifecycleRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, [
'req-1',
]);
// The cause entry is still recorded; only the close is withheld.
expect(activityRepository.createActivity).toHaveBeenCalledExactlyOnceWith(
expect.objectContaining({ type: 'workflow.archived' }),
@@ -149,7 +159,7 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('does nothing when no open request is linked — no write, no broadcast', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([]);
await service.afterWorkflowArchived('wf-1', 'user-9');
@@ -160,7 +170,9 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('swallows and logs repository errors instead of failing the workflow mutation', async () => {
requestRepository.findOpenRequestsForWorkflows.mockRejectedValue(new Error('db down'));
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockRejectedValue(
new Error('db down'),
);
await expect(service.afterWorkflowArchived('wf-1', 'user-9')).resolves.toBeUndefined();
@@ -170,11 +182,11 @@ describe('WorkflowReviewLifecycleService', () => {
// Reporting happens after commit, so listener failures cannot undo the archive.
it('archives anyway when reporting the close throws, and still runs the sweep', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request: openRequest(), links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
// Reconciliation also finds req-9.
requestRepository.findUnreviewableOpenRequestIds.mockImplementation(
lifecycleRepository.findUnreviewableOpenRequestIds.mockImplementation(
async (_ctx, ids) => ids ?? ['req-9'],
);
eventService.emit.mockImplementation(() => {
@@ -193,7 +205,7 @@ describe('WorkflowReviewLifecycleService', () => {
it('records workflow.moved for each open request and broadcasts once per affected workflow', async () => {
const first = openRequest({ id: 'req-1' });
const second = openRequest({ id: 'req-2' });
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{
request: first,
links: [
@@ -206,7 +218,7 @@ describe('WorkflowReviewLifecycleService', () => {
await service.afterWorkflowsTransferred(['wf-1', 'wf-2', 'wf-3'], 'user-9');
expect(requestRepository.findOpenRequestsForWorkflows).toHaveBeenCalledWith(
expect(lifecycleRepository.findOpenRequestsAffectedByWorkflows).toHaveBeenCalledWith(
['wf-1', 'wf-2', 'wf-3'],
ctx,
);
@@ -238,7 +250,9 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('swallows repository errors on transfer too — the move already committed', async () => {
requestRepository.findOpenRequestsForWorkflows.mockRejectedValue(new Error('db down'));
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockRejectedValue(
new Error('db down'),
);
await expect(service.afterWorkflowsTransferred(['wf-1'], 'user-9')).resolves.toBeUndefined();
@@ -248,7 +262,7 @@ describe('WorkflowReviewLifecycleService', () => {
describe('delete', () => {
it('captures before the delete without writing anything', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{
request: openRequest(),
links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }],
@@ -257,7 +271,10 @@ describe('WorkflowReviewLifecycleService', () => {
await service.beforeWorkflowDeleted('wf-1', 'user-9');
expect(requestRepository.findOpenRequestsForWorkflows).toHaveBeenCalledWith(['wf-1'], {});
expect(lifecycleRepository.findOpenRequestsAffectedByWorkflows).toHaveBeenCalledWith(
['wf-1'],
{},
);
expect(activityRepository.createActivity).not.toHaveBeenCalled();
expect(requestRepository.closeRequests).not.toHaveBeenCalled();
expect(dbLockService.withLockContext).not.toHaveBeenCalled();
@@ -265,7 +282,9 @@ describe('WorkflowReviewLifecycleService', () => {
// Review bookkeeping must not block deletion. Reconciliation handles missed captures.
it('never throws from the capture, even when the repository fails', async () => {
requestRepository.findOpenRequestsForWorkflows.mockRejectedValue(new Error('db down'));
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockRejectedValue(
new Error('db down'),
);
await expect(service.beforeWorkflowDeleted('wf-1', 'user-9')).resolves.toBeUndefined();
@@ -274,7 +293,7 @@ describe('WorkflowReviewLifecycleService', () => {
it('records the captured deletion and closes, after the delete committed', async () => {
const request = openRequest();
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request, links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
await service.beforeWorkflowDeleted('wf-1', 'user-9');
@@ -307,9 +326,11 @@ describe('WorkflowReviewLifecycleService', () => {
it('evaluates a batch as one affected set: per-workflow cause entries, one close', async () => {
const request = openRequest();
requestRepository.findOpenRequestsForWorkflows.mockImplementation(async ([workflowId]) => [
{ request, links: [{ workflowId, workflowVersionId: `wfv-${workflowId}` }] },
]);
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockImplementation(
async ([workflowId]) => [
{ request, links: [{ workflowId, workflowVersionId: `wfv-${workflowId}` }] },
],
);
await service.beforeWorkflowDeleted('wf-1', 'user-9');
await service.beforeWorkflowDeleted('wf-2', 'user-9');
requestRepository.findById.mockResolvedValue(request);
@@ -331,7 +352,9 @@ describe('WorkflowReviewLifecycleService', () => {
ctx,
);
// Evaluate and close the request once.
expect(requestRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, ['req-1']);
expect(lifecycleRepository.findUnreviewableOpenRequestIds).toHaveBeenCalledWith(ctx, [
'req-1',
]);
expect(
activityRepository.createActivity.mock.calls.filter(
([input]) => input.type === 'review.closed',
@@ -345,7 +368,7 @@ describe('WorkflowReviewLifecycleService', () => {
it('records nothing for a request that closed between capture and delete', async () => {
const request = openRequest();
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request, links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
await service.beforeWorkflowDeleted('wf-1', 'user-9');
@@ -359,7 +382,7 @@ describe('WorkflowReviewLifecycleService', () => {
it('consumes the capture: a second after-hook for the same workflow records nothing', async () => {
const request = openRequest();
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{ request, links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }] },
]);
await service.beforeWorkflowDeleted('wf-1', 'user-9');
@@ -376,7 +399,7 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('degrades to the sweep when nothing was captured', async () => {
requestRepository.findUnreviewableOpenRequestIds.mockResolvedValue(['req-9']);
lifecycleRepository.findUnreviewableOpenRequestIds.mockResolvedValue(['req-9']);
await service.afterWorkflowsDeleted(['wf-1']);
@@ -394,7 +417,7 @@ describe('WorkflowReviewLifecycleService', () => {
// The delete is already committed and cannot be undone here.
it('swallows repository errors after a delete', async () => {
requestRepository.findUnreviewableOpenRequestIds.mockRejectedValue(new Error('db down'));
lifecycleRepository.findUnreviewableOpenRequestIds.mockRejectedValue(new Error('db down'));
await expect(service.afterWorkflowsDeleted(['wf-1', 'wf-2'])).resolves.toBeUndefined();
@@ -468,9 +491,9 @@ describe('WorkflowReviewLifecycleService', () => {
describe('reconciliation sweep', () => {
it('closes the requests the mutation stranded and explains each of them', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([]);
// Reconciliation finds req-9 and req-10.
requestRepository.findUnreviewableOpenRequestIds.mockImplementation(
lifecycleRepository.findUnreviewableOpenRequestIds.mockImplementation(
async (_ctx, ids) => ids ?? ['req-9', 'req-10'],
);
@@ -502,7 +525,7 @@ describe('WorkflowReviewLifecycleService', () => {
});
it('stays quiet when nothing is left unreviewable', async () => {
requestRepository.findUnreviewableOpenRequestIds.mockResolvedValue([]);
lifecycleRepository.findUnreviewableOpenRequestIds.mockResolvedValue([]);
await service.afterWorkflowsDeleted(['wf-1']);
@@ -512,7 +535,7 @@ describe('WorkflowReviewLifecycleService', () => {
// Activity and closing share a transaction, so both roll back on failure.
it('leaves a review it cannot explain to the next sweep', async () => {
requestRepository.findUnreviewableOpenRequestIds.mockResolvedValue(['req-9']);
lifecycleRepository.findUnreviewableOpenRequestIds.mockResolvedValue(['req-9']);
activityRepository.createActivity.mockRejectedValue(new Error('db down'));
await expect(service.afterWorkflowsDeleted(['wf-1'])).resolves.toBeUndefined();
@@ -525,8 +548,8 @@ describe('WorkflowReviewLifecycleService', () => {
// Reconciliation retries closes that rolled back after an archive or move committed.
it('runs after the targeted close on archive, and again on transfer', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
requestRepository.findUnreviewableOpenRequestIds.mockImplementation(
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([]);
lifecycleRepository.findUnreviewableOpenRequestIds.mockImplementation(
async (_ctx, ids) => ids ?? ['req-9'],
);
@@ -542,8 +565,10 @@ describe('WorkflowReviewLifecycleService', () => {
// A failed targeted close must not skip reconciliation.
it('still runs when the targeted close on archive failed', async () => {
requestRepository.findOpenRequestsForWorkflows.mockRejectedValue(new Error('db down'));
requestRepository.findUnreviewableOpenRequestIds.mockImplementation(
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockRejectedValue(
new Error('db down'),
);
lifecycleRepository.findUnreviewableOpenRequestIds.mockImplementation(
async (_ctx, ids) => ids ?? ['req-9'],
);
@@ -557,16 +582,16 @@ describe('WorkflowReviewLifecycleService', () => {
// The pre-delete hook only captures; reconciliation waits for the delete to commit.
it('does not run before a delete', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([]);
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([]);
await service.beforeWorkflowDeleted('wf-1', 'user-9');
expect(requestRepository.findUnreviewableOpenRequestIds).not.toHaveBeenCalled();
expect(lifecycleRepository.findUnreviewableOpenRequestIds).not.toHaveBeenCalled();
});
});
it('a failed broadcast is only warned about, never thrown', async () => {
requestRepository.findOpenRequestsForWorkflows.mockResolvedValue([
lifecycleRepository.findOpenRequestsAffectedByWorkflows.mockResolvedValue([
{
request: openRequest(),
links: [{ workflowId: 'wf-1', workflowVersionId: 'wfv-1' }],
packages/cli/src/modules/workflow-reviews.ee/workflow-review-inbox.service.ts
+4 -4
View File
@@ -8,7 +8,7 @@ import type {
WorkflowReviewVersionSnapshot,
} from '@n8n/api-types';
import {
WorkflowReviewRequestRepository,
WorkflowReviewInboxRepository,
WorkflowReviewRequestWorkflowRepository,
type InboxCursor,
type User,
@@ -41,7 +41,7 @@ export class WorkflowReviewInboxService {
private readonly featureGate: WorkflowReviewFeatureGate,
private readonly authorizationService: WorkflowReviewAuthorizationService,
private readonly workflowHistoryService: WorkflowHistoryService,
private readonly workflowReviewRequestRepository: WorkflowReviewRequestRepository,
private readonly workflowReviewInboxRepository: WorkflowReviewInboxRepository,
private readonly workflowReviewRequestWorkflowRepository: WorkflowReviewRequestWorkflowRepository,
private readonly participantResolver: WorkflowReviewParticipantResolver,
) {}
@@ -54,7 +54,7 @@ export class WorkflowReviewInboxService {
const visibility = await this.authorizationService.resolveInboxVisibility(user);
const { limit } = query;
const rows = await this.workflowReviewRequestRepository.findManyForInbox({
const rows = await this.workflowReviewInboxRepository.findRequests({
visibility,
state: query.state ?? 'open',
category:
@@ -91,7 +91,7 @@ export class WorkflowReviewInboxService {
await this.featureGate.assertAvailable();
const visibility = await this.authorizationService.resolveInboxVisibility(user);
return await this.workflowReviewRequestRepository.countByStateForInbox({ visibility });
return await this.workflowReviewInboxRepository.countRequestsByState(visibility);
}
async getDetail(
packages/cli/src/modules/workflow-reviews.ee/workflow-review-lifecycle.service.ts
+13 -9
View File
@@ -5,6 +5,7 @@ import {
DbLock,
DbLockService,
WorkflowReviewActivityRepository,
WorkflowReviewLifecycleRepository,
WorkflowReviewRequestRepository,
WorkflowReviewRequestWorkflowRepository,
} from '@n8n/db';
@@ -50,6 +51,7 @@ export class WorkflowReviewLifecycleService implements WorkflowMutationHooks {
constructor(
private readonly logger: Logger,
private readonly workflowReviewRequestRepository: WorkflowReviewRequestRepository,
private readonly workflowReviewLifecycleRepository: WorkflowReviewLifecycleRepository,
private readonly workflowReviewRequestWorkflowRepository: WorkflowReviewRequestWorkflowRepository,
private readonly activityRepository: WorkflowReviewActivityRepository,
private readonly dbLockService: DbLockService,
@@ -70,10 +72,11 @@ export class WorkflowReviewLifecycleService implements WorkflowMutationHooks {
/** Capture request data before deletion without writing activity or blocking the delete. */
async beforeWorkflowDeleted(workflowId: string, userId: string | null): Promise<void> {
try {
const openRequests = await this.workflowReviewRequestRepository.findOpenRequestsForWorkflows(
[workflowId],
{},
);
const openRequests =
await this.workflowReviewLifecycleRepository.findOpenRequestsAffectedByWorkflows(
[workflowId],
{},
);
// Keep only the latest state captured before deletion.
this.pendingDeleteCaptures.delete(workflowId);
@@ -205,10 +208,11 @@ export class WorkflowReviewLifecycleService implements WorkflowMutationHooks {
await this.recordCauseEventsAndClose(type, workflowIds, async (ctx) => {
// Read under the lock to avoid racing a decision or version update.
const openRequests = await this.workflowReviewRequestRepository.findOpenRequestsForWorkflows(
workflowIds,
ctx,
);
const openRequests =
await this.workflowReviewLifecycleRepository.findOpenRequestsAffectedByWorkflows(
workflowIds,
ctx,
);
const affected = new Set<string>();
const candidateRequestIds: string[] = [];
@@ -326,7 +330,7 @@ export class WorkflowReviewLifecycleService implements WorkflowMutationHooks {
candidateRequestIds?: string[],
): Promise<string[]> {
const closableRequestIds =
await this.workflowReviewRequestRepository.findUnreviewableOpenRequestIds(
await this.workflowReviewLifecycleRepository.findUnreviewableOpenRequestIds(
ctx,
candidateRequestIds,
);