feat(core): Add a new by-time insights route that return only time saved data (#16727)

This commit is contained in:
Guillaume Jacquart
2025-06-27 11:45:31 +02:00
committed by GitHub
parent 8f9ce72dc4
commit 3ba8a84d2b
8 changed files with 260 additions and 30 deletions

View File

@@ -85,6 +85,19 @@ export const insightsByTimeDataSchemas = {
export const insightsByTimeSchema = z.object(insightsByTimeDataSchemas).strict();
export type InsightsByTime = z.infer<typeof insightsByTimeSchema>;
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<typeof restrictedInsightsByTimeSchema>;
export const insightsDateRangeSchema = z
.object({
key: z.enum(['day', 'week', '2weeks', 'month', 'quarter', '6months', 'year']),

View File

@@ -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<AuthenticatedRequest>(),
mock<Response>(),
{ 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<AuthenticatedRequest>(),
mock<Response>(),
{ 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<AuthenticatedRequest>(),
mock<Response>(),
{ 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,
},
},
]);
});
});
});

View File

@@ -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', () => {

View File

@@ -53,6 +53,7 @@ describe('InsightsByPeriodRepository', () => {
const result = await insightsByPeriodRepository.getInsightsByTime({
maxAgeInDays: 1,
periodUnit: 'day',
insightTypes: ['success', 'failure', 'time_saved_min'],
});
// ASSERT

View File

@@ -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<InsightsByPeriod> {
'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<InsightsByPeriod> {
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))

View File

@@ -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<InsightsByTime[]> {
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<RestrictedInsightsByTime[]> {
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[];
}
}

View File

@@ -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 { 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,
};
});
}

View File

@@ -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);
});
});