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 index fb0f35c3d49..5631e076b75 100644 --- 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 @@ -35,5 +35,6 @@ export interface ListWorkflowReviewInboxResponse { } export interface GetWorkflowReviewInboxSummaryResponse { - hasAny: boolean; + open: number; + closed: number; } 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 1113815ed0c..183f6ad986d 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 @@ -22,10 +22,12 @@ describe('WorkflowReviewRequestRepository', () => { queryBuilder.orderBy.mockReturnThis(); queryBuilder.addOrderBy.mockReturnThis(); queryBuilder.select.mockReturnThis(); + queryBuilder.addSelect.mockReturnThis(); + queryBuilder.groupBy.mockReturnThis(); queryBuilder.take.mockReturnThis(); queryBuilder.limit.mockReturnThis(); queryBuilder.getMany.mockResolvedValue([]); - queryBuilder.getRawOne.mockResolvedValue(undefined); + queryBuilder.getRawMany.mockResolvedValue([]); vi.spyOn(repo, 'createQueryBuilder').mockReturnValue(queryBuilder); }); @@ -222,67 +224,62 @@ describe('WorkflowReviewRequestRepository', () => { }); }); - describe('existsAnyForInbox', () => { - it('queries by requester only when projectIds is empty', async () => { - queryBuilder.getRawOne.mockResolvedValueOnce({ '?column?': 1 }); + describe('countByStateForInbox', () => { + it('groups by state applying the requester-only filter when projectIds is empty', async () => { + queryBuilder.getRawMany.mockResolvedValueOnce([{ state: 'open', count: '2' }]); - const result = await repo.existsAnyForInbox({ projectIds: [], requesterId: 'user-1' }); + const result = await repo.countByStateForInbox({ projectIds: [], requesterId: 'user-1' }); - expect(result).toBe(true); + expect(result).toEqual({ open: 2, closed: 0 }); expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); - expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(queryBuilder.select).toHaveBeenCalledWith('review.state', 'state'); + expect(queryBuilder.addSelect).toHaveBeenCalledWith('COUNT(*)', 'count'); + expect(queryBuilder.groupBy).toHaveBeenCalledWith('review.state'); 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 }); + it('skips the projectId filter when projectIds is null (global scope counts all)', async () => { + queryBuilder.getRawMany.mockResolvedValueOnce([ + { state: 'open', count: '3' }, + { state: 'closed', count: '12' }, + ]); - const result = await repo.existsAnyForInbox({ - projectIds: null, - requesterId: 'user-1', - state: 'open', - }); + const result = await repo.countByStateForInbox({ projectIds: null, requesterId: 'user-1' }); - expect(result).toBe(true); - expect(repo.createQueryBuilder).toHaveBeenCalledWith('review'); - expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(result).toEqual({ open: 3, closed: 12 }); expect(queryBuilder.where).not.toHaveBeenCalled(); - expect(queryBuilder.andWhere).toHaveBeenCalledWith('review.state = :state', { - state: 'open', - }); - expect(queryBuilder.limit).toHaveBeenCalledWith(1); + expect(queryBuilder.groupBy).toHaveBeenCalledWith('review.state'); }); - it('returns true when at least one matching row exists', async () => { - queryBuilder.getRawOne.mockResolvedValueOnce({ '?column?': 1 }); + it('matches projects OR the requester', async () => { + queryBuilder.getRawMany.mockResolvedValueOnce([ + { state: 'open', count: 1 }, + { state: 'closed', count: 4 }, + ]); - const result = await repo.existsAnyForInbox({ - projectIds: ['proj-1'], + const result = await repo.countByStateForInbox({ + projectIds: ['proj-1', 'proj-2'], requesterId: 'user-1', - state: 'open', }); - expect(result).toBe(true); - expect(queryBuilder.select).toHaveBeenCalledWith('1'); + expect(result).toEqual({ open: 1, closed: 4 }); expect(queryBuilder.where).toHaveBeenCalledWith( '(review.projectId IN (:...projectIds) OR review.createdById = :requesterId)', - { projectIds: ['proj-1'], requesterId: 'user-1' }, + { projectIds: ['proj-1', 'proj-2'], 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); + it('defaults absent states to zero', async () => { + queryBuilder.getRawMany.mockResolvedValueOnce([]); - await expect( - repo.existsAnyForInbox({ projectIds: ['proj-1'], requesterId: 'user-1' }), - ).resolves.toBe(false); + const result = await repo.countByStateForInbox({ + projectIds: ['proj-1'], + requesterId: 'user-1', + }); + + expect(result).toEqual({ open: 0, closed: 0 }); }); }); }); 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 5bad98e487f..b26a25acc3b 100644 --- a/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts +++ b/packages/@n8n/db/src/repositories/workflow-review-request.repository.ts @@ -28,12 +28,16 @@ export type FindManyForInboxOptions = { cursor?: InboxCursor; }; -export type ExistsAnyForInboxOptions = { +export type CountByStateForInboxOptions = { /** `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; +}; + +export type InboxStateCounts = { + open: number; + closed: number; }; @Service() @@ -148,19 +152,25 @@ export class WorkflowReviewRequestRepository extends Repository { - const { projectIds, requesterId, state } = options; + async countByStateForInbox(options: CountByStateForInboxOptions): Promise { + const { projectIds, requesterId } = options; - const queryBuilder = this.createQueryBuilder('review').select('1'); + const queryBuilder = this.createQueryBuilder('review') + .select('review.state', 'state') + .addSelect('COUNT(*)', 'count') + .groupBy('review.state'); this.applyInboxVisibility(queryBuilder, projectIds, requesterId); - if (state !== undefined) { - queryBuilder.andWhere('review.state = :state', { state }); - } + const rows = await queryBuilder.getRawMany<{ + state: WorkflowReviewRequestState; + count: string | number; + }>(); - const row = await queryBuilder.limit(1).getRawOne>(); - return row !== undefined; + return { + open: Number(rows.find((row) => row.state === 'open')?.count ?? 0), + closed: Number(rows.find((row) => row.state === 'closed')?.count ?? 0), + }; } /** 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 2f07dde83ff..577cfcc058b 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 @@ -772,31 +772,31 @@ async function seedInboxRequests() { } describe('GET /workflow-review-requests/summary', () => { - test('returns hasAny true for instance owner', async () => { + test('returns open/closed counts for instance owner', async () => { await seedInboxRequests(); const response = await ownerAgent.get('/workflow-review-requests/summary').expect(200); - expect(response.body.data).toEqual({ hasAny: true }); + expect(response.body.data).toEqual({ open: 1, closed: 1 }); }); - test('returns hasAny true for project editor', async () => { + test('returns open/closed counts for project editor', async () => { await seedInboxRequests(); const response = await memberAgent.get('/workflow-review-requests/summary').expect(200); - expect(response.body.data).toEqual({ hasAny: true }); + expect(response.body.data).toEqual({ open: 1, closed: 1 }); }); - test('returns hasAny false for project viewer', async () => { + test('returns zero counts for project viewer with nothing visible', async () => { await seedInboxRequests(); const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200); - expect(response.body.data).toEqual({ hasAny: false }); + expect(response.body.data).toEqual({ open: 0, closed: 0 }); }); - test('returns hasAny true for a requester of their own review', async () => { + test('counts a requester their own review regardless of project scope', async () => { await requestRepository.createRequest({ projectId: teamProject.id, title: 'Review submitted by viewer', @@ -806,7 +806,7 @@ describe('GET /workflow-review-requests/summary', () => { const response = await viewerAgent.get('/workflow-review-requests/summary').expect(200); - expect(response.body.data).toEqual({ hasAny: true }); + expect(response.body.data).toEqual({ open: 1, closed: 0 }); }); test('returns 403 when feature is disabled', async () => { 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 6439e117353..ecf3309074d 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 @@ -307,13 +307,10 @@ export class WorkflowReviewRequestService { 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({ + return await this.workflowReviewRequestRepository.countByStateForInbox({ projectIds, requesterId: user.id, }); - - return { hasAny }; } private async assertFeatureAvailable(): Promise { 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 36b1d4d3963..1fe777368b7 100644 --- a/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue +++ b/packages/frontend/@n8n/design-system/src/components/N8nTabs/Tabs.vue @@ -223,6 +223,13 @@ const scrollRight = () => scroll(50); color: var(--color--primary); } + /* Inset outline so the ring isn't clipped by the scroll container and adds no layout shift. */ + &:focus-visible { + outline: var(--border-width) solid var(--focus--border-color); + outline-offset: calc(-1 * var(--border-width)); + border-radius: var(--radius--3xs); + } + span + span { margin-left: var(--spacing--4xs); } diff --git a/packages/frontend/@n8n/i18n/src/locales/en.json b/packages/frontend/@n8n/i18n/src/locales/en.json index bb272a5d326..ecb36574e05 100644 --- a/packages/frontend/@n8n/i18n/src/locales/en.json +++ b/packages/frontend/@n8n/i18n/src/locales/en.json @@ -5167,9 +5167,11 @@ "workflowReviews.disclaimer.body": "Submit workflow versions for review before publishing. When reviews are assigned to your projects, they will appear here.", "workflowReviews.sidebar.tabs.open": "Open", "workflowReviews.sidebar.tabs.closed": "Closed", - "workflowReviews.sidebar.empty": "No reviews in this tab", + "workflowReviews.sidebar.empty.open": "No open reviews", + "workflowReviews.sidebar.empty.closed": "No closed reviews", "workflowReviews.detail.placeholder": "Full review details are coming soon.", - "workflowReviews.emptyState.body": "There are no reviews in this tab.", + "workflowReviews.emptyState.body.open": "There are no open reviews right now.", + "workflowReviews.emptyState.body.closed": "There are no closed reviews right now.", "workflowReviews.noSelection.body": "Select a review to see more information.", "workflowReviews.error.load": "Could not load workflow reviews", "settings.security.personalSpace.title": "Personal Space", 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 index 03abefe6fe0..76c2001dcac 100644 --- a/packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/components/WorkflowReviewRequestsSidebar.vue @@ -17,6 +17,8 @@ import TimeAgo from '@/app/components/TimeAgo.vue'; const props = defineProps<{ items: WorkflowReviewInboxItem[]; activeState: WorkflowReviewRequestState; + openCount: number; + closedCount: number; selectedId: string | null; loading: boolean; loadingMore: boolean; @@ -36,8 +38,16 @@ const listRef = ref(null); const loadMoreSentinel = ref(null); const tabOptions = computed(() => [ - { label: i18n.baseText('workflowReviews.sidebar.tabs.open'), value: 'open' as const }, - { label: i18n.baseText('workflowReviews.sidebar.tabs.closed'), value: 'closed' as const }, + { + label: i18n.baseText('workflowReviews.sidebar.tabs.open'), + value: 'open' as const, + tag: String(props.openCount), + }, + { + label: i18n.baseText('workflowReviews.sidebar.tabs.closed'), + value: 'closed' as const, + tag: String(props.closedCount), + }, ]); const { observe: observeForLoadMore } = useIntersectionObserver({ @@ -78,7 +88,6 @@ function onListBackgroundClick() { @@ -99,7 +108,7 @@ function onListBackgroundClick() { size="small" data-test-id="workflow-reviews-empty" > - {{ i18n.baseText('workflowReviews.sidebar.empty') }} + {{ i18n.baseText(`workflowReviews.sidebar.empty.${activeState}`) }} { vi.resetAllMocks(); }); - it('probes summary and loads open reviews when hasAny is true', async () => { + it('probes summary and loads open reviews when counts are non-zero', async () => { vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ - hasAny: true, + open: 3, + closed: 12, }); vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockResolvedValue({ data: [ @@ -38,23 +39,48 @@ describe('useReviewInboxStore', () => { expect(workflowReviewsApi.fetchWorkflowReviewInboxSummary).toHaveBeenCalledTimes(1); expect(store.hasAnyReviews).toBe(true); + expect(store.openCount).toBe(3); + expect(store.closedCount).toBe(12); 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 () => { + it('skips list fetch when both counts are zero', async () => { vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ - hasAny: false, + open: 0, + closed: 0, }); const store = useReviewInboxStore(); await store.probeInbox(); expect(workflowReviewsApi.fetchWorkflowReviewInbox).not.toHaveBeenCalled(); + expect(store.hasAnyReviews).toBe(false); + expect(store.openCount).toBe(0); + expect(store.closedCount).toBe(0); expect(store.showSidebar).toBe(false); }); + it('shows the sidebar when only closed reviews exist', async () => { + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ + open: 0, + closed: 5, + }); + vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockResolvedValue({ + data: [], + nextCursor: null, + hasMore: false, + }); + + const store = useReviewInboxStore(); + await store.probeInbox(); + + expect(store.hasAnyReviews).toBe(true); + expect(store.showSidebar).toBe(true); + expect(workflowReviewsApi.fetchWorkflowReviewInbox).toHaveBeenCalledTimes(1); + }); + it('refetches when switching tabs', async () => { vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockResolvedValue({ data: [], @@ -119,8 +145,8 @@ describe('useReviewInboxStore', () => { }); it('does not apply probe results after reset', async () => { - let resolveSummary!: (value: { hasAny: boolean }) => void; - const summaryPromise = new Promise<{ hasAny: boolean }>((resolve) => { + let resolveSummary!: (value: { open: number; closed: number }) => void; + const summaryPromise = new Promise<{ open: number; closed: number }>((resolve) => { resolveSummary = resolve; }); vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockImplementationOnce( @@ -130,7 +156,7 @@ describe('useReviewInboxStore', () => { const store = useReviewInboxStore(); const probe = store.probeInbox(); store.reset(); - resolveSummary({ hasAny: true }); + resolveSummary({ open: 1, closed: 0 }); await probe; expect(store.probeSettled).toBe(false); @@ -193,7 +219,8 @@ describe('useReviewInboxStore', () => { it('does not treat a failed list fetch as an empty inbox', async () => { vi.mocked(workflowReviewsApi.fetchWorkflowReviewInboxSummary).mockResolvedValue({ - hasAny: true, + open: 1, + closed: 0, }); vi.mocked(workflowReviewsApi.fetchWorkflowReviewInbox).mockRejectedValue(new Error('network')); 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 index 950f4faa803..ec4967b376d 100644 --- a/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/reviewInbox.store.ts @@ -21,6 +21,8 @@ export const useReviewInboxStore = defineStore('workflowReviewInbox', () => { const probeSettled = ref(false); const hasAnyReviews = ref(false); + const openCount = ref(0); + const closedCount = ref(0); const items = ref([]); const selectedId = ref(null); const activeState = ref('open'); @@ -69,10 +71,12 @@ export const useReviewInboxStore = defineStore('workflowReviewInbox', () => { return; } - hasAnyReviews.value = summary.hasAny; + openCount.value = summary.open; + closedCount.value = summary.closed; + hasAnyReviews.value = summary.open + summary.closed > 0; probeSettled.value = true; - if (summary.hasAny) { + if (hasAnyReviews.value) { await fetchList({ reset: true }); } } catch (e) { @@ -167,6 +171,8 @@ export const useReviewInboxStore = defineStore('workflowReviewInbox', () => { listRequestSeq += 1; probeSettled.value = false; hasAnyReviews.value = false; + openCount.value = 0; + closedCount.value = 0; items.value = []; selectedId.value = null; activeState.value = 'open'; @@ -180,6 +186,8 @@ export const useReviewInboxStore = defineStore('workflowReviewInbox', () => { return { probeSettled, hasAnyReviews, + openCount, + closedCount, items, selectedId, selectedItem, 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 index d4e588c4556..de666adc96f 100644 --- a/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue +++ b/packages/frontend/editor-ui/src/features/workflow-reviews/views/WorkflowReviewRequestsView.vue @@ -23,6 +23,8 @@ const { loadingMore, hasMore, isEmpty, + openCount, + closedCount, } = storeToRefs(store); const i18n = useI18n(); @@ -76,6 +78,8 @@ onUnmounted(() => { v-if="showSidebar" :items="items" :active-state="activeState" + :open-count="openCount" + :closed-count="closedCount" :selected-id="selectedId" :loading="loading" :loading-more="loadingMore" @@ -133,7 +137,7 @@ onUnmounted(() => { size="medium" data-test-id="workflow-reviews-empty-state" > - {{ i18n.baseText('workflowReviews.emptyState.body') }} + {{ i18n.baseText(`workflowReviews.emptyState.body.${activeState}`) }}