diff --git a/packages/@n8n/api-types/src/schemas/insights.schema.ts b/packages/@n8n/api-types/src/schemas/insights.schema.ts index 334a9f5ada..4c3195f339 100644 --- a/packages/@n8n/api-types/src/schemas/insights.schema.ts +++ b/packages/@n8n/api-types/src/schemas/insights.schema.ts @@ -85,6 +85,19 @@ export const insightsByTimeDataSchemas = { export const insightsByTimeSchema = z.object(insightsByTimeDataSchemas).strict(); export type InsightsByTime = z.infer; +export const restrictedInsightsByTimeDataSchema = { + date: z.string().refine((val) => !isNaN(Date.parse(val)) && new Date(val).toISOString() === val, { + message: 'Invalid date format, must be ISO 8601 format', + }), + values: z + .object({ + timeSaved: z.number(), + }) + .strict(), +} as const; +export const restrictedInsightsByTimeSchema = z.object(restrictedInsightsByTimeDataSchema).strict(); +export type RestrictedInsightsByTime = z.infer; + export const insightsDateRangeSchema = z .object({ key: z.enum(['day', 'week', '2weeks', 'month', 'quarter', '6months', 'year']), diff --git a/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts b/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts index 72ba3fe9d3..3231d9db36 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts @@ -104,4 +104,115 @@ describe('InsightsController', () => { }); }); }); + + describe('getInsightsByTime', () => { + it('should return insights by time with empty data', async () => { + // ARRANGE + insightsByPeriodRepository.getInsightsByTime.mockResolvedValue([]); + + // ACT + const response = await controller.getInsightsByTime( + mock(), + mock(), + { dateRange: 'week' }, + ); + + // ASSERT + expect(response).toEqual([]); + }); + + it('should return insights by time with all data', async () => { + // ARRANGE + const mockData = [ + { + periodStart: '2023-10-01T00:00:00.000Z', + succeeded: 10, + timeSaved: 0, + failed: 2, + runTime: 10, + }, + { + periodStart: '2023-10-02T00:00:00.000Z', + succeeded: 12, + timeSaved: 0, + failed: 4, + runTime: 10, + }, + ]; + insightsByPeriodRepository.getInsightsByTime.mockResolvedValue(mockData); + + // ACT + const response = await controller.getInsightsByTime( + mock(), + mock(), + { dateRange: 'year' }, + ); + + // ASSERT + expect(response).toEqual([ + { + date: '2023-10-01T00:00:00.000Z', + values: { + succeeded: 10, + timeSaved: 0, + failed: 2, + averageRunTime: 10 / 12, + failureRate: 2 / 12, + total: 12, + }, + }, + { + date: '2023-10-02T00:00:00.000Z', + values: { + succeeded: 12, + timeSaved: 0, + failed: 4, + averageRunTime: 10 / 16, + failureRate: 4 / 16, + total: 16, + }, + }, + ]); + }); + }); + + describe('getTimeSavedInsightsByTime', () => { + it('should return insights by time with limited data', async () => { + // ARRANGE + const mockData = [ + { + periodStart: '2023-10-01T00:00:00.000Z', + timeSaved: 0, + }, + { + periodStart: '2023-10-02T00:00:00.000Z', + timeSaved: 2, + }, + ]; + insightsByPeriodRepository.getInsightsByTime.mockResolvedValue(mockData); + + // ACT + const response = await controller.getTimeSavedInsightsByTime( + mock(), + mock(), + { dateRange: 'week' }, + ); + + // ASSERT + expect(response).toEqual([ + { + date: '2023-10-01T00:00:00.000Z', + values: { + timeSaved: 0, + }, + }, + { + date: '2023-10-02T00:00:00.000Z', + values: { + timeSaved: 2, + }, + }, + ]); + }); + }); }); diff --git a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts index b13c055b71..dc8a220a3d 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts @@ -578,6 +578,53 @@ describe('getInsightsByTime', () => { timeSaved: 0, }); }); + + test('compacted data with limited insight types are grouped by time correctly', async () => { + // ARRANGE + for (const workflow of [workflow1, workflow2]) { + await createCompactedInsightsEvent(workflow, { + type: 'success', + value: workflow === workflow1 ? 1 : 2, + periodUnit: 'day', + periodStart: DateTime.utc(), + }); + await createCompactedInsightsEvent(workflow, { + type: 'failure', + value: 2, + periodUnit: 'day', + periodStart: DateTime.utc(), + }); + await createCompactedInsightsEvent(workflow, { + type: 'time_saved_min', + value: workflow === workflow1 ? 10 : 20, + periodUnit: 'day', + periodStart: DateTime.utc().minus({ days: 10 }), + }); + } + + // ACT + const byTime = await insightsService.getInsightsByTime({ + maxAgeInDays: 14, + periodUnit: 'day', + insightTypes: ['time_saved_min', 'failure'], + }); + + // ASSERT + expect(byTime).toHaveLength(2); + + // expect results to contain only failure and time saved insights + expect(byTime[0].date).toEqual(DateTime.utc().minus({ days: 10 }).startOf('day').toISO()); + expect(byTime[0].values).toEqual({ + timeSaved: 30, + failed: 0, + }); + + expect(byTime[1].date).toEqual(DateTime.utc().startOf('day').toISO()); + expect(byTime[1].values).toEqual({ + timeSaved: 0, + failed: 4, + }); + }); }); describe('getAvailableDateRanges', () => { diff --git a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts index 566cb58517..aafb30377f 100644 --- a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts +++ b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.test.ts @@ -53,6 +53,7 @@ describe('InsightsByPeriodRepository', () => { const result = await insightsByPeriodRepository.getInsightsByTime({ maxAgeInDays: 1, periodUnit: 'day', + insightTypes: ['success', 'failure', 'time_saved_min'], }); // ASSERT diff --git a/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts b/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts index 5f901cfd64..e7e65a0f71 100644 --- a/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts +++ b/packages/cli/src/modules/insights/database/repositories/insights-by-period.repository.ts @@ -7,10 +7,16 @@ import { DateTime } from 'luxon'; import { z } from 'zod'; import { InsightsByPeriod } from '../entities/insights-by-period'; -import type { PeriodUnit } from '../entities/insights-shared'; +import type { PeriodUnit, TypeUnit } from '../entities/insights-shared'; import { PeriodUnitToNumber, TypeToNumber } from '../entities/insights-shared'; const dbType = Container.get(GlobalConfig).database.type; +const displayTypeName = { + [TypeToNumber.success]: 'succeeded', + [TypeToNumber.failure]: 'failed', + [TypeToNumber.runtime_ms]: 'runTime', + [TypeToNumber.time_saved_min]: 'timeSaved', +}; const summaryParser = z .object({ @@ -38,6 +44,11 @@ const aggregatedInsightsByWorkflowParser = z }) .array(); +const optionalNumberLike = z + .union([z.number(), z.string()]) + .optional() + .transform((value) => (value !== undefined ? Number(value) : undefined)); + const aggregatedInsightsByTimeParser = z .object({ periodStart: z.union([z.date(), z.string()]).transform((value) => { @@ -53,10 +64,10 @@ const aggregatedInsightsByTimeParser = z // fallback on native date parsing return new Date(value).toISOString(); }), - runTime: z.union([z.number(), z.string()]).transform((value) => Number(value)), - succeeded: z.union([z.number(), z.string()]).transform((value) => Number(value)), - failed: z.union([z.number(), z.string()]).transform((value) => Number(value)), - timeSaved: z.union([z.number(), z.string()]).transform((value) => Number(value)), + runTime: optionalNumberLike, + succeeded: optionalNumberLike, + failed: optionalNumberLike, + timeSaved: optionalNumberLike, }) .array(); @@ -340,15 +351,15 @@ export class InsightsByPeriodRepository extends Repository { 'metadata.workflowName AS "workflowName"', 'metadata.projectId AS "projectId"', 'metadata.projectName AS "projectName"', - `SUM(CASE WHEN insights.type = ${TypeToNumber.success} THEN value ELSE 0 END) AS "succeeded"`, - `SUM(CASE WHEN insights.type = ${TypeToNumber.failure} THEN value ELSE 0 END) AS "failed"`, + `SUM(CASE WHEN insights.type = ${TypeToNumber.success} THEN value ELSE 0 END) AS "${displayTypeName[TypeToNumber.success]}"`, + `SUM(CASE WHEN insights.type = ${TypeToNumber.failure} THEN value ELSE 0 END) AS "${displayTypeName[TypeToNumber.failure]}"`, `SUM(CASE WHEN insights.type IN (${TypeToNumber.success}, ${TypeToNumber.failure}) THEN value ELSE 0 END) AS "total"`, sql`CASE WHEN ${sumOfExecutions} = 0 THEN 0 ELSE 1.0 * SUM(CASE WHEN insights.type = ${TypeToNumber.failure.toString()} THEN value ELSE 0 END) / ${sumOfExecutions} END AS "failureRate"`, - `SUM(CASE WHEN insights.type = ${TypeToNumber.runtime_ms} THEN value ELSE 0 END) AS "runTime"`, - `SUM(CASE WHEN insights.type = ${TypeToNumber.time_saved_min} THEN value ELSE 0 END) AS "timeSaved"`, + `SUM(CASE WHEN insights.type = ${TypeToNumber.runtime_ms} THEN value ELSE 0 END) AS "${displayTypeName[TypeToNumber.runtime_ms]}"`, + `SUM(CASE WHEN insights.type = ${TypeToNumber.time_saved_min} THEN value ELSE 0 END) AS "${displayTypeName[TypeToNumber.time_saved_min]}"`, sql`CASE WHEN ${sumOfExecutions} = 0 THEN 0 ELSE 1.0 * SUM(CASE WHEN insights.type = ${TypeToNumber.runtime_ms.toString()} THEN value ELSE 0 END) / ${sumOfExecutions} @@ -373,17 +384,17 @@ export class InsightsByPeriodRepository extends Repository { async getInsightsByTime({ maxAgeInDays, periodUnit, - }: { maxAgeInDays: number; periodUnit: PeriodUnit }) { + insightTypes, + }: { maxAgeInDays: number; periodUnit: PeriodUnit; insightTypes: TypeUnit[] }) { const cte = sql`SELECT ${this.getAgeLimitQuery(maxAgeInDays)} AS start_date`; + + const typesAggregation = insightTypes.map((type) => { + return `SUM(CASE WHEN type = ${TypeToNumber[type]} THEN value ELSE 0 END) AS "${displayTypeName[TypeToNumber[type]]}"`; + }); + const rawRowsQuery = this.createQueryBuilder() .addCommonTableExpression(cte, 'date_range') - .select([ - `${this.getPeriodStartExpr(periodUnit)} as "periodStart"`, - `SUM(CASE WHEN type = ${TypeToNumber.runtime_ms} THEN value ELSE 0 END) AS "runTime"`, - `SUM(CASE WHEN type = ${TypeToNumber.success} THEN value ELSE 0 END) AS "succeeded"`, - `SUM(CASE WHEN type = ${TypeToNumber.failure} THEN value ELSE 0 END) AS "failed"`, - `SUM(CASE WHEN type = ${TypeToNumber.time_saved_min} THEN value ELSE 0 END) AS "timeSaved"`, - ]) + .select([`${this.getPeriodStartExpr(periodUnit)} as "periodStart"`, ...typesAggregation]) .innerJoin('date_range', 'date_range', '1=1') .where(`${this.escapeField('periodStart')} >= date_range.start_date`) .groupBy(this.getPeriodStartExpr(periodUnit)) diff --git a/packages/cli/src/modules/insights/insights.controller.ts b/packages/cli/src/modules/insights/insights.controller.ts index 20309a2282..2d00c9e2bb 100644 --- a/packages/cli/src/modules/insights/insights.controller.ts +++ b/packages/cli/src/modules/insights/insights.controller.ts @@ -1,5 +1,6 @@ import { InsightsDateFilterDto, ListInsightsWorkflowQueryDto } from '@n8n/api-types'; import type { InsightsSummary, InsightsByTime, InsightsByWorkflow } from '@n8n/api-types'; +import type { RestrictedInsightsByTime } from '@n8n/api-types/src/schemas/insights.schema'; import { AuthenticatedRequest } from '@n8n/db'; import { Get, GlobalScope, Licensed, Query, RestController } from '@n8n/decorators'; import type { UserError } from 'n8n-workflow'; @@ -71,9 +72,34 @@ export class InsightsController { @Query payload: InsightsDateFilterDto, ): Promise { const dateRangeAndMaxAgeInDays = this.getMaxAgeInDaysAndGranularity(payload); - return await this.insightsService.getInsightsByTime({ + + // Cast to full insights by time type + // as the service returns all types by default + return (await this.insightsService.getInsightsByTime({ maxAgeInDays: dateRangeAndMaxAgeInDays.maxAgeInDays, periodUnit: dateRangeAndMaxAgeInDays.granularity, - }); + })) as InsightsByTime[]; + } + + /** + * This endpoint is used to get the time saved insights by time. + * time data for time saved insights is not restricted by the license + */ + @Get('/by-time/time-saved') + @GlobalScope('insights:list') + async getTimeSavedInsightsByTime( + _req: AuthenticatedRequest, + _res: Response, + @Query payload: InsightsDateFilterDto, + ): Promise { + const dateRangeAndMaxAgeInDays = this.getMaxAgeInDaysAndGranularity(payload); + + // Cast to restricted insights by time type + // as the service returns only time saved data + return (await this.insightsService.getInsightsByTime({ + maxAgeInDays: dateRangeAndMaxAgeInDays.maxAgeInDays, + periodUnit: dateRangeAndMaxAgeInDays.granularity, + insightTypes: ['time_saved_min'], + })) as RestrictedInsightsByTime[]; } } diff --git a/packages/cli/src/modules/insights/insights.service.ts b/packages/cli/src/modules/insights/insights.service.ts index c6987e6218..0f9a022d4d 100644 --- a/packages/cli/src/modules/insights/insights.service.ts +++ b/packages/cli/src/modules/insights/insights.service.ts @@ -6,7 +6,7 @@ import { InstanceSettings } from 'n8n-core'; import { UserError } from 'n8n-workflow'; import type { PeriodUnit, TypeUnit } from './database/entities/insights-shared'; -import { NumberToType } from './database/entities/insights-shared'; +import { NumberToType, TypeToNumber } from './database/entities/insights-shared'; import { InsightsByPeriodRepository } from './database/repositories/insights-by-period.repository'; import { InsightsCollectionService } from './insights-collection.service'; import { InsightsCompactionService } from './insights-compaction.service'; @@ -174,24 +174,36 @@ export class InsightsService { async getInsightsByTime({ maxAgeInDays, periodUnit, - }: { maxAgeInDays: number; periodUnit: PeriodUnit }) { + // Default to all insight types + insightTypes = Object.keys(TypeToNumber) as TypeUnit[], + }: { maxAgeInDays: number; periodUnit: PeriodUnit; insightTypes?: TypeUnit[] }) { const rows = await this.insightsByPeriodRepository.getInsightsByTime({ maxAgeInDays, periodUnit, + insightTypes, }); return rows.map((r) => { - const total = r.succeeded + r.failed; + const { periodStart, runTime, ...rest } = r; + const values: typeof rest & { + total?: number; + successRate?: number; + failureRate?: number; + averageRunTime?: number; + } = rest; + + // Compute ratio if total has been computed + if (typeof r.succeeded === 'number' && typeof r.failed === 'number') { + const total = r.succeeded + r.failed; + values.total = total; + values.failureRate = total ? r.failed / total : 0; + if (typeof runTime === 'number') { + values.averageRunTime = total ? runTime / total : 0; + } + } return { date: r.periodStart, - values: { - total, - succeeded: r.succeeded, - failed: r.failed, - failureRate: r.failed / total, - averageRunTime: r.runTime / total, - timeSaved: r.timeSaved, - }, + values, }; }); } diff --git a/packages/cli/test/integration/insights/insights.api.test.ts b/packages/cli/test/integration/insights/insights.api.test.ts index b5ed6d0a39..0620573f48 100644 --- a/packages/cli/test/integration/insights/insights.api.test.ts +++ b/packages/cli/test/integration/insights/insights.api.test.ts @@ -33,6 +33,9 @@ describe('GET /insights routes work for owner and admins for server with dashboa const authAgent = agents[agentName]; await authAgent.get('/insights/summary').expect(agentName.includes('member') ? 403 : 200); await authAgent.get('/insights/by-time').expect(agentName.includes('member') ? 403 : 200); + await authAgent + .get('/insights/by-time/time-saved') + .expect(agentName.includes('member') ? 403 : 200); await authAgent.get('/insights/by-workflow').expect(agentName.includes('member') ? 403 : 200); }, ); @@ -48,6 +51,9 @@ describe('GET /insights routes return 403 for dashboard routes when summary lice const authAgent = agents[agentName]; await authAgent.get('/insights/summary').expect(agentName.includes('member') ? 403 : 200); await authAgent.get('/insights/by-time').expect(403); + await authAgent + .get('/insights/by-time/time-saved') + .expect(agentName.includes('member') ? 403 : 200); await authAgent.get('/insights/by-workflow').expect(403); }, ); @@ -62,6 +68,7 @@ describe('GET /insights routes return 403 if date range outside license limits', const authAgent = agents.admin; await authAgent.get('/insights/summary').expect(403); await authAgent.get('/insights/by-time').expect(403); + await authAgent.get('/insights/by-time/time-saved').expect(403); await authAgent.get('/insights/by-workflow').expect(403); }); @@ -69,6 +76,7 @@ describe('GET /insights routes return 403 if date range outside license limits', const authAgent = agents.admin; await authAgent.get('/insights/summary?dateRange=day').expect(403); await authAgent.get('/insights/by-time?dateRange=day').expect(403); + await authAgent.get('/insights/by-time/time-saved?dateRange=day').expect(403); await authAgent.get('/insights/by-workflow?dateRange=day').expect(403); }); }); @@ -97,6 +105,7 @@ describe('GET /insights routes return 200 if date range inside license limits', const authAgent = agents.admin; await authAgent.get(`/insights/summary?dateRange=${dateRange}`).expect(200); await authAgent.get(`/insights/by-time?dateRange=${dateRange}`).expect(200); + await authAgent.get(`/insights/by-time/time-saved?dateRange=${dateRange}`).expect(200); await authAgent.get(`/insights/by-workflow?dateRange=${dateRange}`).expect(200); }); });