From b2cbcc1b8199ad2e649d6497262a49e1a09dabd8 Mon Sep 17 00:00:00 2001 From: Sandra Zollner Date: Thu, 23 Jul 2026 09:40:36 +0000 Subject: [PATCH] feat(core): Create reviews page and render inbox (#34357) --- packages/@n8n/api-types/src/dto/index.ts | 1 + .../list-workflow-review-inbox.dto.ts | 39 +++ ...workflow-review-request.repository.test.ts | 165 +++++++++++ packages/@n8n/db/src/repositories/index.ts | 5 +- ...flow-review-request-workflow.repository.ts | 18 ++ .../workflow-review-request.repository.ts | 95 +++++- ...rkflow-review-request.list.service.test.ts | 194 ++++++++++++ .../workflow-review-request.service.test.ts | 9 +- ...ew-requests.controller.integration.test.ts | 240 ++++++++++++++- ...view-requests.env-gate.integration.test.ts | 12 +- .../workflow-review-request.service.ts | 130 +++++++- .../workflow-review-requests.controller.ts | 28 +- .../src/components/N8nTabs/Tabs.vue | 7 +- .../frontend/@n8n/i18n/src/locales/en.json | 10 + .../editor-ui/src/app/modules.manifest.ts | 2 + .../projects/components/ProjectNavigation.vue | 18 ++ .../projects/composables/useProjectPages.ts | 5 + .../collaboration/projects/projects.store.ts | 5 + .../WorkflowReviewRequestsSidebar.vue | 278 ++++++++++++++++++ .../features/workflow-reviews/constants.ts | 1 + .../workflow-reviews/module.descriptor.ts | 34 +++ .../reviewInbox.store.test.ts | 206 +++++++++++++ .../workflow-reviews/reviewInbox.store.ts | 202 +++++++++++++ .../views/WorkflowReviewRequestsView.test.ts | 144 +++++++++ .../views/WorkflowReviewRequestsView.vue | 183 ++++++++++++ .../workflow-reviews/workflowReviews.api.ts | 23 ++ 26 files changed, 2044 insertions(+), 10 deletions(-) create mode 100644 packages/@n8n/api-types/src/dto/workflow-reviews/list-workflow-review-inbox.dto.ts create mode 100644 packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.list.service.test.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/constants.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/module.descriptor.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.test.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.test.ts create mode 100644 packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index aee2d68f378..935ea9fb99d 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -295,6 +295,7 @@ export type { EncryptionKeyResponseDto } from './encryption/encryption-key-respo export { CreateWorkflowReviewRequestDto } from './workflow-reviews/create-workflow-review-request.dto'; export { GetWorkflowReviewEligibleReviewersQueryDto } from './workflow-reviews/get-eligible-reviewers-query.dto'; export { ListWorkflowReviewRequestsQueryDto } from './workflow-reviews/list-workflow-review-requests-query.dto'; +export * from './workflow-reviews/list-workflow-review-inbox.dto'; export { UpdateOtelSettingsDto } from './otel/update-otel-settings.dto'; export { TestOtelTraceDto } from './otel/test-otel-trace.dto'; diff --git a/packages/@n8n/api-types/src/dto/workflow-reviews/list-workflow-review-inbox.dto.ts b/packages/@n8n/api-types/src/dto/workflow-reviews/list-workflow-review-inbox.dto.ts new file mode 100644 index 00000000000..fb0f35c3d49 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/workflow-reviews/list-workflow-review-inbox.dto.ts @@ -0,0 +1,39 @@ +import { z } from 'zod'; + +import { + type WorkflowReviewRequestSummary, + workflowReviewRequestStateSchema, +} from '../../workflow-review-request-summary'; +import { Z } from '../../zod-class'; + +/** + * Inbox list item — extends {@link WorkflowReviewRequestSummary} (the + * workflow-scoped list payload used by the review-required toggle sync) with the + * cross-project fields the inbox needs. + */ +export interface WorkflowReviewInboxItem extends WorkflowReviewRequestSummary { + projectId: string; + title: string; + workflowName: string | null; +} + +/** + * Inbox query. Defaults to open requests when `state` is omitted. + * Deferred (LIGO-594): `projectId`, `reviewer`, and `author` filters. + */ +export class ListWorkflowReviewInboxQueryDto extends Z.class({ + limit: z.coerce.number().int().min(1).max(100).default(15), + // Opaque base64url keyset cursor (ISO createdAt + id), not a bare UUID. + cursor: z.string().min(1).max(256).optional(), + state: workflowReviewRequestStateSchema.optional(), +}) {} + +export interface ListWorkflowReviewInboxResponse { + data: WorkflowReviewInboxItem[]; + nextCursor: string | null; + hasMore: boolean; +} + +export interface GetWorkflowReviewInboxSummaryResponse { + hasAny: boolean; +} diff --git a/packages/@n8n/db/src/repositories/__tests__/workflow-review-request.repository.test.ts b/packages/@n8n/db/src/repositories/__tests__/workflow-review-request.repository.test.ts index f8d3eac05e0..1113815ed0c 100644 --- a/packages/@n8n/db/src/repositories/__tests__/workflow-review-request.repository.test.ts +++ b/packages/@n8n/db/src/repositories/__tests__/workflow-review-request.repository.test.ts @@ -11,8 +11,23 @@ describe('WorkflowReviewRequestRepository', () => { const entityManager = mockEntityManager(WorkflowReviewRequest); const repo = Container.get(WorkflowReviewRequestRepository); + let queryBuilder: Mocked>; + beforeEach(() => { vi.resetAllMocks(); + + queryBuilder = mock>(); + queryBuilder.where.mockReturnThis(); + queryBuilder.andWhere.mockReturnThis(); + queryBuilder.orderBy.mockReturnThis(); + queryBuilder.addOrderBy.mockReturnThis(); + queryBuilder.select.mockReturnThis(); + queryBuilder.take.mockReturnThis(); + queryBuilder.limit.mockReturnThis(); + queryBuilder.getMany.mockResolvedValue([]); + queryBuilder.getRawOne.mockResolvedValue(undefined); + + vi.spyOn(repo, 'createQueryBuilder').mockReturnValue(queryBuilder); }); describe('createRequest', () => { @@ -120,4 +135,154 @@ describe('WorkflowReviewRequestRepository', () => { expect(queryBuilder.take).toHaveBeenCalledWith(0); }); }); + + describe('findManyForInbox', () => { + it('filters by requester only when projectIds is empty', async () => { + const rows = [mock({ id: 'req-1' })]; + queryBuilder.getMany.mockResolvedValueOnce(rows); + + const result = await repo.findManyForInbox({ + projectIds: [], + requesterId: 'user-1', + limit: 15, + }); + + expect(result).toBe(rows); + expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); + expect(queryBuilder.where).toHaveBeenCalledWith('review.createdById = :requesterId', { + requesterId: 'user-1', + }); + expect(queryBuilder.take).toHaveBeenCalledWith(15); + }); + + it('skips the projectId filter when projectIds is null', async () => { + const rows = [mock({ id: 'req-1' })]; + queryBuilder.getMany.mockResolvedValueOnce(rows); + + const result = await repo.findManyForInbox({ + projectIds: null, + requesterId: 'user-1', + state: 'open', + limit: 15, + }); + + expect(result).toBe(rows); + expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); + expect(queryBuilder.where).not.toHaveBeenCalled(); + expect(queryBuilder.orderBy).toHaveBeenCalledWith('review.createdAt', 'DESC'); + expect(queryBuilder.addOrderBy).toHaveBeenCalledWith('review.id', 'ASC'); + expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.state = :state', { + state: 'open', + }); + expect(queryBuilder.take).toHaveBeenCalledWith(15); + }); + + it('matches projects OR the requester, filtering by state and ordering newest first', async () => { + const rows = [mock({ id: 'req-1' })]; + queryBuilder.getMany.mockResolvedValueOnce(rows); + + const result = await repo.findManyForInbox({ + projectIds: ['proj-1', 'proj-2'], + requesterId: 'user-1', + state: 'open', + limit: 15, + }); + + expect(result).toBe(rows); + expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); + expect(queryBuilder.where).toHaveBeenCalledWith( + '(review.projectId IN (:...projectIds) OR review.createdById = :requesterId)', + { projectIds: ['proj-1', 'proj-2'], requesterId: 'user-1' }, + ); + expect(queryBuilder.orderBy).toHaveBeenCalledWith('review.createdAt', 'DESC'); + expect(queryBuilder.addOrderBy).toHaveBeenCalledWith('review.id', 'ASC'); + expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.state = :state', { + state: 'open', + }); + expect(queryBuilder.take).toHaveBeenCalledWith(15); + }); + + it('applies the keyset boundary carried in the cursor without an anchor lookup', async () => { + const findOneSpy = vi.spyOn(repo, 'findOne'); + queryBuilder.getMany.mockResolvedValueOnce([]); + const createdAt = new Date('2024-01-02T00:00:00.000Z'); + + await repo.findManyForInbox({ + projectIds: ['proj-1'], + requesterId: 'user-1', + limit: 10, + cursor: { createdAt, id: 'req-cursor' }, + }); + + expect(findOneSpy).not.toHaveBeenCalled(); + expect(queryBuilder.andWhere).toHaveBeenCalledWith( + '(review.createdAt < :createdAt OR (review.createdAt = :createdAt AND review.id > :id))', + { createdAt, id: 'req-cursor' }, + ); + }); + }); + + describe('existsAnyForInbox', () => { + it('queries by requester only when projectIds is empty', async () => { + queryBuilder.getRawOne.mockResolvedValueOnce({ '?column?': 1 }); + + const result = await repo.existsAnyForInbox({ projectIds: [], requesterId: 'user-1' }); + + expect(result).toBe(true); + expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); + expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(queryBuilder.where).toHaveBeenCalledWith('review.createdById = :requesterId', { + requesterId: 'user-1', + }); + expect(queryBuilder.limit).toHaveBeenCalledWith(1); + }); + + it('skips the projectId filter when projectIds is null', async () => { + queryBuilder.getRawOne.mockResolvedValueOnce({ '?column?': 1 }); + + const result = await repo.existsAnyForInbox({ + projectIds: null, + requesterId: 'user-1', + state: 'open', + }); + + expect(result).toBe(true); + expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); + expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(queryBuilder.where).not.toHaveBeenCalled(); + expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.state = :state', { + state: 'open', + }); + expect(queryBuilder.limit).toHaveBeenCalledWith(1); + }); + + it('returns true when at least one matching row exists', async () => { + queryBuilder.getRawOne.mockResolvedValueOnce({ '?column?': 1 }); + + const result = await repo.existsAnyForInbox({ + projectIds: ['proj-1'], + requesterId: 'user-1', + state: 'open', + }); + + expect(result).toBe(true); + expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(queryBuilder.where).toHaveBeenCalledWith( + '(review.projectId IN (:...projectIds) OR review.createdById = :requesterId)', + { projectIds: ['proj-1'], requesterId: 'user-1' }, + ); + expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.state = :state', { + state: 'open', + }); + expect(queryBuilder.limit).toHaveBeenCalledWith(1); + }); + + it('returns false when no rows match', async () => { + queryBuilder.getRawOne.mockResolvedValueOnce(undefined); + + await expect( + repo.existsAnyForInbox({ projectIds: ['proj-1'], requesterId: 'user-1' }), + ).resolves.toBe(false); + }); + }); }); diff --git a/packages/@n8n/db/src/repositories/index.ts b/packages/@n8n/db/src/repositories/index.ts index d81b1bcc260..f4692947439 100644 --- a/packages/@n8n/db/src/repositories/index.ts +++ b/packages/@n8n/db/src/repositories/index.ts @@ -71,7 +71,10 @@ export { type PublishedWorkflowDataForExecution, } from './workflow-published-version.repository'; export { WorkflowPublishHistoryRepository } from './workflow-publish-history.repository'; -export { WorkflowReviewRequestRepository } from './workflow-review-request.repository'; +export { + WorkflowReviewRequestRepository, + type InboxCursor, +} from './workflow-review-request.repository'; export { WorkflowReviewRequestWorkflowRepository } from './workflow-review-request-workflow.repository'; export { WorkflowReviewRequestReviewerRepository } from './workflow-review-request-reviewer.repository'; export { WorkflowReviewRequestAuthorRepository } from './workflow-review-request-author.repository'; diff --git a/packages/@n8n/db/src/repositories/workflow-review-request-workflow.repository.ts b/packages/@n8n/db/src/repositories/workflow-review-request-workflow.repository.ts index 97f4150d0a5..ad737318e29 100644 --- a/packages/@n8n/db/src/repositories/workflow-review-request-workflow.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-review-request-workflow.repository.ts @@ -2,6 +2,7 @@ import { Service } from '@n8n/di'; import type { EntityManager } from '@n8n/typeorm'; import { DataSource, Repository } from '@n8n/typeorm'; +import { WorkflowEntity } from '../entities/workflow-entity'; import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee'; @Service() @@ -36,4 +37,21 @@ export class WorkflowReviewRequestWorkflowRepository extends Repository> { + if (requestIds.length === 0) { + return new Map(); + } + + // Join via entity so DB_TABLE_PREFIX is applied (postgres ITs). + const rows = await this.createQueryBuilder('wrw') + .innerJoin(WorkflowEntity, 'workflow', 'workflow.id = wrw.workflowId') + .select('wrw.workflowReviewRequestId', 'requestId') + .addSelect('workflow.name', 'workflowName') + .where('wrw.workflowReviewRequestId IN (:...requestIds)', { requestIds }) + .getRawMany<{ requestId: string; workflowName: string }>(); + + return new Map(rows.map((row) => [row.requestId, row.workflowName])); + } } diff --git a/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts b/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts index d1fcb78c868..5bad98e487f 100644 --- a/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts @@ -1,5 +1,5 @@ import { Service } from '@n8n/di'; -import type { EntityManager } from '@n8n/typeorm'; +import type { EntityManager, SelectQueryBuilder } from '@n8n/typeorm'; import { DataSource, Repository } from '@n8n/typeorm'; import { WorkflowReviewRequestWorkflow } from '../entities/workflow-review-request-workflow.ee'; @@ -9,6 +9,33 @@ import { type WorkflowReviewRequestState, } from '../entities/workflow-review-request.ee'; +/** + * 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; +}; + +export type FindManyForInboxOptions = { + /** `null` means all projects (no filter); `[]` means no publish-scoped projects. */ + projectIds: string[] | null; + /** Requesters always see the reviews they created, regardless of project scope. */ + requesterId: string; + state?: WorkflowReviewRequestState; + limit: number; + cursor?: InboxCursor; +}; + +export type ExistsAnyForInboxOptions = { + /** `null` means all projects (no filter); `[]` means no publish-scoped projects. */ + projectIds: string[] | null; + /** Requesters always see the reviews they created, regardless of project scope. */ + requesterId: string; + state?: WorkflowReviewRequestState; +}; + @Service() export class WorkflowReviewRequestRepository extends Repository { constructor(dataSource: DataSource) { @@ -95,4 +122,70 @@ export class WorkflowReviewRequestRepository extends Repository { + const { projectIds, requesterId, state, limit, cursor } = options; + + const queryBuilder = this.createQueryBuilder('review') + .orderBy('review.createdAt', 'DESC') + .addOrderBy('review.id', 'ASC'); + + this.applyInboxVisibility(queryBuilder, projectIds, requesterId); + + 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 existsAnyForInbox(options: ExistsAnyForInboxOptions): Promise { + const { projectIds, requesterId, state } = options; + + const queryBuilder = this.createQueryBuilder('review').select('1'); + + this.applyInboxVisibility(queryBuilder, projectIds, requesterId); + + if (state !== undefined) { + queryBuilder.andWhere('review.state = :state', { state }); + } + + const row = await queryBuilder.limit(1).getRawOne>(); + return row !== undefined; + } + + /** + * Inbox visibility: a review is visible if the caller is its requester OR it + * belongs to a project the caller can publish to. `projectIds === null` + * means global scope (no filter). An empty `projectIds` still matches the + * caller's own requests. + */ + private applyInboxVisibility( + queryBuilder: SelectQueryBuilder, + projectIds: string[] | null, + requesterId: string, + ): void { + if (projectIds === null) { + return; + } + + if (projectIds.length === 0) { + queryBuilder.where('review.createdById = :requesterId', { requesterId }); + return; + } + + queryBuilder.where( + '(review.projectId IN (:...projectIds) OR review.createdById = :requesterId)', + { projectIds, requesterId }, + ); + } } diff --git a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.list.service.test.ts b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.list.service.test.ts new file mode 100644 index 00000000000..14f35183921 --- /dev/null +++ b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.list.service.test.ts @@ -0,0 +1,194 @@ +import { mockInstance } from '@n8n/backend-test-utils'; +import { LicenseState, type Logger } from '@n8n/backend-common'; +import type { + DbLockService, + SharedWorkflowRepository, + User, + UserRepository, + WorkflowReviewRequest, + WorkflowReviewRequestReviewerRepository, +} from '@n8n/db'; +import { + WorkflowReviewRequestAuthorRepository, + WorkflowReviewRequestRepository, + WorkflowReviewRequestWorkflowRepository, +} from '@n8n/db'; +import { mock } from 'vitest-mock-extended'; + +import type { CollaborationService } from '@/collaboration/collaboration.service'; +import { ProjectService } from '@/services/project.service.ee'; +import type { RoleService } from '@/services/role.service'; +import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; +import type { WorkflowFinderService } from '@/workflows/workflow-finder.service'; +import type { WorkflowHistoryService } from '@/workflows/workflow-history/workflow-history.service'; + +import { WorkflowReviewRequestService } from '../workflow-review-request.service'; + +describe('WorkflowReviewRequestService list', () => { + const logger = mock(); + const workflowReviewPolicyService = mockInstance(WorkflowReviewPolicyService); + const workflowFinderService = mock(); + const workflowHistoryService = mock(); + const sharedWorkflowRepository = mock(); + const workflowReviewRequestRepository = mockInstance(WorkflowReviewRequestRepository); + const workflowReviewRequestWorkflowRepository = mockInstance( + WorkflowReviewRequestWorkflowRepository, + ); + const workflowReviewRequestAuthorRepository = mockInstance(WorkflowReviewRequestAuthorRepository); + const reviewerRepository = mock(); + const userRepository = mock(); + const roleService = mock(); + const projectService = mockInstance(ProjectService); + const licenseState = mockInstance(LicenseState); + const dbLockService = mock(); + const collaborationService = mock(); + + let service: WorkflowReviewRequestService; + + const user = mock({ id: 'user-1', role: { slug: 'global:member', scopes: [] } }); + + beforeEach(() => { + vi.resetAllMocks(); + process.env.N8N_ENV_FEAT_WORKFLOW_REVIEWS = 'true'; + licenseState.isWorkflowReviewsLicensed.mockReturnValue(true); + workflowReviewPolicyService.get.mockResolvedValue({ enabled: true }); + + service = new WorkflowReviewRequestService( + logger, + workflowReviewPolicyService, + workflowFinderService, + workflowHistoryService, + sharedWorkflowRepository, + workflowReviewRequestRepository, + workflowReviewRequestWorkflowRepository, + workflowReviewRequestAuthorRepository, + reviewerRepository, + userRepository, + roleService, + projectService, + licenseState, + dbLockService, + collaborationService, + ); + }); + + describe('listForInbox', () => { + function mockAccessibleProjects(projectIds: string[] = ['proj-1']) { + projectService.getProjectIdsWithScope.mockResolvedValueOnce(projectIds); + } + + it('returns paginated data with hasMore and nextCursor', async () => { + mockAccessibleProjects(); + const rows = [ + mock({ + id: 'req-2', + projectId: 'proj-1', + title: 'Second', + decision: 'pending', + state: 'open', + createdAt: new Date('2024-01-02T00:00:00.000Z'), + updatedAt: new Date('2024-01-02T00:00:00.000Z'), + }), + mock({ + id: 'req-1', + projectId: 'proj-1', + title: 'First', + decision: 'pending', + state: 'open', + createdAt: new Date('2024-01-01T00:00:00.000Z'), + updatedAt: new Date('2024-01-01T00:00:00.000Z'), + }), + ]; + workflowReviewRequestRepository.findManyForInbox.mockResolvedValue(rows); + workflowReviewRequestWorkflowRepository.findWorkflowNamesByRequestIds.mockResolvedValue( + new Map([['req-2', 'Linked workflow']]), + ); + + const result = await service.listForInbox(user, { limit: 1 }); + + expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith({ + projectIds: ['proj-1'], + requesterId: 'user-1', + state: 'open', + limit: 2, + cursor: undefined, + }); + expect(result.data).toHaveLength(1); + expect(result.data[0]?.workflowName).toBe('Linked workflow'); + expect(result.hasMore).toBe(true); + // nextCursor encodes the last row's keyset boundary (createdAt + id). + const expectedCursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString( + 'base64url', + ); + expect(result.nextCursor).toBe(expectedCursor); + }); + + it('decodes the incoming cursor into a keyset boundary', async () => { + mockAccessibleProjects(); + workflowReviewRequestRepository.findManyForInbox.mockResolvedValue([]); + workflowReviewRequestWorkflowRepository.findWorkflowNamesByRequestIds.mockResolvedValue( + new Map(), + ); + const cursor = Buffer.from('2024-01-02T00:00:00.000Z|req-2', 'utf8').toString('base64url'); + + await service.listForInbox(user, { limit: 15, cursor }); + + expect(workflowReviewRequestRepository.findManyForInbox).toHaveBeenCalledWith( + expect.objectContaining({ + cursor: { createdAt: new Date('2024-01-02T00:00:00.000Z'), id: 'req-2' }, + }), + ); + }); + + it('rejects a malformed cursor', async () => { + mockAccessibleProjects(); + const cursor = Buffer.from('not-a-valid-cursor', 'utf8').toString('base64url'); + + await expect(service.listForInbox(user, { limit: 15, cursor })).rejects.toThrow( + 'Invalid pagination cursor', + ); + }); + }); + + describe('resolveAccessibleProjectIds', () => { + it('returns the publish-scoped project ids for members', async () => { + projectService.getProjectIdsWithScope.mockResolvedValueOnce(['publish-proj']); + + expect(await service.resolveAccessibleProjectIds(user)).toEqual(['publish-proj']); + expect(projectService.getProjectIdsWithScope).toHaveBeenCalledWith(user, [ + 'workflow:publish', + ]); + expect(projectService.getProjectIdsWithScope).toHaveBeenCalledTimes(1); + // Requesters see their own reviews via the repository's requesterId filter, + // so no personal-project fallback is needed here. + expect(projectService.getPersonalProject).not.toHaveBeenCalled(); + }); + + it('returns null (all projects) for users with global workflow:publish without enumerating projects', async () => { + const owner = mock({ + role: { + slug: 'global:owner', + scopes: [{ slug: 'workflow:publish' }], + }, + }); + + expect(await service.resolveAccessibleProjectIds(owner)).toBeNull(); + expect(projectService.getProjectIdsWithScope).not.toHaveBeenCalled(); + }); + + it('does not short-circuit for a global project:delete role without workflow:publish', async () => { + const admin = mock({ + role: { + slug: 'custom:global', + scopes: [{ slug: 'project:delete' }], + }, + }); + projectService.getProjectIdsWithScope.mockResolvedValueOnce(['publish-proj']); + + expect(await service.resolveAccessibleProjectIds(admin)).toEqual(['publish-proj']); + expect(projectService.getProjectIdsWithScope).toHaveBeenCalledWith(admin, [ + 'workflow:publish', + ]); + }); + }); +}); diff --git a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.service.test.ts b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.service.test.ts index c1c86aaadcd..9a1a5e7462c 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.service.test.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-request.service.test.ts @@ -3,7 +3,7 @@ import type { GetWorkflowReviewEligibleReviewersQueryDto, ListWorkflowReviewRequestsQueryDto, } from '@n8n/api-types'; -import type { Logger } from '@n8n/backend-common'; +import type { LicenseState, Logger } from '@n8n/backend-common'; import type { AuthIdentity, DbLockService, @@ -26,6 +26,7 @@ import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ConflictError } from '@/errors/response-errors/conflict.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import type { ProjectService } from '@/services/project.service.ee'; import type { RoleService } from '@/services/role.service'; import type { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; import type { WorkflowFinderService } from '@/workflows/workflow-finder.service'; @@ -59,6 +60,8 @@ describe('WorkflowReviewRequestService', () => { const reviewerRepository = mock(); const userRepository = mock(); const roleService = mock(); + const projectService = mock(); + const licenseState = mock(); const dbLockService = mock(); const collaborationService = mock(); const logger = mock(); @@ -76,12 +79,16 @@ describe('WorkflowReviewRequestService', () => { reviewerRepository, userRepository, roleService, + projectService, + licenseState, dbLockService, collaborationService, ); beforeEach(() => { vi.resetAllMocks(); + process.env.N8N_ENV_FEAT_WORKFLOW_REVIEWS = 'true'; + licenseState.isWorkflowReviewsLicensed.mockReturnValue(true); // Feature enabled by default; the disabled path is exercised explicitly. workflowReviewPolicyService.get.mockResolvedValue({ enabled: true }); // By default, run the critical section against the mocked transaction. diff --git a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts index 628b18a3220..2f07dde83ff 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.controller.integration.test.ts @@ -15,13 +15,13 @@ import { WorkflowReviewRequestWorkflowRepository, } from '@n8n/db'; import { Container } from '@n8n/di'; + +import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; import { createAdmin, createMember, createOwner, createUser } from '@test-integration/db/users'; import { createWorkflowHistoryItem } from '@test-integration/db/workflow-history'; import type { SuperAgentTest } from '@test-integration/types'; import * as utils from '@test-integration/utils'; -import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; - const testServer = utils.setupTestServer({ endpointGroups: ['workflow-reviews'], enabledFeatures: ['feat:workflowReviews'], @@ -30,9 +30,12 @@ const testServer = utils.setupTestServer({ let owner: User; let member: User; +let viewer: User; let ownerProject: Project; +let teamProject: Project; let ownerAgent: SuperAgentTest; let memberAgent: SuperAgentTest; +let viewerAgent: SuperAgentTest; let requestRepository: WorkflowReviewRequestRepository; let workflowRepository: WorkflowReviewRequestWorkflowRepository; @@ -49,6 +52,9 @@ beforeAll(() => { }); beforeEach(async () => { + process.env.N8N_ENV_FEAT_WORKFLOW_REVIEWS = 'true'; + testServer.license.enable('feat:workflowReviews'); + await testDb.truncate([ 'WorkflowReviewRequestAuthor', 'WorkflowReviewRequestReviewer', @@ -68,9 +74,15 @@ beforeEach(async () => { owner = await createOwner(); member = await createMember(); + viewer = await createMember(); ownerProject = await getPersonalProject(owner); + teamProject = await createTeamProject('Reviews Project', owner); + await linkUserToProject(member, teamProject, 'project:editor'); + await linkUserToProject(viewer, teamProject, 'project:viewer'); + ownerAgent = testServer.authAgentFor(owner); memberAgent = testServer.authAgentFor(member); + viewerAgent = testServer.authAgentFor(viewer); }); /** Create a workflow owned by `owner` with a pinned history version. */ @@ -742,3 +754,227 @@ describe('GET /workflow-review-requests', () => { testServer.license.enable('feat:workflowReviews'); }); }); + +async function seedInboxRequests() { + const openRequest = await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Open review request', + createdById: owner.id, + state: 'open', + }); + const closedRequest = await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Closed review request', + createdById: owner.id, + state: 'closed', + }); + return { openRequest, closedRequest }; +} + +describe('GET /workflow-review-requests/summary', () => { + test('returns hasAny true for instance owner', async () => { + await seedInboxRequests(); + + const response = await ownerAgent.get('/workflow-review-requests/summary').expect(200); + + expect(response.body.data).toEqual({ hasAny: true }); + }); + + test('returns hasAny true for project editor', async () => { + await seedInboxRequests(); + + const response = await memberAgent.get('/workflow-review-requests/summary').expect(200); + + expect(response.body.data).toEqual({ hasAny: true }); + }); + + test('returns hasAny false for project viewer', async () => { + await seedInboxRequests(); + + const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200); + + expect(response.body.data).toEqual({ hasAny: false }); + }); + + test('returns hasAny true for a requester of their own review', async () => { + await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Review submitted by viewer', + createdById: viewer.id, + state: 'open', + }); + + const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200); + + expect(response.body.data).toEqual({ hasAny: true }); + }); + + test('returns 403 when feature is disabled', async () => { + await policyService.set(false); + + await ownerAgent.get('/workflow-review-requests/summary').expect(403); + }); +}); + +describe('GET /workflow-review-requests/inbox', () => { + test('returns reviews for instance owner', async () => { + const { openRequest } = await seedInboxRequests(); + + const response = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ state: 'open', limit: 15 }) + .expect(200); + + expect(response.body.data.data).toHaveLength(1); + expect(response.body.data.data[0]).toMatchObject({ + id: openRequest.id, + title: 'Open review request', + state: 'open', + workflowName: null, + }); + expect(response.body.data.hasMore).toBe(false); + expect(response.body.data.nextCursor).toBeNull(); + }); + + test('returns empty list for project viewer', async () => { + await seedInboxRequests(); + + const response = await viewerAgent.get('/workflow-review-requests/inbox').expect(200); + + expect(response.body.data.data).toEqual([]); + expect(response.body.data.hasMore).toBe(false); + }); + + test('returns 403 when license is disabled', async () => { + testServer.license.disable('feat:workflowReviews'); + + await ownerAgent.get('/workflow-review-requests/inbox').expect(403); + }); + + test('returns cursor pagination metadata', async () => { + await seedInboxRequests(); + await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Second open review', + createdById: owner.id, + state: 'open', + }); + + const firstPage = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ state: 'open', limit: 1 }) + .expect(200); + + expect(firstPage.body.data.data).toHaveLength(1); + expect(firstPage.body.data.hasMore).toBe(true); + expect(firstPage.body.data.nextCursor).toBeTruthy(); + + const secondPage = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ + state: 'open', + limit: 1, + cursor: firstPage.body.data.nextCursor, + }) + .expect(200); + + expect(secondPage.body.data.data).toHaveLength(1); + expect(secondPage.body.data.data[0].id).not.toBe(firstPage.body.data.data[0].id); + }); + + test('includes workflow name on list items', async () => { + const workflow = await createWorkflow({ name: 'Inbox Workflow' }, teamProject); + const enrichedRequest = await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Enriched review request', + createdById: owner.id, + state: 'open', + }); + + await workflowRepository.createWorkflowRow({ + workflowReviewRequestId: enrichedRequest.id, + workflowId: workflow.id, + }); + + const response = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ state: 'open', limit: 15 }) + .expect(200); + + expect(response.body.data.data).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: enrichedRequest.id, + title: 'Enriched review request', + workflowName: 'Inbox Workflow', + }), + ]), + ); + }); + + test('hides reviews from projects the member cannot access', async () => { + const otherProject = await createTeamProject('Other Reviews Project', owner); + await requestRepository.createRequest({ + projectId: otherProject.id, + title: 'Private other-project review', + createdById: owner.id, + state: 'open', + }); + + const memberResponse = await memberAgent.get('/workflow-review-requests/inbox').expect(200); + expect(memberResponse.body.data.data).toEqual([]); + + const ownerResponse = await ownerAgent.get('/workflow-review-requests/inbox').expect(200); + expect(ownerResponse.body.data.data).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + title: 'Private other-project review', + }), + ]), + ); + }); + + test('shows requesters their own reviews even without project access', async () => { + const otherProject = await createTeamProject('Unrelated Project', owner); + const ownRequest = await requestRepository.createRequest({ + projectId: otherProject.id, + title: 'Review I submitted', + createdById: member.id, + state: 'open', + }); + + const response = await memberAgent.get('/workflow-review-requests/inbox').expect(200); + + expect(response.body.data.data).toEqual( + expect.arrayContaining([expect.objectContaining({ id: ownRequest.id })]), + ); + }); + + test('does not truncate pagination when the cursor row is deleted', async () => { + await seedInboxRequests(); + await requestRepository.createRequest({ + projectId: teamProject.id, + title: 'Second open review', + createdById: owner.id, + state: 'open', + }); + + const firstPage = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ state: 'open', limit: 1 }) + .expect(200); + const cursor = firstPage.body.data.nextCursor as string; + const firstId = firstPage.body.data.data[0].id as string; + + // Delete the anchor row before requesting the next page. + await requestRepository.delete({ id: firstId }); + + const secondPage = await ownerAgent + .get('/workflow-review-requests/inbox') + .query({ state: 'open', limit: 1, cursor }) + .expect(200); + + expect(secondPage.body.data.data).toHaveLength(1); + expect(secondPage.body.data.data[0].id).not.toBe(firstId); + }); +}); diff --git a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.env-gate.integration.test.ts b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.env-gate.integration.test.ts index 455ffdac1ca..55afca59faf 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.env-gate.integration.test.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/__tests__/workflow-review-requests.env-gate.integration.test.ts @@ -21,8 +21,8 @@ beforeEach(async () => { ownerAgent = testServer.authAgentFor(owner); }); -describe('POST /workflow-review-requests (env flag off)', () => { - test('is unreachable (404) even though the license is present', async () => { +describe('workflow-review-requests (env flag off)', () => { + test('POST is unreachable (404) even though the license is present', async () => { await ownerAgent .post('/workflow-review-requests') .send({ @@ -31,6 +31,14 @@ describe('POST /workflow-review-requests (env flag off)', () => { }) .expect(404); }); + + test('GET inbox is unreachable (404)', async () => { + await ownerAgent.get('/workflow-review-requests/inbox').expect(404); + }); + + test('GET summary is unreachable (404)', async () => { + await ownerAgent.get('/workflow-review-requests/summary').expect(404); + }); }); describe('GET /workflow-review-requests (env flag off)', () => { diff --git a/packages/cli/src/modules/workflow-reviews.ee/workflow-review-request.service.ts b/packages/cli/src/modules/workflow-reviews.ee/workflow-review-request.service.ts index 02eca6cd872..6439e117353 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/workflow-review-request.service.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/workflow-review-request.service.ts @@ -5,8 +5,12 @@ import type { WorkflowReviewEligibleReviewersList, WorkflowReviewRequestList, WorkflowReviewRequestSummary, + ListWorkflowReviewInboxQueryDto, + ListWorkflowReviewInboxResponse, + GetWorkflowReviewInboxSummaryResponse, + WorkflowReviewInboxItem, } from '@n8n/api-types'; -import { Logger } from '@n8n/backend-common'; +import { LicenseState, Logger } from '@n8n/backend-common'; import { DbLock, DbLockService, @@ -16,15 +20,20 @@ import { WorkflowReviewRequestRepository, WorkflowReviewRequestReviewerRepository, WorkflowReviewRequestWorkflowRepository, + type InboxCursor, type User, + type WorkflowReviewRequest, } from '@n8n/db'; import { Service } from '@n8n/di'; +import { hasGlobalScope } from '@n8n/permissions'; import { CollaborationService } from '@/collaboration/collaboration.service'; +import { isWorkflowReviewsFeatureAvailable } from '@/constants/workflow-reviews'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ConflictError } from '@/errors/response-errors/conflict.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { ProjectService } from '@/services/project.service.ee'; import { RoleService } from '@/services/role.service'; import { WorkflowReviewPolicyService } from '@/services/workflow-review-policy.service'; import { WorkflowFinderService } from '@/workflows/workflow-finder.service'; @@ -44,6 +53,8 @@ export class WorkflowReviewRequestService { private readonly workflowReviewRequestReviewerRepository: WorkflowReviewRequestReviewerRepository, private readonly userRepository: UserRepository, private readonly roleService: RoleService, + private readonly projectService: ProjectService, + private readonly licenseState: LicenseState, private readonly dbLockService: DbLockService, private readonly collaborationService: CollaborationService, ) {} @@ -253,4 +264,121 @@ export class WorkflowReviewRequestService { updatedAt: request.updatedAt.toISOString(), }; } + + /** + * Cross-project inbox. + * + * Defaults to open requests when `state` is omitted. Deferred (LIGO-594): + * `projectId`, `reviewer`, and `author` filters. + */ + async listForInbox( + user: User, + query: ListWorkflowReviewInboxQueryDto, + ): Promise { + await this.assertFeatureAvailable(); + + const projectIds = await this.resolveAccessibleProjectIds(user); + const { limit } = query; + const rows = await this.workflowReviewRequestRepository.findManyForInbox({ + projectIds, + requesterId: user.id, + state: query.state ?? 'open', + limit: limit + 1, + cursor: query.cursor ? this.decodeInboxCursor(query.cursor) : undefined, + }); + + const hasMore = rows.length > limit; + const data = rows.slice(0, limit); + const lastRow = data.at(-1); + const nextCursor = hasMore && lastRow ? this.encodeInboxCursor(lastRow) : null; + const workflowNamesByRequestId = + await this.workflowReviewRequestWorkflowRepository.findWorkflowNamesByRequestIds( + data.map((row) => row.id), + ); + + return { + data: data.map((row) => this.toInboxItem(row, workflowNamesByRequestId.get(row.id) ?? null)), + nextCursor, + hasMore, + }; + } + + async getInboxSummaryForUser(user: User): Promise { + await this.assertFeatureAvailable(); + + const projectIds = await this.resolveAccessibleProjectIds(user); + // Any state — sidebar must appear so users can open the Closed tab. + const hasAny = await this.workflowReviewRequestRepository.existsAnyForInbox({ + projectIds, + requesterId: user.id, + }); + + return { hasAny }; + } + + private async assertFeatureAvailable(): Promise { + if (!isWorkflowReviewsFeatureAvailable(this.licenseState.isWorkflowReviewsLicensed())) { + throw new ForbiddenError('Workflow reviews are not available on this instance'); + } + + const policy = await this.workflowReviewPolicyService.get(); + if (!policy.enabled) { + throw new ForbiddenError('Workflow reviews are disabled on this instance'); + } + } + + /** + * Project IDs for inbox queries. `null` means "all projects, unfiltered" — + * correct for users with `workflow:publish` scoped globally. Requesters always + * see their own reviews regardless (repository OR-matches `requesterId`), so no + * personal-project fallback is needed. + */ + async resolveAccessibleProjectIds(user: User): Promise { + if (hasGlobalScope(user, 'workflow:publish')) { + return null; + } + + return await this.projectService.getProjectIdsWithScope(user, ['workflow:publish']); + } + + /** + * Encode the keyset boundary (createdAt + id) into an opaque cursor so the + * next page is resolved without re-reading the anchor row — a review deleted + * between requests no longer truncates the rest of the inbox. + */ + private encodeInboxCursor(row: WorkflowReviewRequest): string { + return Buffer.from(`${row.createdAt.toISOString()}|${row.id}`, 'utf8').toString('base64url'); + } + + private decodeInboxCursor(cursor: string): InboxCursor { + const decoded = Buffer.from(cursor, 'base64url').toString('utf8'); + const separatorIndex = decoded.indexOf('|'); + if (separatorIndex === -1) { + throw new BadRequestError('Invalid pagination cursor'); + } + + const createdAt = new Date(decoded.slice(0, separatorIndex)); + const id = decoded.slice(separatorIndex + 1); + if (id.length === 0 || Number.isNaN(createdAt.getTime())) { + throw new BadRequestError('Invalid pagination cursor'); + } + + return { createdAt, id }; + } + + private toInboxItem( + entity: WorkflowReviewRequest, + workflowName: string | null, + ): WorkflowReviewInboxItem { + return { + id: entity.id, + projectId: entity.projectId, + title: entity.title, + workflowName, + decision: entity.decision, + state: entity.state, + createdAt: entity.createdAt.toISOString(), + updatedAt: entity.updatedAt.toISOString(), + }; + } } diff --git a/packages/cli/src/modules/workflow-reviews.ee/workflow-review-requests.controller.ts b/packages/cli/src/modules/workflow-reviews.ee/workflow-review-requests.controller.ts index 5c2ebfbe17b..b19bd8299f2 100644 --- a/packages/cli/src/modules/workflow-reviews.ee/workflow-review-requests.controller.ts +++ b/packages/cli/src/modules/workflow-reviews.ee/workflow-review-requests.controller.ts @@ -2,10 +2,13 @@ import { CreateWorkflowReviewRequestDto, GetWorkflowReviewEligibleReviewersQueryDto, ListWorkflowReviewRequestsQueryDto, + type GetWorkflowReviewInboxSummaryResponse, + type ListWorkflowReviewInboxResponse, + ListWorkflowReviewInboxQueryDto, } from '@n8n/api-types'; import { AuthenticatedRequest } from '@n8n/db'; import { Body, Get, Licensed, Post, Query, RestController } from '@n8n/decorators'; -import { Response } from 'express'; +import type { Response } from 'express'; import { WorkflowReviewRequestService } from './workflow-review-request.service'; @@ -46,4 +49,27 @@ export class WorkflowReviewRequestsController { res.status(201); return request; } + + /** + * Cross-project inbox. Lives under `/inbox` so `GET /` stays free for the + * workflow-scoped list used by review-required toggle sync (LIGO-838). + */ + @Get('/inbox') + @Licensed('feat:workflowReviews') + async listInbox( + req: AuthenticatedRequest, + _res: Response, + @Query query: ListWorkflowReviewInboxQueryDto, + ): Promise { + return await this.workflowReviewRequestService.listForInbox(req.user, query); + } + + @Get('/summary') + @Licensed('feat:workflowReviews') + async getSummary( + req: AuthenticatedRequest, + _res: Response, + ): Promise { + return await this.workflowReviewRequestService.getInboxSummaryForUser(req.user); + } } diff --git a/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue b/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue index a4448a3e82b..36b1d4d3963 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue +++ b/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue @@ -87,7 +87,7 @@ const scrollRight = () => scroll(50);
-
+
scroll(50);
(() => ({ preview: true, })); +const { isWorkflowReviewsEnabled: isWorkflowReviewsNavVisible } = useWorkflowReviewsFeature(); + +const workflowReviews = computed(() => ({ + id: 'workflow-reviews', + icon: 'message-square-text', + label: locale.baseText('workflowReviews.menu.title'), + route: { to: { name: WORKFLOW_REVIEW_REQUESTS_VIEW } }, + preview: true, +})); const chat = computed(() => ({ id: 'chat', icon: 'message-circle', @@ -180,6 +191,13 @@ onBeforeUnmount(() => { :active="activeTabId === 'shared'" data-test-id="project-shared-menu-item" /> + { route.name === VIEWS.SHARED_CREDENTIALS, ); + const isWorkflowReviewsSubPage = computed(() => route.name === WORKFLOW_REVIEW_REQUESTS_VIEW); + return reactive({ isOverviewSubPage, isSharedSubPage, isProjectsSubPage, + isWorkflowReviewsSubPage, }); }; diff --git a/packages/frontend/editor-ui/src/features/collaboration/projects/projects.store.ts b/packages/frontend/editor-ui/src/features/collaboration/projects/projects.store.ts index 42758ca3b00..cdc22e16671 100644 --- a/packages/frontend/editor-ui/src/features/collaboration/projects/projects.store.ts +++ b/packages/frontend/editor-ui/src/features/collaboration/projects/projects.store.ts @@ -327,6 +327,11 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => { setCurrentProject(null); } + if (newRoute?.path?.includes('workflow-review-requests')) { + projectNavActiveId.value = 'workflow-reviews'; + setCurrentProject(null); + } + if (newRoute?.path?.includes('workflow/')) { if (currentProjectId.value) { projectNavActiveId.value = currentProjectId.value; diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue b/packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue new file mode 100644 index 00000000000..03abefe6fe0 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue @@ -0,0 +1,278 @@ + + + + + diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/constants.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/constants.ts new file mode 100644 index 00000000000..d5d95ba187a --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/constants.ts @@ -0,0 +1 @@ +export const WORKFLOW_REVIEW_REQUESTS_VIEW = 'WorkflowReviewRequestsView'; diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/module.descriptor.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/module.descriptor.ts new file mode 100644 index 00000000000..b06e73c8a34 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/module.descriptor.ts @@ -0,0 +1,34 @@ +import type { FrontendModuleDescription } from '@n8n/frontend-module-sdk'; + +import { VIEWS } from '@/app/constants'; + +import { useWorkflowReviewsFeature } from './composables/useWorkflowReviewsFeature'; +import { WORKFLOW_REVIEW_REQUESTS_VIEW } from './constants'; + +const WorkflowReviewRequestsView = async () => + await import('./views/WorkflowReviewRequestsView.vue'); + +export const WorkflowReviewsModule: FrontendModuleDescription = { + id: 'workflow-reviews', + name: 'Workflow Reviews', + description: 'Cross-project workflow review inbox.', + icon: 'message-square-text', + routes: [ + { + path: '/workflow-review-requests', + name: WORKFLOW_REVIEW_REQUESTS_VIEW, + component: WorkflowReviewRequestsView, + beforeEnter() { + return ( + useWorkflowReviewsFeature().isWorkflowReviewsEnabled.value || { + name: VIEWS.NOT_FOUND, + } + ); + }, + meta: { + layout: 'default', + middleware: ['authenticated', 'custom'], + }, + }, + ], +}; diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.test.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.test.ts new file mode 100644 index 00000000000..2cb113eac15 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.test.ts @@ -0,0 +1,206 @@ +import { createPinia, setActivePinia } from 'pinia'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import * as workflowReviewsApi from './workflowReviews.api'; +import { useReviewInboxStore } from './reviewInbox.store'; + +vi.mock('./workflowReviews.api'); + +describe('useReviewInboxStore', () => { + beforeEach(() => { + setActivePinia(createPinia()); + vi.resetAllMocks(); + }); + + it('probes summary and loads open reviews when hasAny is true', async () => { + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ + hasAny: true, + }); + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockResolvedValue({ + data: [ + { + id: 'req-1', + projectId: 'proj-1', + title: 'Review', + workflowName: 'My workflow', + decision: 'pending', + state: 'open', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + ], + nextCursor: null, + hasMore: false, + }); + + const store = useReviewInboxStore(); + await store.probeInbox(); + + expect(workflowReviewsApi.fetchWorkflowReviewInboxSummary).toHaveBeenCalledTimes(1); + expect(store.hasAnyReviews).toBe(true); + expect(store.probeSettled).toBe(true); + expect(store.showSidebar).toBe(true); + expect(store.items).toHaveLength(1); + }); + + it('skips list fetch when summary hasAny is false', async () => { + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ + hasAny: false, + }); + + const store = useReviewInboxStore(); + await store.probeInbox(); + + expect(workflowReviewsApi.fetchWorkflowReviewInbox).not.toHaveBeenCalled(); + expect(store.showSidebar).toBe(false); + }); + + it('refetches when switching tabs', async () => { + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockResolvedValue({ + data: [], + nextCursor: null, + hasMore: false, + }); + + const store = useReviewInboxStore(); + await store.setActiveState('closed'); + + expect(store.activeState).toBe('closed'); + expect(workflowReviewsApi.fetchWorkflowReviewInbox).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ state: 'closed' }), + ); + }); + + it('ignores stale list responses', async () => { + let resolveFirst!: (value: { + data: []; + nextCursor: null; + hasMore: false; + }) => void; + const firstResponse = new Promise<{ + data: []; + nextCursor: null; + hasMore: false; + }>((resolve) => { + resolveFirst = resolve; + }); + + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox) + .mockImplementationOnce(async () => await firstResponse) + .mockResolvedValueOnce({ + data: [ + { + id: 'req-2', + projectId: 'proj-1', + title: 'Newer', + workflowName: null, + decision: 'pending', + state: 'closed', + createdAt: '2024-01-02T00:00:00.000Z', + updatedAt: '2024-01-02T00:00:00.000Z', + }, + ], + nextCursor: null, + hasMore: false, + }); + + const store = useReviewInboxStore(); + const firstFetch = store.fetchList({ reset: true }); + await store.setActiveState('closed'); + await vi.waitFor(() => { + expect(store.items).toEqual([expect.objectContaining({ id: 'req-2', title: 'Newer' })]); + }); + + resolveFirst({ data: [], nextCursor: null, hasMore: false }); + await firstFetch; + + expect(store.items).toEqual([expect.objectContaining({ id: 'req-2', title: 'Newer' })]); + }); + + it('does not apply probe results after reset', async () => { + let resolveSummary!: (value: { hasAny: boolean }) => void; + const summaryPromise = new Promise<{ hasAny: boolean }>((resolve) => { + resolveSummary = resolve; + }); + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockImplementationOnce( + async () => await summaryPromise, + ); + + const store = useReviewInboxStore(); + const probe = store.probeInbox(); + store.reset(); + resolveSummary({ hasAny: true }); + await probe; + + expect(store.probeSettled).toBe(false); + expect(store.hasAnyReviews).toBe(false); + expect(workflowReviewsApi.fetchWorkflowReviewInbox).not.toHaveBeenCalled(); + }); + + it('clears loadingMore when a reset list request invalidates pagination', async () => { + let resolveMore!: (value: { + data: []; + nextCursor: null; + hasMore: false; + }) => void; + const morePromise = new Promise<{ + data: []; + nextCursor: null; + hasMore: false; + }>((resolve) => { + resolveMore = resolve; + }); + + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox) + .mockResolvedValueOnce({ + data: [ + { + id: 'req-1', + projectId: 'proj-1', + title: 'Open', + workflowName: null, + decision: 'pending', + state: 'open', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }, + ], + nextCursor: 'req-1', + hasMore: true, + }) + .mockImplementationOnce(async () => await morePromise) + .mockResolvedValueOnce({ + data: [], + nextCursor: null, + hasMore: false, + }); + + const store = useReviewInboxStore(); + await store.fetchList({ reset: true }); + const loadMore = store.loadMore(); + await vi.waitFor(() => { + expect(store.loadingMore).toBe(true); + }); + + await store.setActiveState('closed'); + expect(store.loadingMore).toBe(false); + + resolveMore({ data: [], nextCursor: null, hasMore: false }); + await loadMore; + expect(store.loadingMore).toBe(false); + }); + + it('does not treat a failed list fetch as an empty inbox', async () => { + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ + hasAny: true, + }); + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockRejectedValue(new Error('network')); + + const store = useReviewInboxStore(); + await expect(store.probeInbox()).rejects.toThrow('network'); + + expect(store.error).toEqual(new Error('network')); + expect(store.isEmpty).toBe(false); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts new file mode 100644 index 00000000000..950f4faa803 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts @@ -0,0 +1,202 @@ +import { defineStore } from 'pinia'; +import type { + ListWorkflowReviewInboxResponse, + WorkflowReviewRequestState, + WorkflowReviewInboxItem, +} from '@n8n/api-types'; +import { computed, ref } from 'vue'; + +import { useRootStore } from '@n8n/stores/useRootStore'; + +import { fetchWorkflowReviewInbox, fetchWorkflowReviewInboxSummary } from './workflowReviews.api'; + +const DEFAULT_LIMIT = 15; + +function toError(error: unknown): Error { + return error instanceof Error ? error : new Error(String(error)); +} + +export const useReviewInboxStore = defineStore('workflowReviewInbox', () => { + const rootStore = useRootStore(); + + const probeSettled = ref(false); + const hasAnyReviews = ref(false); + const items = ref([]); + const selectedId = ref(null); + const activeState = ref('open'); + const nextCursor = ref(null); + const hasMore = ref(false); + const loading = ref(false); + const loadingMore = ref(false); + const error = ref(null); + + let listRequestSeq = 0; + let probeRequestSeq = 0; + + const selectedItem = computed( + () => items.value.find((item) => item.id === selectedId.value) ?? null, + ); + const showSidebar = computed(() => probeSettled.value && hasAnyReviews.value); + const isEmpty = computed( + () => showSidebar.value && !loading.value && error.value === null && items.value.length === 0, + ); + + function applyListResponse( + response: ListWorkflowReviewInboxResponse, + { append }: { append: boolean }, + ): void { + items.value = append ? [...items.value, ...response.data] : response.data; + nextCursor.value = response.nextCursor; + hasMore.value = response.hasMore; + } + + async function requestList(cursor?: string): Promise { + return await fetchWorkflowReviewInbox(rootStore.restApiContext, { + state: activeState.value, + limit: DEFAULT_LIMIT, + cursor, + }); + } + + async function probeInbox() { + const requestSeq = ++probeRequestSeq; + probeSettled.value = false; + error.value = null; + + try { + const summary = await fetchWorkflowReviewInboxSummary(rootStore.restApiContext); + if (requestSeq !== probeRequestSeq) { + return; + } + + hasAnyReviews.value = summary.hasAny; + probeSettled.value = true; + + if (summary.hasAny) { + await fetchList({ reset: true }); + } + } catch (e) { + if (requestSeq !== probeRequestSeq) { + return; + } + error.value = toError(e); + probeSettled.value = true; + throw e; + } + } + + async function fetchList(options: { reset?: boolean } = {}) { + const requestSeq = ++listRequestSeq; + + if (options.reset) { + items.value = []; + nextCursor.value = null; + hasMore.value = false; + selectedId.value = null; + // Invalidate any in-flight loadMore so pagination is not stuck. + loadingMore.value = false; + } + + loading.value = true; + error.value = null; + + try { + const response = await requestList(); + if (requestSeq !== listRequestSeq) { + return; + } + applyListResponse(response, { append: false }); + } catch (e) { + if (requestSeq !== listRequestSeq) { + return; + } + error.value = toError(e); + throw e; + } finally { + if (requestSeq === listRequestSeq) { + loading.value = false; + } + } + } + + async function loadMore() { + if (loading.value || loadingMore.value || !hasMore.value || !nextCursor.value) { + return; + } + + const requestSeq = ++listRequestSeq; + const cursor = nextCursor.value; + loadingMore.value = true; + error.value = null; + + try { + const response = await requestList(cursor); + if (requestSeq !== listRequestSeq) { + return; + } + applyListResponse(response, { append: true }); + } catch (e) { + if (requestSeq !== listRequestSeq) { + return; + } + error.value = toError(e); + throw e; + } finally { + if (requestSeq === listRequestSeq) { + loadingMore.value = false; + } + } + } + + async function setActiveState(state: WorkflowReviewRequestState) { + if (activeState.value === state) return; + activeState.value = state; + await fetchList({ reset: true }); + } + + function selectItem(id: string) { + selectedId.value = id; + } + + function clearSelection() { + selectedId.value = null; + } + + function reset() { + probeRequestSeq += 1; + listRequestSeq += 1; + probeSettled.value = false; + hasAnyReviews.value = false; + items.value = []; + selectedId.value = null; + activeState.value = 'open'; + nextCursor.value = null; + hasMore.value = false; + loading.value = false; + loadingMore.value = false; + error.value = null; + } + + return { + probeSettled, + hasAnyReviews, + items, + selectedId, + selectedItem, + activeState, + nextCursor, + hasMore, + loading, + loadingMore, + error, + showSidebar, + isEmpty, + probeInbox, + fetchList, + loadMore, + setActiveState, + selectItem, + clearSelection, + reset, + }; +}); diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.test.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.test.ts new file mode 100644 index 00000000000..52a14e36b0e --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.test.ts @@ -0,0 +1,144 @@ +import { createTestingPinia } from '@pinia/testing'; +import { createComponentRenderer } from '@/__tests__/render'; +import { mockedStore, waitAllPromises } from '@/__tests__/utils'; +import { useToast } from '@/app/composables/useToast'; + +import { useReviewInboxStore } from '../reviewInbox.store'; +import WorkflowReviewRequestsView from './WorkflowReviewRequestsView.vue'; + +vi.mock('@/app/composables/useToast', () => ({ + useToast: vi.fn(), +})); + +vi.mock('@/app/composables/useDocumentTitle', () => ({ + useDocumentTitle: () => ({ + set: vi.fn(), + }), +})); + +const showError = vi.fn(); + +const renderComponent = createComponentRenderer(WorkflowReviewRequestsView, { + global: { + stubs: { + PageViewLayout: { + template: '
', + }, + WorkflowReviewRequestsSidebar: { + template: '
', + }, + }, + }, +}); + +describe('WorkflowReviewRequestsView', () => { + let store: ReturnType>; + + beforeEach(() => { + createTestingPinia(); + showError.mockReset(); + vi.mocked(useToast).mockReturnValue({ showError } as unknown as ReturnType); + + store = mockedStore(useReviewInboxStore); + store.probeSettled = false; + store.showSidebar = false; + store.selectedItem = null; + store.items = []; + store.activeState = 'open'; + store.selectedId = null; + store.loading = false; + store.loadingMore = false; + store.hasMore = false; + store.isEmpty = false; + store.probeInbox.mockResolvedValue(undefined); + store.reset.mockClear(); + }); + + it('probes the inbox on mount', async () => { + renderComponent(); + await waitAllPromises(); + + expect(store.probeInbox).toHaveBeenCalledTimes(1); + }); + + it('shows loading while the inbox probe has not settled', async () => { + store.probeSettled = false; + + const { container, queryByTestId } = renderComponent(); + await waitAllPromises(); + + expect(container.querySelector('.n8n-loading')).toBeInTheDocument(); + expect(queryByTestId('workflow-reviews-disclaimer')).not.toBeInTheDocument(); + }); + + it('shows the disclaimer when settled with nothing selected', async () => { + store.probeSettled = true; + store.showSidebar = false; + store.selectedItem = null; + + const { container, getByTestId, queryByTestId } = renderComponent(); + await waitAllPromises(); + + expect(getByTestId('workflow-reviews-disclaimer')).toBeInTheDocument(); + expect(container.querySelector('.n8n-loading')).not.toBeInTheDocument(); + expect(queryByTestId('workflow-review-requests-sidebar')).not.toBeInTheDocument(); + }); + + it('renders the sidebar and selected title when a review is selected', async () => { + store.probeSettled = true; + store.showSidebar = true; + store.selectedItem = { + id: 'req-1', + projectId: 'proj-1', + title: 'Needs review', + workflowName: 'My workflow', + decision: 'pending', + state: 'open', + createdAt: '2024-01-01T00:00:00.000Z', + updatedAt: '2024-01-01T00:00:00.000Z', + }; + + const { getByTestId } = renderComponent(); + await waitAllPromises(); + + expect(getByTestId('workflow-review-requests-sidebar')).toBeInTheDocument(); + expect(getByTestId('workflow-review-request-title')).toHaveTextContent('Needs review'); + expect(getByTestId('workflow-review-request-detail-stub')).toBeInTheDocument(); + }); + + it('shows an error toast when probing the inbox fails', async () => { + const error = new Error('network'); + store.probeInbox.mockRejectedValueOnce(error); + + renderComponent(); + await waitAllPromises(); + + expect(showError).toHaveBeenCalledWith(error, 'Could not load workflow reviews'); + }); + + it('does not toast probe errors after unmount', async () => { + let rejectProbe!: (error: Error) => void; + store.probeInbox.mockImplementationOnce( + async () => + await new Promise((_resolve, reject) => { + rejectProbe = reject; + }), + ); + + const { unmount } = renderComponent(); + unmount(); + rejectProbe(new Error('network')); + await waitAllPromises(); + + expect(showError).not.toHaveBeenCalled(); + }); + + it('resets the store on unmount', async () => { + const { unmount } = renderComponent(); + await waitAllPromises(); + + unmount(); + + expect(store.reset).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue b/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue new file mode 100644 index 00000000000..d4e588c4556 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue @@ -0,0 +1,183 @@ + + + + + diff --git a/packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.api.ts b/packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.api.ts index 9c818c8272a..62a80940e72 100644 --- a/packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.api.ts +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/workflowReviews.api.ts @@ -1,4 +1,6 @@ import type { + GetWorkflowReviewInboxSummaryResponse, + ListWorkflowReviewInboxResponse, WorkflowReviewEligibleReviewersList, WorkflowReviewRequestList, WorkflowReviewRequestState, @@ -16,6 +18,13 @@ export interface CreateWorkflowReviewRequestPayload { reviewerUserIds?: string[]; } +export type FetchWorkflowReviewInboxParams = { + state?: WorkflowReviewRequestState; + limit?: number; + cursor?: string; +}; + +/** Workflow-scoped list used by review-required toggle sync. */ export async function fetchWorkflowReviewRequests( context: IRestApiContext, query: { workflowId: string; state?: WorkflowReviewRequestState; take?: number; skip?: number }, @@ -51,3 +60,17 @@ export async function createWorkflowReviewRequest( { ...payload }, ); } + +export async function fetchWorkflowReviewInboxSummary( + context: IRestApiContext, +): Promise { + return await makeRestApiRequest(context, 'GET', '/workflow-review-requests/summary'); +} + +/** Cross-project inbox list. */ +export async function fetchWorkflowReviewInbox( + context: IRestApiContext, + params: FetchWorkflowReviewInboxParams, +): Promise { + return await makeRestApiRequest(context, 'GET', '/workflow-review-requests/inbox', params); +}