From d002cc3f7d241cec14c95a37e09a37b77b6759bf Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 14 Jul 2025 10:10:03 +0200 Subject: [PATCH] fix(core): Allow insights breakdown by workflow to be sorted by workflow name (#17184) --- .../__tests__/list-workflow-query.dto.test.ts | 20 ++ .../dto/insights/list-workflow-query.dto.ts | 2 + .../integration/insights/insights.api.test.ts | 187 +++++++++- .../tables/InsightsTableWorkflows.test.ts | 325 ++++++++++++++++++ .../tables/InsightsTableWorkflows.vue | 7 +- 5 files changed, 530 insertions(+), 11 deletions(-) create mode 100644 packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.test.ts diff --git a/packages/@n8n/api-types/src/dto/insights/__tests__/list-workflow-query.dto.test.ts b/packages/@n8n/api-types/src/dto/insights/__tests__/list-workflow-query.dto.test.ts index c47d81156f..39d8c4070d 100644 --- a/packages/@n8n/api-types/src/dto/insights/__tests__/list-workflow-query.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/insights/__tests__/list-workflow-query.dto.test.ts @@ -20,6 +20,26 @@ describe('ListInsightsWorkflowQueryDto', () => { sortBy: 'total:asc', }, }, + { + name: 'valid sortBy workflowName:asc', + request: { + sortBy: 'workflowName:asc', + }, + parsedResult: { + ...DEFAULT_PAGINATION, + sortBy: 'workflowName:asc', + }, + }, + { + name: 'valid sortBy workflowName:desc', + request: { + sortBy: 'workflowName:desc', + }, + parsedResult: { + ...DEFAULT_PAGINATION, + sortBy: 'workflowName:desc', + }, + }, { name: 'valid skip and take', request: { diff --git a/packages/@n8n/api-types/src/dto/insights/list-workflow-query.dto.ts b/packages/@n8n/api-types/src/dto/insights/list-workflow-query.dto.ts index 29ae5d79ae..092b2bf26c 100644 --- a/packages/@n8n/api-types/src/dto/insights/list-workflow-query.dto.ts +++ b/packages/@n8n/api-types/src/dto/insights/list-workflow-query.dto.ts @@ -21,6 +21,8 @@ const VALID_SORT_OPTIONS = [ 'runTime:desc', 'averageRunTime:asc', 'averageRunTime:desc', + 'workflowName:asc', + 'workflowName:desc', ] as const; // --------------------- diff --git a/packages/cli/test/integration/insights/insights.api.test.ts b/packages/cli/test/integration/insights/insights.api.test.ts index 7d73f2bb81..296d2ba3da 100644 --- a/packages/cli/test/integration/insights/insights.api.test.ts +++ b/packages/cli/test/integration/insights/insights.api.test.ts @@ -1,7 +1,9 @@ import type { InsightsDateRange } from '@n8n/api-types'; -import { mockInstance } from '@n8n/backend-test-utils'; +import { mockInstance, createWorkflow, createTeamProject, testDb } from '@n8n/backend-test-utils'; +import { DateTime } from 'luxon'; import { Telemetry } from '@/telemetry'; +import { createCompactedInsightsEvent } from '@/modules/insights/database/entities/__tests__/db-utils'; import { createUser } from '../shared/db/users'; import type { SuperAgentTest } from '../shared/types'; @@ -116,11 +118,20 @@ describe('GET /insights/by-workflow', () => { features: ['feat:insights:viewSummary', 'feat:insights:viewDashboard'], }); }); - test('Call should work with valid query parameters', async () => { - await agents.owner - .get('/insights/by-workflow') - .query({ skip: '10', take: '20', sortBy: 'total:desc' }) - .expect(200); + + test.each([ + { + skip: '10', + take: '20', + sortBy: 'total:desc', + }, + { + skip: '1', + take: '25', + sortBy: 'workflowName:asc', + }, + ])('Call should work with valid query parameters: %s', async (queryParams) => { + await agents.owner.get('/insights/by-workflow').query(queryParams).expect(200); }); test.each<{ skip: string; take?: string; sortBy?: string }>([ @@ -149,4 +160,168 @@ describe('GET /insights/by-workflow', () => { }) .expect(400); }); + + test.each([ + 'total:asc', + 'total:desc', + 'succeeded:asc', + 'succeeded:desc', + 'failed:asc', + 'failed:desc', + 'failureRate:asc', + 'failureRate:desc', + 'timeSaved:asc', + 'timeSaved:desc', + 'runTime:asc', + 'runTime:desc', + 'averageRunTime:asc', + 'averageRunTime:desc', + 'workflowName:asc', + 'workflowName:desc', + ])('Call should return 200 with valid sortBy option: %s', async (sortBy) => { + const response = await agents.owner.get('/insights/by-workflow').query({ sortBy }).expect(200); + + expect(response.body).toHaveProperty('data'); + expect(response.body.data).toHaveProperty('count'); + expect(Array.isArray(response.body.data.data)).toBe(true); + }); + + describe('sorting order verification', () => { + afterEach(async () => { + await testDb.truncate([ + 'InsightsRaw', + 'InsightsByPeriod', + 'InsightsMetadata', + 'SharedWorkflow', + 'WorkflowEntity', + 'Project', + ]); + }); + + test('should return workflows sorted by total:desc', async () => { + const project = await createTeamProject('Test Project 1'); + const testData = [ + { name: 'Workflow A', total: 10 }, + { name: 'Workflow B', total: 5 }, + { name: 'Workflow C', total: 15 }, + ]; + + const workflows = await Promise.all( + testData.map(async (data) => await createWorkflow({ name: data.name }, project)), + ); + + const periodStart = DateTime.utc().startOf('day'); + + await Promise.all( + workflows.map( + async (workflow, index) => + await createCompactedInsightsEvent(workflow, { + type: 'success', + value: testData[index].total, + periodUnit: 'day', + periodStart, + }), + ), + ); + + const response = await agents.owner + .get('/insights/by-workflow') + .query({ sortBy: 'total:desc' }) + .expect(200); + + expect(response.body.data.count).toBe(3); + expect(response.body.data.data).toHaveLength(3); + + // Verify descending order by total + const expectedOrder = [15, 10, 5]; + response.body.data.data.forEach((item: any, index: number) => { + expect(item.total).toBe(expectedOrder[index]); + }); + }); + + test('should return workflows sorted by workflowName:asc', async () => { + const project = await createTeamProject('Test Project A'); + const workflowNames = ['Zebra Workflow', 'Alpha Workflow', 'Beta Workflow']; + + const workflows = await Promise.all( + workflowNames.map(async (name) => await createWorkflow({ name }, project)), + ); + + const periodStart = DateTime.utc().startOf('day'); + + await Promise.all( + workflows.map( + async (workflow) => + await createCompactedInsightsEvent(workflow, { + type: 'success', + value: 5, + periodUnit: 'day', + periodStart, + }), + ), + ); + + const response = await agents.owner + .get('/insights/by-workflow') + .query({ sortBy: 'workflowName:asc' }) + .expect(200); + + expect(response.body.data.count).toBe(3); + expect(response.body.data.data).toHaveLength(3); + + // Verify ascending order by workflow name + const expectedOrder = ['Alpha Workflow', 'Beta Workflow', 'Zebra Workflow']; + response.body.data.data.forEach((item: any, index: number) => { + expect(item.workflowName).toBe(expectedOrder[index]); + }); + }); + + test('should return workflows sorted by failureRate:asc', async () => { + const project = await createTeamProject('Another Test Project'); + const testData = [ + { name: 'Low Failure', success: 9, failure: 1 }, // 10% failure rate + { name: 'High Failure', success: 2, failure: 8 }, // 80% failure rate + { name: 'Medium Failure', success: 5, failure: 5 }, // 50% failure rate + ]; + + const workflows = await Promise.all( + testData.map(async (data) => await createWorkflow({ name: data.name }, project)), + ); + + const periodStart = DateTime.utc().startOf('day'); + + // Create insights events for each workflow individually to avoid race conditions + for (const [index, workflow] of workflows.entries()) { + const data = testData[index]; + + await createCompactedInsightsEvent(workflow, { + type: 'success', + value: data.success, + periodUnit: 'day', + periodStart, + }); + + await createCompactedInsightsEvent(workflow, { + type: 'failure', + value: data.failure, + periodUnit: 'day', + periodStart, + }); + } + + const response = await agents.owner + .get('/insights/by-workflow') + .query({ sortBy: 'failureRate:asc' }) + .expect(200); + + expect(response.body.data.count).toBe(3); + expect(response.body.data.data).toHaveLength(3); + + // Verify ascending order by failure rate + const expectedOrder = [0.1, 0.5, 0.8]; // 10%, 50%, 80% + response.body.data.data.forEach((item: any, index: number) => { + expect(item.failureRate).toBe(expectedOrder[index]); + }); + }); + }); }); diff --git a/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.test.ts b/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.test.ts new file mode 100644 index 0000000000..d03778ff16 --- /dev/null +++ b/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.test.ts @@ -0,0 +1,325 @@ +import { defineComponent } from 'vue'; +import { createTestingPinia } from '@pinia/testing'; +import { screen, within } from '@testing-library/vue'; +import { vi } from 'vitest'; +import userEvent from '@testing-library/user-event'; +import { createComponentRenderer } from '@/__tests__/render'; +import { useEmitters } from '@/__tests__/utils'; +import InsightsTableWorkflows from '@/features/insights/components/tables/InsightsTableWorkflows.vue'; +import type { InsightsByWorkflow } from '@n8n/api-types'; + +const { emitters, addEmitter } = useEmitters<'n8nDataTableServer'>(); + +// Mock telemetry +const mockTelemetry = { + track: vi.fn(), +}; +vi.mock('@/composables/useTelemetry', () => ({ + useTelemetry: () => mockTelemetry, +})); + +// Mock N8nDataTableServer like in SettingsUsersTable.test.ts +vi.mock('@n8n/design-system', async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + N8nDataTableServer: defineComponent({ + props: { + headers: { type: Array, required: true }, + items: { type: Array, required: true }, + itemsLength: { type: Number, required: true }, + sortBy: { type: Array }, + page: { type: Number }, + itemsPerPage: { type: Number }, + }, + setup(_, { emit }) { + addEmitter('n8nDataTableServer', emit); + }, + template: ` +
+
+
+ + {{ header.value(item) }} + {{ item[header.key] }} + +
+
+
`, + }), + }; +}); + +const mockInsightsData: InsightsByWorkflow = { + count: 2, + data: [ + { + workflowId: 'workflow-1', + workflowName: 'Test Workflow 1', + total: 100, + failed: 5, + failureRate: 0.05, + timeSaved: 3600, + averageRunTime: 1200, + projectName: 'Test Project 1', + projectId: 'project-1', + succeeded: 95, + runTime: 114000, + }, + { + workflowId: 'workflow-2', + workflowName: 'Test Workflow 2 With Very Long Name That Should Be Truncated', + total: 50, + failed: 0, + failureRate: 0, + timeSaved: 0, + averageRunTime: 800, + projectName: '', + projectId: 'project-2', + succeeded: 50, + runTime: 40000, + }, + ], +}; + +let renderComponent: ReturnType; + +describe('InsightsTableWorkflows', () => { + beforeEach(() => { + renderComponent = createComponentRenderer(InsightsTableWorkflows, { + pinia: createTestingPinia(), + props: { + data: mockInsightsData, + loading: false, + }, + global: { + stubs: { + 'router-link': { + template: '', + }, + }, + }, + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + describe('basic rendering', () => { + it('should display the correct heading', () => { + renderComponent(); + + expect(screen.getByRole('heading', { name: 'Workflow insights' })).toBeInTheDocument(); + }); + + it('should render workflow data in table rows', () => { + renderComponent(); + + expect(screen.getByTestId('workflow-row-workflow-1')).toBeInTheDocument(); + expect(screen.getByTestId('workflow-row-workflow-2')).toBeInTheDocument(); + }); + }); + + describe('data display', () => { + it('should display formatted numbers for total and failed columns', () => { + renderComponent(); + + const row1 = screen.getByTestId('workflow-row-workflow-1'); + expect(within(row1).getByText('100')).toBeInTheDocument(); + expect(within(row1).getByText('5')).toBeInTheDocument(); + + const row2 = screen.getByTestId('workflow-row-workflow-2'); + expect(within(row2).getByText('50')).toBeInTheDocument(); + expect(within(row2).getByText('0')).toBeInTheDocument(); + }); + + it('should handle large numbers with locale formatting', () => { + const largeNumberData = { + ...mockInsightsData, + data: [ + { + ...mockInsightsData.data[0], + total: 1000000, + failed: 50000, + }, + ], + }; + + renderComponent = createComponentRenderer(InsightsTableWorkflows, { + pinia: createTestingPinia(), + props: { + data: largeNumberData, + loading: false, + }, + }); + + renderComponent(); + + const row = screen.getByTestId('workflow-row-workflow-1'); + expect(within(row).getByText('1,000,000')).toBeInTheDocument(); + expect(within(row).getByText('50,000')).toBeInTheDocument(); + }); + }); + + describe('template slots', () => { + describe('workflowName slot', () => { + it('should render workflow name with tooltip', () => { + renderComponent(); + + const row1 = screen.getByTestId('workflow-row-workflow-1'); + expect(within(row1).getByText('Test Workflow 1')).toBeInTheDocument(); + }); + + it('should track telemetry on workflow name click', async () => { + const user = userEvent.setup(); + renderComponent(); + + const row1 = screen.getByTestId('workflow-row-workflow-1'); + const workflowLink = within(row1).getByText('Test Workflow 1'); + await user.click(workflowLink); + + expect(mockTelemetry.track).toHaveBeenCalledWith( + 'User clicked on workflow from insights table', + { + workflow_id: 'workflow-1', + }, + ); + }); + }); + + describe('timeSaved slot', () => { + it('should show estimate link when timeSaved is 0', () => { + renderComponent(); + + const row2 = screen.getByTestId('workflow-row-workflow-2'); + expect(within(row2).getByText('Estimate')).toBeInTheDocument(); + }); + + it('should show formatted value when timeSaved exists', () => { + renderComponent(); + + // The actual formatted value will be processed by the utility functions + // We just verify the slot is rendered with the item that has timeSaved + const row1 = screen.getByTestId('workflow-row-workflow-1'); + expect(row1).toBeInTheDocument(); + }); + }); + + describe('projectName slot', () => { + it('should render project name with tooltip when projectName exists', () => { + renderComponent(); + + const row1 = screen.getByTestId('workflow-row-workflow-1'); + expect(within(row1).getByText('Test Project 1')).toBeInTheDocument(); + }); + + it('should render dash when projectName is null', () => { + renderComponent(); + + const row2 = screen.getByTestId('workflow-row-workflow-2'); + expect(within(row2).getByText('-')).toBeInTheDocument(); + }); + }); + }); + + describe('event delegation', () => { + it('should delegate update:options event from N8nDataTableServer', () => { + const { emitted } = renderComponent(); + + const updateOptions = { + page: 1, + itemsPerPage: 20, + sortBy: [{ id: 'workflowName', desc: false }], + }; + + emitters.n8nDataTableServer.emit('update:options', updateOptions); + + expect(emitted()).toHaveProperty('update:options'); + expect(emitted()['update:options'][0]).toEqual([updateOptions]); + }); + }); + + describe('telemetry tracking', () => { + it('should track sort changes with correct payload', async () => { + const { rerender } = renderComponent(); + + // Simulate sortBy change + await rerender({ + sortBy: [{ id: 'total', desc: true }], + }); + + expect(mockTelemetry.track).toHaveBeenCalledWith('User sorted insights table', { + sorted_by: [ + { + id: 'total', + desc: true, + label: 'Prod. executions', + }, + ], + }); + }); + + it('should handle empty sortBy array', async () => { + const { rerender } = renderComponent(); + + // Simulate sortBy change to empty array + await rerender({ + sortBy: [], + }); + + expect(mockTelemetry.track).toHaveBeenCalledWith('User sorted insights table', { + sorted_by: [], + }); + }); + }); + + describe('edge cases', () => { + it('should handle empty data array', () => { + const emptyData = { count: 0, data: [] }; + + renderComponent = createComponentRenderer(InsightsTableWorkflows, { + pinia: createTestingPinia(), + props: { + data: emptyData, + loading: false, + }, + }); + + renderComponent(); + + expect(screen.getByTestId('insights-table')).toBeInTheDocument(); + expect(screen.queryByTestId('workflow-row-workflow-1')).not.toBeInTheDocument(); + }); + + it('should handle loading state', () => { + renderComponent = createComponentRenderer(InsightsTableWorkflows, { + pinia: createTestingPinia(), + props: { + data: mockInsightsData, + loading: true, + }, + }); + + renderComponent(); + + expect(screen.getByTestId('insights-table')).toBeInTheDocument(); + }); + + it('should handle missing optional props', () => { + renderComponent = createComponentRenderer(InsightsTableWorkflows, { + pinia: createTestingPinia(), + props: { + data: mockInsightsData, + // loading prop omitted + }, + }); + + renderComponent(); + + expect(screen.getByTestId('insights-table')).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.vue b/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.vue index 16b6791ba8..4e9f6a11d4 100644 --- a/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.vue +++ b/packages/frontend/editor-ui/src/features/insights/components/tables/InsightsTableWorkflows.vue @@ -7,10 +7,8 @@ import { transformInsightsTimeSaved, } from '@/features/insights/insights.utils'; import type { InsightsByWorkflow } from '@n8n/api-types'; -import { N8nTooltip } from '@n8n/design-system'; -import N8nDataTableServer, { - type TableHeader, -} from '@n8n/design-system/components/N8nDataTableServer/N8nDataTableServer.vue'; +import { N8nTooltip, N8nDataTableServer } from '@n8n/design-system'; +import type { TableHeader } from '@n8n/design-system/components/N8nDataTableServer'; import { smartDecimal } from '@n8n/utils/number/smartDecimal'; import { useTelemetry } from '@/composables/useTelemetry'; import { VIEWS } from '@/constants'; @@ -34,7 +32,6 @@ const headers = ref>>([ title: 'Name', key: 'workflowName', width: 400, - disableSort: true, }, { title: i18n.baseText('insights.banner.title.total'),