From ffc0a596e00d1cbf14ff2980894d52025bfc1187 Mon Sep 17 00:00:00 2001 From: Guillaume Jacquart Date: Wed, 26 Mar 2025 17:56:56 +0100 Subject: [PATCH] feat(API): Return null deviation on insights summary if previous period has no data (#14193) --- .../api-types/src/schemas/insights.schema.ts | 10 ++++---- .../__tests__/insights.controller.test.ts | 22 +++++++++--------- .../__tests__/insights.service.test.ts | 17 ++++++++++++++ .../src/modules/insights/insights.service.ts | 14 +++++++---- .../components/InsightsSummary.test.ts | 9 ++++++++ .../insights/components/InsightsSummary.vue | 2 +- .../InsightsSummary.test.ts.snap | 23 +++++++++++++++++++ .../src/features/insights/insights.types.ts | 2 +- .../src/features/insights/insights.utils.ts | 4 +++- 9 files changed, 79 insertions(+), 24 deletions(-) diff --git a/packages/@n8n/api-types/src/schemas/insights.schema.ts b/packages/@n8n/api-types/src/schemas/insights.schema.ts index 57f2608769..adb58448da 100644 --- a/packages/@n8n/api-types/src/schemas/insights.schema.ts +++ b/packages/@n8n/api-types/src/schemas/insights.schema.ts @@ -15,27 +15,27 @@ export type InsightsSummaryUnit = z.infer; export const insightsSummaryDataSchemas = { total: z.object({ value: z.number(), - deviation: z.number(), + deviation: z.union([z.null(), z.number()]), unit: z.literal('count'), }), failed: z.object({ value: z.number(), - deviation: z.number(), + deviation: z.union([z.null(), z.number()]), unit: z.literal('count'), }), failureRate: z.object({ value: z.number(), - deviation: z.number(), + deviation: z.union([z.null(), z.number()]), unit: z.literal('ratio'), }), timeSaved: z.object({ value: z.number(), - deviation: z.number(), + deviation: z.union([z.null(), z.number()]), unit: z.literal('time'), }), averageRunTime: z.object({ value: z.number(), - deviation: z.number(), + deviation: z.union([z.null(), z.number()]), unit: z.literal('time'), }), } as const; 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 c13f0b8f43..ffe2b630bf 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.controller.test.ts @@ -34,15 +34,15 @@ describe('InsightsController', () => { // ASSERT expect(response).toEqual({ - total: { deviation: 0, unit: 'count', value: 0 }, - failed: { deviation: 0, unit: 'count', value: 0 }, - failureRate: { deviation: 0, unit: 'ratio', value: 0 }, - averageRunTime: { deviation: 0, unit: 'time', value: 0 }, - timeSaved: { deviation: 0, unit: 'time', value: 0 }, + total: { deviation: null, unit: 'count', value: 0 }, + failed: { deviation: null, unit: 'count', value: 0 }, + failureRate: { deviation: null, unit: 'ratio', value: 0 }, + averageRunTime: { deviation: null, unit: 'time', value: 0 }, + timeSaved: { deviation: null, unit: 'time', value: 0 }, }); }); - it('should return the insights summary with deviation = current if insights exist only for current period', async () => { + it('should return the insights summary with null deviation if insights exist only for current period', async () => { // ARRANGE insightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates.mockResolvedValue([ { period: 'current', type: TypeToNumber.success, total_value: 20 }, @@ -56,11 +56,11 @@ describe('InsightsController', () => { // ASSERT expect(response).toEqual({ - total: { deviation: 30, unit: 'count', value: 30 }, - failed: { deviation: 10, unit: 'count', value: 10 }, - failureRate: { deviation: 0.33, unit: 'ratio', value: 0.33 }, - averageRunTime: { deviation: 10, unit: 'time', value: 10 }, - timeSaved: { deviation: 10, unit: 'time', value: 10 }, + total: { deviation: null, unit: 'count', value: 30 }, + failed: { deviation: null, unit: 'count', value: 10 }, + failureRate: { deviation: null, unit: 'ratio', value: 0.33 }, + averageRunTime: { deviation: null, unit: 'time', value: 10 }, + timeSaved: { deviation: null, unit: 'time', value: 10 }, }); }); 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 f529be4a3a..8dffc5f2b0 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts @@ -784,4 +784,21 @@ describe('getInsightsSummary', () => { total: { deviation: 3, unit: 'count', value: 4 }, }); }); + + test('no data for previous period should return null deviation', async () => { + // ARRANGE + // last 7 days + await createCompactedInsightsEvent(workflow, { + type: 'success', + value: 1, + periodUnit: 'day', + periodStart: DateTime.utc(), + }); + + // ACT + const summary = await insightsService.getInsightsSummary(); + + // ASSERT + expect(Object.values(summary).map((v) => v.deviation)).toEqual([null, null, null, null, null]); + }); }); diff --git a/packages/cli/src/modules/insights/insights.service.ts b/packages/cli/src/modules/insights/insights.service.ts index 54ef08b533..d2838b1eb2 100644 --- a/packages/cli/src/modules/insights/insights.service.ts +++ b/packages/cli/src/modules/insights/insights.service.ts @@ -265,32 +265,36 @@ export class InsightsService { const currentTimeSaved = getValueByType('current', 'time_saved_min'); const previousTimeSaved = getValueByType('previous', 'time_saved_min'); + // If the previous period has no executions, we discard deviation + const getDeviation = (current: number, previous: number) => + previousTotal === 0 ? null : current - previous; + // Return the formatted result const result: InsightsSummary = { averageRunTime: { value: currentAvgRuntime, unit: 'time', - deviation: currentAvgRuntime - previousAvgRuntime, + deviation: getDeviation(currentAvgRuntime, previousAvgRuntime), }, failed: { value: currentFailures, unit: 'count', - deviation: currentFailures - previousFailures, + deviation: getDeviation(currentFailures, previousFailures), }, failureRate: { value: currentFailureRate, unit: 'ratio', - deviation: currentFailureRate - previousFailureRate, + deviation: getDeviation(currentFailureRate, previousFailureRate), }, timeSaved: { value: currentTimeSaved, unit: 'time', - deviation: currentTimeSaved - previousTimeSaved, + deviation: getDeviation(currentTimeSaved, previousTimeSaved), }, total: { value: currentTotal, unit: 'count', - deviation: currentTotal - previousTotal, + deviation: getDeviation(currentTotal, previousTotal), }, }; diff --git a/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.test.ts b/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.test.ts index 7708f24c60..846fe2fd16 100644 --- a/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.test.ts +++ b/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.test.ts @@ -44,6 +44,15 @@ describe('InsightsSummary', () => { { id: 'averageRunTime', value: 2.5, deviation: 0.5, unit: 's' }, ], ], + [ + [ + { id: 'total', value: 525, deviation: null, unit: '' }, + { id: 'failed', value: 14, deviation: null, unit: '' }, + { id: 'failureRate', value: 1.9, deviation: null, unit: '%' }, + { id: 'timeSaved', value: 55.55555555555556, deviation: null, unit: 'h' }, + { id: 'averageRunTime', value: 2.5, deviation: null, unit: 's' }, + ], + ], ])('should render the summary correctly', (summary) => { const { html } = renderComponent({ props: { diff --git a/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.vue b/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.vue index e38678cf30..779ffb3cf6 100644 --- a/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.vue +++ b/packages/frontend/editor-ui/src/features/insights/components/InsightsSummary.vue @@ -76,7 +76,7 @@ const getImpactStyle = (id: keyof InsightsSummary, value: number) => { {{ smartDecimal(value) }} {{ unit }} - + should render the summary correctly 4`] = ` " `; + +exports[`InsightsSummary > should render the summary correctly 5`] = ` +"
+

Production executions for the last 7 days

+
    +
  • +

    Total525

    +
    • +
  • +

    Failed14

    +
    • +
  • +

    Failure rate1.9 %

    +
    • +
  • +

    Time saved55.56 h

    +
    • +
  • +

    Avg. run time2.5 s

    +
    • +
+
" +`; diff --git a/packages/frontend/editor-ui/src/features/insights/insights.types.ts b/packages/frontend/editor-ui/src/features/insights/insights.types.ts index 8665a8bc8e..a514f07f05 100644 --- a/packages/frontend/editor-ui/src/features/insights/insights.types.ts +++ b/packages/frontend/editor-ui/src/features/insights/insights.types.ts @@ -7,7 +7,7 @@ export type InsightsSummaryDisplay = Array< [K in keyof InsightsDisplayUnits]: { id: K; value: number; - deviation: number; + deviation: number | null; unit: InsightsDisplayUnits[K]; }; }[keyof InsightsDisplayUnits] diff --git a/packages/frontend/editor-ui/src/features/insights/insights.utils.ts b/packages/frontend/editor-ui/src/features/insights/insights.utils.ts index 82ecaa4dd7..1538993264 100644 --- a/packages/frontend/editor-ui/src/features/insights/insights.utils.ts +++ b/packages/frontend/editor-ui/src/features/insights/insights.utils.ts @@ -16,7 +16,9 @@ export const transformInsightsSummary = (data: InsightsSummary | null): Insights ? INSIGHTS_SUMMARY_ORDER.map((key) => ({ id: key, value: transformInsightsValues[key]?.(data[key].value) ?? data[key].value, - deviation: transformInsightsValues[key]?.(data[key].deviation) ?? data[key].deviation, + deviation: data[key].deviation + ? (transformInsightsValues[key]?.(data[key].deviation) ?? data[key].deviation) + : null, unit: INSIGHTS_UNIT_MAPPING[key], })) : [];