From 1dc45d8c74e3697bc86e3d3b3b2e10979a4ab828 Mon Sep 17 00:00:00 2001 From: Ihor Mykhno Date: Mon, 7 Sep 2026 12:25:14 +0200 Subject: [PATCH 1/4] feat(scorecard): include aggregation chart display color in scalar aggregation response Signed-off-by: Ihor Mykhno imykhno@redhat.com Assisted-By: Cursor --- .../scorecard/.changeset/hungry-walls-burn.md | 7 ++ .../mockAggregatedMetricResult.ts | 1 + .../scorecard-backend/docs/aggregation.md | 1 + .../src/constants/aggregationKPIs.ts | 21 +++++ .../strategies/ScalarAggregationStrategy.ts | 18 +++- .../WeightedStatusScoreAggregationStrategy.ts | 36 ++----- .../scalarAggregationStrategy.test.ts | 71 +++++++++++++- ...htedStatusScoreAggregationStrategy.test.ts | 34 +++++++ .../src/service/mappers.test.ts | 2 + .../src/service/router.test.ts | 3 +- .../getAggregationChartDisplayColor.test.ts | 94 +++++++++++++++++++ .../getAggregationChartDisplayColor.ts | 53 +++++++++++ .../plugins/scorecard-common/report.api.md | 3 +- .../scorecard-common/src/types/aggregation.ts | 3 +- .../WeightedStatusScoreCardComponent.tsx | 7 +- .../__tests__/ScorecardHomepageCard.test.tsx | 1 + 16 files changed, 315 insertions(+), 40 deletions(-) create mode 100644 workspaces/scorecard/.changeset/hungry-walls-burn.md create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts diff --git a/workspaces/scorecard/.changeset/hungry-walls-burn.md b/workspaces/scorecard/.changeset/hungry-walls-burn.md new file mode 100644 index 00000000000..e89d52ac602 --- /dev/null +++ b/workspaces/scorecard/.changeset/hungry-walls-burn.md @@ -0,0 +1,7 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-common': minor +--- + +Skip scalar aggregation threshold coloring when no successful samples contributed (`total` is 0); return a null display color and keep the card grey fallback. diff --git a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockAggregatedMetricResult.ts b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockAggregatedMetricResult.ts index 2b80e44b821..50ce0ea99d9 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockAggregatedMetricResult.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockAggregatedMetricResult.ts @@ -53,6 +53,7 @@ export const mockScalarAggregationResult: ScalarAggregationResult = { timestamp: '2025-01-01T10:30:00.000Z', entitiesConsidered: 2, calculationErrorCount: 0, + aggregationChartDisplayColor: 'warning.main', }; export const mockWeightedStatusScoreAggregationResult: WeightedStatusScoreAggregationResult = diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md index bb370356406..a2b30b48a24 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md @@ -306,6 +306,7 @@ Example scalar response with status filter: "entitiesConsidered": 10, "calculationErrorCount": 1, "timestamp": "2026-02-17T10:30:00.000Z", + "aggregationChartDisplayColor": "rgb(224, 189, 108)", "thresholds": { "rules": [ { "key": "success", "expression": "<100" }, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts index 1c1961a348b..d5831fcfee7 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts @@ -46,3 +46,24 @@ export const DEFAULT_WEIGHTED_STATUS_SCORE_KPI_RESULT_THRESHOLDS: ThresholdConfi }, ], }; + +export const DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS: ThresholdConfig = + { + rules: [ + { + key: 'success', + expression: '<10', + color: ScorecardThresholdRuleColors.SUCCESS, + }, + { + key: 'warning', + expression: '10-50', + color: ScorecardThresholdRuleColors.WARNING, + }, + { + key: 'error', + expression: '>50', + color: ScorecardThresholdRuleColors.ERROR, + }, + ], + }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts index cf02b76297e..0f24c1970a0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts @@ -31,6 +31,8 @@ import type { AggregationStrategy } from './types'; import { isScalarAggregationConfig } from '../../../utils/aggregation/isScalarAggregationConfig'; import { classifyNumberAgainstThresholds } from '../../../utils/aggregation/classifyNumberAgainstThresholds'; import { ThresholdEvaluator } from '../../../threshold/ThresholdEvaluator'; +import { getRequiredAggregationChartDisplayColor } from '../../../utils/aggregation/getAggregationChartDisplayColor'; +import { DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS } from '../../../constants'; export class ScalarAggregationStrategy implements AggregationStrategy { constructor( @@ -50,8 +52,7 @@ export class ScalarAggregationStrategy implements AggregationStrategy { ); } - const { thresholds: headlineThresholds = DEFAULT_NUMBER_THRESHOLDS } = - aggregationConfig.options ?? {}; + const { thresholds: headlineThresholds } = aggregationConfig.options ?? {}; const { value, @@ -66,13 +67,24 @@ export class ScalarAggregationStrategy implements AggregationStrategy { aggregationConfig.filter, ); + const aggregationChartDisplayColor = + total > 0 + ? getRequiredAggregationChartDisplayColor( + value, + headlineThresholds ?? + DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS, + `The color for value '${value}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, + ) + : null; + const result = { value, total, entitiesConsidered, calculationErrorCount, timestamp, - thresholds: headlineThresholds, + aggregationChartDisplayColor, + thresholds: headlineThresholds ?? DEFAULT_NUMBER_THRESHOLDS, } satisfies ScalarAggregationResult; return AggregatedMetricMapper.toAggregatedMetricResult( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts index 64a53f663fa..f03c5b9c2b4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts @@ -18,7 +18,6 @@ import { type AggregatedMetric, type WeightedStatusScoreAggregationResult, type AggregatedMetricResult, - type ThresholdConfig, ThresholdRule, aggregationTypes, type StatusScoreAggregationOption, @@ -29,7 +28,7 @@ import type { AggregatedMetricLoader } from '../AggregatedMetricLoader'; import type { AggregationOptions } from '../types'; import type { AggregationStrategy } from './types'; import { LoggerService } from '@backstage/backend-plugin-api'; -import { ThresholdEvaluator } from '../../../threshold/ThresholdEvaluator'; +import { getRequiredAggregationChartDisplayColor } from '../../../utils/aggregation/getAggregationChartDisplayColor'; export class WeightedStatusScoreAggregationStrategy implements AggregationStrategy @@ -77,16 +76,14 @@ export class WeightedStatusScoreAggregationStrategy weightedSum, ); - const aggregationChartDisplayColor = this.getAggregationChartDisplayColor( - weightedStatusScore, - headlineThresholds, - ); - - if (!aggregationChartDisplayColor) { - throw new Error( - `The color for percentage '${weightedStatusScore}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, - ); - } + const aggregationChartDisplayColor = + aggregatedMetric.total > 0 + ? getRequiredAggregationChartDisplayColor( + weightedStatusScore, + headlineThresholds, + `The color for percentage '${weightedStatusScore}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, + ) + : null; const result = { total: aggregatedMetric.total, @@ -131,21 +128,6 @@ export class WeightedStatusScoreAggregationStrategy return weightedSum; } - private getAggregationChartDisplayColor( - scorePercent: number, - thresholds: ThresholdConfig, - ): string | undefined { - const thresholdEvaluator = new ThresholdEvaluator(); - - const matchedThresholdKey = thresholdEvaluator.getFirstMatchingThreshold( - scorePercent, - 'number', - thresholds, - ); - - return thresholds.rules.find(r => r.key === matchedThresholdKey)?.color; - } - private prepareWeightedStatusScoreValues( numberOfEntities: Pick['total'], statusScores: StatusScoreAggregationOption, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts index fece891807a..6e59a405142 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/scalarAggregationStrategy.test.ts @@ -129,7 +129,11 @@ describe('ScalarAggregationStrategy', () => { expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( metric, - { ...loadedScalarMetric, thresholds: DEFAULT_NUMBER_THRESHOLDS }, + { + ...loadedScalarMetric, + thresholds: DEFAULT_NUMBER_THRESHOLDS, + aggregationChartDisplayColor: 'error.main', + }, defaultAggregationConfig, ); }); @@ -144,7 +148,64 @@ describe('ScalarAggregationStrategy', () => { expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( metric, - { ...loadedScalarMetric, thresholds: mockHigherIsBetterThresholds }, + { + ...loadedScalarMetric, + thresholds: mockHigherIsBetterThresholds, + aggregationChartDisplayColor: 'green', + }, + aggregationConfig, + ); + }); + + it('should throw when aggregation chart display color is not configured', async () => { + const aggregationConfigWithoutColors = mockScalarAggregationConfig( + aggregationTypes.sum, + { + id: 'totalOpenPrs', + metricId: metric.id, + options: { + thresholds: { + rules: [{ key: 'success', expression: '<10' }], + }, + }, + }, + ); + + await expect(() => + strategy.aggregate({ + metric, + entityRefs, + thresholds: mockHigherIsBetterThresholds, + aggregationConfig: aggregationConfigWithoutColors, + }), + ).rejects.toThrow( + `The color for value '${loadedScalarMetric.value}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.totalOpenPrs.options.thresholds' configuration.`, + ); + }); + + it('should set aggregationChartDisplayColor to null when total is 0', async () => { + (loader.loadScalarMetricByEntityRefs as jest.Mock).mockResolvedValueOnce({ + ...loadedScalarMetric, + value: 0, + total: 0, + }); + + await strategy.aggregate({ + metric, + entityRefs, + thresholds: mockHigherIsBetterThresholds, + aggregationConfig, + }); + + expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( + metric, + { + ...loadedScalarMetric, + value: 0, + total: 0, + thresholds: mockHigherIsBetterThresholds, + aggregationChartDisplayColor: null, + }, aggregationConfig, ); }); @@ -200,7 +261,11 @@ describe('ScalarAggregationStrategy', () => { expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( metric, - { ...loadedScalarMetric, thresholds: mockHigherIsBetterThresholds }, + { + ...loadedScalarMetric, + thresholds: mockHigherIsBetterThresholds, + aggregationChartDisplayColor: 'green', + }, aggregationConfig, ); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts index 4440f091d92..49c82f62f56 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts @@ -193,6 +193,40 @@ describe('WeightedStatusScoreAggregationStrategy', () => { ); }); + it('should set aggregationChartDisplayColor to null when total is 0', async () => { + ( + loader.loadStatusGroupedMetricByEntityRefs as jest.Mock + ).mockResolvedValueOnce({ + ...loadedStatusGroupedMetric, + values: {}, + total: 0, + }); + + await strategy.aggregate({ + metric, + entityRefs, + thresholds: mockHigherIsBetterThresholds, + aggregationConfig, + }); + + expect(spyMethods.toAggregatedMetricResultSpy).toHaveBeenCalledWith( + metric, + { + ...mappedWeightedResult, + values: [ + { name: 'success', count: 0, score: 100 }, + { name: 'error', count: 0, score: 0 }, + ], + weightedStatusScore: 0, + weightedStatusSum: 0, + weightedStatusMaxPossible: 0, + aggregationChartDisplayColor: null, + total: 0, + }, + aggregationConfig, + ); + }); + it('should get aggregation result', async () => { const result = await strategy.aggregate({ metric, diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts index 25d93d4a56f..1fe8ce425fe 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/mappers.test.ts @@ -348,6 +348,7 @@ describe('AggregatedMetricMapper', () => { calculationErrorCount: 1, timestamp: '2024-01-15T10:00:00.000Z', thresholds, + aggregationChartDisplayColor: 'warning.main', }, aggregationConfig, ); @@ -369,6 +370,7 @@ describe('AggregatedMetricMapper', () => { calculationErrorCount: 1, timestamp: '2024-01-15T10:00:00.000Z', thresholds, + aggregationChartDisplayColor: 'warning.main', }, aggregationConfig, ); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts index cae23c10960..1dfa37b2035 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/router.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Config } from '@backstage/config'; +import type { Config } from '@backstage/config'; import { mockErrorHandler, mockServices, @@ -1619,6 +1619,7 @@ describe('createRouter', () => { entitiesConsidered: 45, calculationErrorCount: 3, timestamp: '2025-01-01T10:30:00.000Z', + aggregationChartDisplayColor: 'error.main', thresholds: DEFAULT_NUMBER_THRESHOLDS, }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts new file mode 100644 index 00000000000..827b9973aca --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts @@ -0,0 +1,94 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + getAggregationChartDisplayColor, + getRequiredAggregationChartDisplayColor, +} from './getAggregationChartDisplayColor'; + +const overlappingThresholds = { + rules: [ + { + key: 'error', + expression: '>50', + color: 'red', + }, + { + key: 'warning', + expression: '12-50', + color: 'yellow', + }, + { + key: 'success', + expression: '<13', + color: 'green', + }, + ], +}; + +describe('getAggregationChartDisplayColor', () => { + it('should return undefined when no rule matches', () => { + expect( + getAggregationChartDisplayColor(50, { + rules: [{ key: 'success', expression: '<10', color: 'green' }], + }), + ).toBeUndefined(); + }); + + it('should return undefined when the matching rule has no color', () => { + expect( + getAggregationChartDisplayColor(5, { + rules: [{ key: 'success', expression: '<10' }], + }), + ).toBeUndefined(); + }); + + it('should return the color of the first matching rule', () => { + expect(getAggregationChartDisplayColor(12, overlappingThresholds)).toBe( + 'yellow', + ); + }); + + it('should follow rule order when multiple expressions match', () => { + expect( + getAggregationChartDisplayColor(12, { + rules: [...overlappingThresholds.rules].reverse(), + }), + ).toBe('green'); + }); +}); + +describe('getRequiredAggregationChartDisplayColor', () => { + it('should throw the given error when no color matches', () => { + expect(() => + getRequiredAggregationChartDisplayColor( + 50, + { rules: [{ key: 'success', expression: '<10', color: 'green' }] }, + 'color is not configured', + ), + ).toThrow('color is not configured'); + }); + + it('should return the matching color', () => { + expect( + getRequiredAggregationChartDisplayColor( + 12, + overlappingThresholds, + 'color is not configured', + ), + ).toBe('yellow'); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts new file mode 100644 index 00000000000..535cb3c5adf --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts @@ -0,0 +1,53 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import type { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; +import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; + +/** + * Get the aggregation chart display color for a given value and thresholds. + * @param value - The value to get the color for. + * @param thresholds - The thresholds to use. + * @returns The aggregation chart display color. + */ +export function getAggregationChartDisplayColor( + value: number, + thresholds: ThresholdConfig, +): string | undefined { + const thresholdEvaluator = new ThresholdEvaluator(); + + const matchedThresholdKey = thresholdEvaluator.getFirstMatchingThreshold( + value, + 'number', + thresholds, + ); + + return thresholds.rules.find(r => r.key === matchedThresholdKey)?.color; +} + +export function getRequiredAggregationChartDisplayColor( + value: number, + thresholds: ThresholdConfig, + errorMessage: string, +): string { + const color = getAggregationChartDisplayColor(value, thresholds); + + if (!color) { + throw new Error(errorMessage); + } + + return color; +} diff --git a/workspaces/scorecard/plugins/scorecard-common/report.api.md b/workspaces/scorecard/plugins/scorecard-common/report.api.md index a655b08089a..6a3ca5d009b 100644 --- a/workspaces/scorecard/plugins/scorecard-common/report.api.md +++ b/workspaces/scorecard/plugins/scorecard-common/report.api.md @@ -241,6 +241,7 @@ export type ScalarAggregatedTimeSeriesPoint = { // @public (undocumented) export type ScalarAggregationResult = ScalarAggregatedMetric & { thresholds: ThresholdConfig; + aggregationChartDisplayColor: string | null; }; // @public @@ -339,7 +340,7 @@ export type WeightedStatusScoreAggregationResult = weightedStatusScore: number; weightedStatusSum: number; weightedStatusMaxPossible: number; - aggregationChartDisplayColor: string; + aggregationChartDisplayColor: string | null; }; // (No @packageDocumentation comment for this package) diff --git a/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts b/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts index a7008994afa..21d05c7cc3a 100644 --- a/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts +++ b/workspaces/scorecard/plugins/scorecard-common/src/types/aggregation.ts @@ -100,7 +100,7 @@ export type WeightedStatusScoreAggregationResult = weightedStatusScore: number; weightedStatusSum: number; weightedStatusMaxPossible: number; - aggregationChartDisplayColor: string; + aggregationChartDisplayColor: string | null; }; /** @@ -108,6 +108,7 @@ export type WeightedStatusScoreAggregationResult = */ export type ScalarAggregationResult = ScalarAggregatedMetric & { thresholds: ThresholdConfig; + aggregationChartDisplayColor: string | null; }; /** diff --git a/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/WeightedStatusScoreCard/WeightedStatusScoreCardComponent.tsx b/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/WeightedStatusScoreCard/WeightedStatusScoreCardComponent.tsx index 9bf0d0d09e2..06bbffd23a9 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/WeightedStatusScoreCard/WeightedStatusScoreCardComponent.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/AggregatedMetricCards/WeightedStatusScoreCard/WeightedStatusScoreCardComponent.tsx @@ -73,10 +73,9 @@ export const WeightedStatusScoreCardComponent = ({ const centerPercentLabel = `${formatPercentage(weightedStatusScorePercent)}%`; - const arcResolvedColor = resolveStatusColor( - theme, - scorecard.result.aggregationChartDisplayColor, - ); + const arcResolvedColor = scorecard.result.aggregationChartDisplayColor + ? resolveStatusColor(theme, scorecard.result.aggregationChartDisplayColor) + : theme.palette.grey[300]; const weightedStatusScorePieData: PieData[] = [ { diff --git a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx index 34c71fb6f3f..6fc46a65f56 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/__tests__/ScorecardHomepageCard.test.tsx @@ -228,6 +228,7 @@ const mockScalarAggregationScorecard: AggregatedMetricResult = { thresholds: DEFAULT_NUMBER_THRESHOLDS, entitiesConsidered: 4, calculationErrorCount: 0, + aggregationChartDisplayColor: 'warning.main', }, }; From f1f8585bcd7e997e175cf813b4d4b8d6f2055380 Mon Sep 17 00:00:00 2001 From: Ihor Mykhno Date: Mon, 7 Sep 2026 15:56:35 +0200 Subject: [PATCH 2/4] fix(scorecard): apply default theme colors for standard scalar aggregation thresholds Signed-off-by: Ihor Mykhno Assisted-By: Cursor --- .../scorecard-backend/docs/aggregation.md | 19 +++++++++-------- .../scorecard-backend/docs/thresholds.md | 2 +- .../src/constants/aggregationKPIs.ts | 21 ------------------- .../strategies/ScalarAggregationStrategy.ts | 9 ++++---- ...htedStatusScoreAggregationStrategy.test.ts | 21 +++++++++++++++---- .../getAggregationChartDisplayColor.test.ts | 21 ++++++++++++++++++- .../getAggregationChartDisplayColor.ts | 9 +++++++- 7 files changed, 60 insertions(+), 42 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md index a2b30b48a24..52f14d61d22 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md @@ -270,7 +270,7 @@ The response shape includes **`id`**, **`status`**, **`metadata`** (title, descr - **`statusGrouped`**: counts per threshold rule, **`total`**, **`thresholds`**, **`entitiesConsidered`**, **`calculationErrorCount`**, **`timestamp`**. - **`weightedStatusScore`**: same as status-grouped, plus **`weightedStatusScore`** (portfolio percentage in \[0, 100\], one decimal), **`weightedStatusSum`**, **`weightedStatusMaxPossible`**, and **`aggregationChartDisplayColor`** (see backend README). The homepage card shows a donut gauge for this type instead of a multi-slice status pie. -- **Scalar types** (`sum`, `average`, `max`, `min`, `count`): see [Scalar result fields](#scalar-result-fields) below. When **`filter.status`** is configured, **`metadata.filter`** is also returned. +- **Scalar types** (`sum`, `average`, `max`, `min`, `count`): see [Scalar result fields](#scalar-result-fields) below, including **`aggregationChartDisplayColor`**. When **`filter.status`** is configured, **`metadata.filter`** is also returned. For a daily history of a **scalar** KPI over owned entities, see [`GET /aggregations/:aggregationId/time-series`](#get-aggregationsaggregationidtime-series). @@ -278,14 +278,15 @@ For a daily history of a **scalar** KPI over owned entities, see [`GET /aggregat When **`metadata.aggregationType`** is one of **`sum`**, **`average`**, **`max`**, **`min`**, or **`count`**, **`result`** is a scalar aggregation payload: -| Field | Description | -| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| **`value`** | Aggregated number from the KPI type (`sum` / `average` / `max` / `min` / `count`) over contributing latest non-null rows. Classified by **`options.thresholds`** when present. | -| **`total`** | How many latest rows contributed to **`value`** (non-null, calculation failures excluded, optionally narrowed by **`filter.status`**). For **`count`**, equals **`value`**. | -| **`entitiesConsidered`** | Owned entities in scope that have at least one latest row for this metric (includes calculation-error rows). | -| **`calculationErrorCount`** | Among **`entitiesConsidered`**, how many latest rows are metric calculation failures (`error_message` set and `value` null). | -| **`timestamp`** | Portfolio data freshness — ISO timestamp of the most recent latest row in scope (same merge rule as other aggregation types). | -| **`thresholds`** | Number-style rules for classifying **`value`**; from **`options.thresholds`** or **`DEFAULT_NUMBER_THRESHOLDS`** when omitted. | +| Field | Description | +| ---------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **`value`** | Aggregated number from the KPI type (`sum` / `average` / `max` / `min` / `count`) over contributing latest non-null rows. Classified by **`options.thresholds`** when present. | +| **`total`** | How many latest rows contributed to **`value`** (non-null, calculation failures excluded, optionally narrowed by **`filter.status`**). For **`count`**, equals **`value`**. | +| **`entitiesConsidered`** | Owned entities in scope that have at least one latest row for this metric (includes calculation-error rows). | +| **`calculationErrorCount`** | Among **`entitiesConsidered`**, how many latest rows are metric calculation failures (`error_message` set and `value` null). | +| **`timestamp`** | Portfolio data freshness — ISO timestamp of the most recent latest row in scope (same merge rule as other aggregation types). | +| **`thresholds`** | Number-style rules for classifying **`value`**; from **`options.thresholds`** or **`DEFAULT_NUMBER_THRESHOLDS`** when omitted. | +| **`aggregationChartDisplayColor`** | Color from the **first** matching rule in **`thresholds`** against **`value`**. Standard keys (`success`, `warning`, `error`) use default theme colors when **`color`** is omitted. **`null`** when **`total`** is **0** (no contributing rows). | Example scalar response with status filter: diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index 681ab054a19..81bddd9ddc4 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -225,7 +225,7 @@ These thresholds are **not** per-entity metric rules. They apply to homepage agg - **Configuration path:** `scorecard.aggregationKPIs..options.thresholds` -- **YAML shape:** Same as metric thresholds — a **`rules`** array of **`key`**, **`expression`**, and optional **`color`** (and optional **`icon`**). Expressions are **number**-style and are evaluated against **`result.value`**, the aggregated scalar from the KPI (see [Entity Aggregation — Scalar result fields](./aggregation.md#scalar-result-fields)). The **first** matching rule wins; its **`color`** and **`key`** can be used by custom UIs that render scalar KPIs. +- **YAML shape:** Same as metric thresholds — a **`rules`** array of **`key`**, **`expression`**, and optional **`color`** (and optional **`icon`**). Expressions are **number**-style and are evaluated against **`result.value`**, the aggregated scalar from the KPI (see [Entity Aggregation — Scalar result fields](./aggregation.md#scalar-result-fields)). The **first** matching rule wins; its **`color`** is returned on the API as **`result.aggregationChartDisplayColor`** (or **`null`** when **`result.total`** is **0**). - **Defaults:** If **`thresholds`** is omitted from app-config under **`options`**, **`ScalarAggregationStrategy`** applies **`DEFAULT_NUMBER_THRESHOLDS`** from scorecard-common when serving an aggregation and includes them on the API as **`result.thresholds`**: **`<10`** → success, **`10-50`** → warning, **`>50`** → error. diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts index d5831fcfee7..1c1961a348b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/constants/aggregationKPIs.ts @@ -46,24 +46,3 @@ export const DEFAULT_WEIGHTED_STATUS_SCORE_KPI_RESULT_THRESHOLDS: ThresholdConfi }, ], }; - -export const DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS: ThresholdConfig = - { - rules: [ - { - key: 'success', - expression: '<10', - color: ScorecardThresholdRuleColors.SUCCESS, - }, - { - key: 'warning', - expression: '10-50', - color: ScorecardThresholdRuleColors.WARNING, - }, - { - key: 'error', - expression: '>50', - color: ScorecardThresholdRuleColors.ERROR, - }, - ], - }; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts index 0f24c1970a0..f5822bd7e80 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts @@ -32,7 +32,6 @@ import { isScalarAggregationConfig } from '../../../utils/aggregation/isScalarAg import { classifyNumberAgainstThresholds } from '../../../utils/aggregation/classifyNumberAgainstThresholds'; import { ThresholdEvaluator } from '../../../threshold/ThresholdEvaluator'; import { getRequiredAggregationChartDisplayColor } from '../../../utils/aggregation/getAggregationChartDisplayColor'; -import { DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS } from '../../../constants'; export class ScalarAggregationStrategy implements AggregationStrategy { constructor( @@ -52,7 +51,8 @@ export class ScalarAggregationStrategy implements AggregationStrategy { ); } - const { thresholds: headlineThresholds } = aggregationConfig.options ?? {}; + const { thresholds: headlineThresholds = DEFAULT_NUMBER_THRESHOLDS } = + aggregationConfig.options ?? {}; const { value, @@ -71,8 +71,7 @@ export class ScalarAggregationStrategy implements AggregationStrategy { total > 0 ? getRequiredAggregationChartDisplayColor( value, - headlineThresholds ?? - DEFAULT_SCALAR_AGGREGATION_KPI_RESULT_THRESHOLDS, + headlineThresholds, `The color for value '${value}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, ) : null; @@ -84,7 +83,7 @@ export class ScalarAggregationStrategy implements AggregationStrategy { calculationErrorCount, timestamp, aggregationChartDisplayColor, - thresholds: headlineThresholds ?? DEFAULT_NUMBER_THRESHOLDS, + thresholds: headlineThresholds, } satisfies ScalarAggregationResult; return AggregatedMetricMapper.toAggregatedMetricResult( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts index 49c82f62f56..627a729baac 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/weightedStatusScoreAggregationStrategy.test.ts @@ -113,26 +113,39 @@ describe('WeightedStatusScoreAggregationStrategy', () => { }); it('should throw when aggregation chart display color is not configured', async () => { + const customStatusThresholds = { + rules: [ + { key: 'ok', expression: '>=80', color: 'green' }, + { key: 'notOk', expression: '<80', color: 'red' }, + ], + }; const aggregationConfigWithoutColors = mockWeightedStatusScoreAggregationConfig({ id: 'weightedOpenPrs', metricId: metric.id, options: { - statusScores: { error: 0, warning: 50, success: 100 }, + statusScores: { notOk: 0, maybe: 50, ok: 100 }, thresholds: { rules: [ - { key: 'success', expression: '>=80' }, - { key: 'error', expression: '<80' }, + { key: 'ok', expression: '>=80' }, + { key: 'notOk', expression: '<80' }, ], }, }, }); + ( + loader.loadStatusGroupedMetricByEntityRefs as jest.Mock + ).mockResolvedValueOnce({ + ...loadedStatusGroupedMetric, + values: { ok: 2 }, + }); + await expect(() => strategy.aggregate({ metric, entityRefs, - thresholds: mockHigherIsBetterThresholds, + thresholds: customStatusThresholds, aggregationConfig: aggregationConfigWithoutColors, }), ).rejects.toThrow( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts index 827b9973aca..288a235322d 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { ScorecardThresholdRuleColors } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { getAggregationChartDisplayColor, getRequiredAggregationChartDisplayColor, @@ -48,11 +49,19 @@ describe('getAggregationChartDisplayColor', () => { ).toBeUndefined(); }); - it('should return undefined when the matching rule has no color', () => { + it('should return the standard default color when the matching rule omits color', () => { expect( getAggregationChartDisplayColor(5, { rules: [{ key: 'success', expression: '<10' }], }), + ).toBe(ScorecardThresholdRuleColors.SUCCESS); + }); + + it('should return undefined when a custom-key rule has no color', () => { + expect( + getAggregationChartDisplayColor(5, { + rules: [{ key: 'elite', expression: '<10' }], + }), ).toBeUndefined(); }); @@ -91,4 +100,14 @@ describe('getRequiredAggregationChartDisplayColor', () => { ), ).toBe('yellow'); }); + + it('should return the standard default color when the matching rule omits color', () => { + expect( + getRequiredAggregationChartDisplayColor( + 5, + { rules: [{ key: 'success', expression: '<10' }] }, + 'color is not configured', + ), + ).toBe(ScorecardThresholdRuleColors.SUCCESS); + }); }); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts index 535cb3c5adf..1dcdea74dba 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts @@ -16,6 +16,7 @@ import type { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; +import { withStandardThresholdDefaults } from './withStandardThresholdDefaults'; /** * Get the aggregation chart display color for a given value and thresholds. @@ -35,7 +36,13 @@ export function getAggregationChartDisplayColor( thresholds, ); - return thresholds.rules.find(r => r.key === matchedThresholdKey)?.color; + const matchedRule = thresholds.rules.find(r => r.key === matchedThresholdKey); + + if (!matchedRule) { + return undefined; + } + + return withStandardThresholdDefaults(matchedRule).color; } export function getRequiredAggregationChartDisplayColor( From 7e22b9bb1dc770e942173f59b331201e57b401d3 Mon Sep 17 00:00:00 2001 From: Ihor Mykhno Date: Mon, 7 Sep 2026 17:40:58 +0200 Subject: [PATCH 3/4] fix(scorecard): unify aggregation chart color resolution via shared threshold classifier Signed-off-by: Ihor Mykhno imykhno@redhat.com Assisted-By: Cursor --- .../scorecard-backend/docs/aggregation.md | 2 +- .../scorecard-backend/docs/thresholds.md | 2 +- .../strategies/ScalarAggregationStrategy.ts | 1 + .../WeightedStatusScoreAggregationStrategy.ts | 3 ++ .../getAggregationChartDisplayColor.test.ts | 51 +++---------------- .../getAggregationChartDisplayColor.ts | 39 +++++--------- 6 files changed, 26 insertions(+), 72 deletions(-) diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md index 52f14d61d22..ba23bc45291 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md @@ -269,7 +269,7 @@ Use this endpoint for all new integrations. The response shape includes **`id`**, **`status`**, **`metadata`** (title, description, type, unit, visualization, aggregation type, and **`filter`** when configured), and **`result`**. The shape of **`result`** depends on the aggregation type: - **`statusGrouped`**: counts per threshold rule, **`total`**, **`thresholds`**, **`entitiesConsidered`**, **`calculationErrorCount`**, **`timestamp`**. -- **`weightedStatusScore`**: same as status-grouped, plus **`weightedStatusScore`** (portfolio percentage in \[0, 100\], one decimal), **`weightedStatusSum`**, **`weightedStatusMaxPossible`**, and **`aggregationChartDisplayColor`** (see backend README). The homepage card shows a donut gauge for this type instead of a multi-slice status pie. +- **`weightedStatusScore`**: same as status-grouped, plus **`weightedStatusScore`** (portfolio percentage in \[0, 100\], one decimal), **`weightedStatusSum`**, **`weightedStatusMaxPossible`**, and **`aggregationChartDisplayColor`** (color from the **first** matching rule in **`thresholds`** against **`weightedStatusScore`**; **`null`** when **`total`** is **0** — see backend README). The homepage card shows a donut gauge for this type instead of a multi-slice status pie. - **Scalar types** (`sum`, `average`, `max`, `min`, `count`): see [Scalar result fields](#scalar-result-fields) below, including **`aggregationChartDisplayColor`**. When **`filter.status`** is configured, **`metadata.filter`** is also returned. For a daily history of a **scalar** KPI over owned entities, see [`GET /aggregations/:aggregationId/time-series`](#get-aggregationsaggregationidtime-series). diff --git a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md index 81bddd9ddc4..d8af4cef6ee 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md +++ b/workspaces/scorecard/plugins/scorecard-backend/docs/thresholds.md @@ -211,7 +211,7 @@ These thresholds are **not** per-entity metric rules. They apply only to homepag - **Configuration path:** `scorecard.aggregationKPIs..options.thresholds` -- **YAML shape:** Same as metric thresholds — a **`rules`** array of **`key`**, **`expression`**, and optional **`color`** (and optional **`icon`**, though icons are not used for the weightedStatusScore KPI donut). Custom keys in aggregation KPI thresholds **only require `color`** (not `icon`). Expressions are **number**-style and are evaluated against **`weightedStatusScore`**, the backend’s portfolio **percentage** in **`[0, 100]`** (one decimal; see [Entity Aggregation](./aggregation.md)). The **first** matching rule wins; its **`color`** is returned on the API as **`result.aggregationChartDisplayColor`**. +- **YAML shape:** Same as metric thresholds — a **`rules`** array of **`key`**, **`expression`**, and optional **`color`** (and optional **`icon`**, though icons are not used for the weightedStatusScore KPI donut). Custom keys in aggregation KPI thresholds **only require `color`** (not `icon`). Expressions are **number**-style and are evaluated against **`weightedStatusScore`**, the backend’s portfolio **percentage** in **`[0, 100]`** (one decimal; see [Entity Aggregation](./aggregation.md)). The **first** matching rule wins; its **`color`** is returned on the API as **`result.aggregationChartDisplayColor`** (or **`null`** when **`result.total`** is **0**). - **Defaults:** If **`thresholds`** is omitted from app-config under **`options`**, it is not injected at config-parse time. **`WeightedStatusScoreAggregationStrategy`** applies **`DEFAULT_WEIGHTED_STATUS_SCORE_KPI_RESULT_THRESHOLDS`** from [`src/constants/aggregationKPIs.ts`](../src/constants/aggregationKPIs.ts) when serving an aggregation: **`<30`** → error, **`30-79`** → warning, **`>=80`** → success (higher percentage = better). When that default path is used, the strategy logs at **info** that the built-in 0–100% scale is in effect. diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts index f5822bd7e80..d6c3c1d28df 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/ScalarAggregationStrategy.ts @@ -72,6 +72,7 @@ export class ScalarAggregationStrategy implements AggregationStrategy { ? getRequiredAggregationChartDisplayColor( value, headlineThresholds, + this.thresholdEvaluator, `The color for value '${value}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, ) : null; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts index f03c5b9c2b4..c02506627b0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/aggregations/strategies/WeightedStatusScoreAggregationStrategy.ts @@ -28,6 +28,7 @@ import type { AggregatedMetricLoader } from '../AggregatedMetricLoader'; import type { AggregationOptions } from '../types'; import type { AggregationStrategy } from './types'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { ThresholdEvaluator } from '../../../threshold/ThresholdEvaluator'; import { getRequiredAggregationChartDisplayColor } from '../../../utils/aggregation/getAggregationChartDisplayColor'; export class WeightedStatusScoreAggregationStrategy @@ -36,6 +37,7 @@ export class WeightedStatusScoreAggregationStrategy constructor( private readonly loader: AggregatedMetricLoader, private readonly logger: LoggerService, + private readonly thresholdEvaluator: ThresholdEvaluator = new ThresholdEvaluator(), ) {} async aggregate({ @@ -81,6 +83,7 @@ export class WeightedStatusScoreAggregationStrategy ? getRequiredAggregationChartDisplayColor( weightedStatusScore, headlineThresholds, + this.thresholdEvaluator, `The color for percentage '${weightedStatusScore}' metric '${metric.id}' is not configured. Check the 'scorecard.aggregationKPIs.${aggregationConfig.id}.options.thresholds' configuration.`, ) : null; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts index 288a235322d..b04a0d17a5b 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.test.ts @@ -15,10 +15,8 @@ */ import { ScorecardThresholdRuleColors } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { - getAggregationChartDisplayColor, - getRequiredAggregationChartDisplayColor, -} from './getAggregationChartDisplayColor'; +import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; +import { getRequiredAggregationChartDisplayColor } from './getAggregationChartDisplayColor'; const overlappingThresholds = { rules: [ @@ -40,52 +38,15 @@ const overlappingThresholds = { ], }; -describe('getAggregationChartDisplayColor', () => { - it('should return undefined when no rule matches', () => { - expect( - getAggregationChartDisplayColor(50, { - rules: [{ key: 'success', expression: '<10', color: 'green' }], - }), - ).toBeUndefined(); - }); - - it('should return the standard default color when the matching rule omits color', () => { - expect( - getAggregationChartDisplayColor(5, { - rules: [{ key: 'success', expression: '<10' }], - }), - ).toBe(ScorecardThresholdRuleColors.SUCCESS); - }); - - it('should return undefined when a custom-key rule has no color', () => { - expect( - getAggregationChartDisplayColor(5, { - rules: [{ key: 'elite', expression: '<10' }], - }), - ).toBeUndefined(); - }); - - it('should return the color of the first matching rule', () => { - expect(getAggregationChartDisplayColor(12, overlappingThresholds)).toBe( - 'yellow', - ); - }); - - it('should follow rule order when multiple expressions match', () => { - expect( - getAggregationChartDisplayColor(12, { - rules: [...overlappingThresholds.rules].reverse(), - }), - ).toBe('green'); - }); -}); - describe('getRequiredAggregationChartDisplayColor', () => { + const evaluator = new ThresholdEvaluator(); + it('should throw the given error when no color matches', () => { expect(() => getRequiredAggregationChartDisplayColor( 50, { rules: [{ key: 'success', expression: '<10', color: 'green' }] }, + evaluator, 'color is not configured', ), ).toThrow('color is not configured'); @@ -96,6 +57,7 @@ describe('getRequiredAggregationChartDisplayColor', () => { getRequiredAggregationChartDisplayColor( 12, overlappingThresholds, + evaluator, 'color is not configured', ), ).toBe('yellow'); @@ -106,6 +68,7 @@ describe('getRequiredAggregationChartDisplayColor', () => { getRequiredAggregationChartDisplayColor( 5, { rules: [{ key: 'success', expression: '<10' }] }, + evaluator, 'color is not configured', ), ).toBe(ScorecardThresholdRuleColors.SUCCESS); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts index 1dcdea74dba..a01de3e33f5 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregation/getAggregationChartDisplayColor.ts @@ -14,46 +14,33 @@ * limitations under the License. */ +import { InputError } from '@backstage/errors'; import type { ThresholdConfig } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; import { ThresholdEvaluator } from '../../threshold/ThresholdEvaluator'; -import { withStandardThresholdDefaults } from './withStandardThresholdDefaults'; +import { classifyNumberAgainstThresholds } from './classifyNumberAgainstThresholds'; /** - * Get the aggregation chart display color for a given value and thresholds. + * Get the required aggregation chart display color for a given value and thresholds. * @param value - The value to get the color for. * @param thresholds - The thresholds to use. - * @returns The aggregation chart display color. + * @param evaluator - Threshold evaluator instance. + * @param errorMessage - The error message to throw if the color is not found. + * @returns The required aggregation chart display color. */ -export function getAggregationChartDisplayColor( - value: number, - thresholds: ThresholdConfig, -): string | undefined { - const thresholdEvaluator = new ThresholdEvaluator(); - - const matchedThresholdKey = thresholdEvaluator.getFirstMatchingThreshold( - value, - 'number', - thresholds, - ); - - const matchedRule = thresholds.rules.find(r => r.key === matchedThresholdKey); - - if (!matchedRule) { - return undefined; - } - - return withStandardThresholdDefaults(matchedRule).color; -} - export function getRequiredAggregationChartDisplayColor( value: number, thresholds: ThresholdConfig, + evaluator: ThresholdEvaluator, errorMessage: string, ): string { - const color = getAggregationChartDisplayColor(value, thresholds); + const color = classifyNumberAgainstThresholds( + value, + thresholds, + evaluator, + )?.color; if (!color) { - throw new Error(errorMessage); + throw new InputError(errorMessage); } return color; From 511f1797a461e4aad8080aa33f7a172099f30c74 Mon Sep 17 00:00:00 2001 From: Ihor Mykhno Date: Tue, 8 Sep 2026 13:12:39 +0200 Subject: [PATCH 4/4] fix(scorecard): add breaking changes to changeset Signed-off-by: Ihor Mykhno --- workspaces/scorecard/.changeset/hungry-walls-burn.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/workspaces/scorecard/.changeset/hungry-walls-burn.md b/workspaces/scorecard/.changeset/hungry-walls-burn.md index e89d52ac602..87107a37c96 100644 --- a/workspaces/scorecard/.changeset/hungry-walls-burn.md +++ b/workspaces/scorecard/.changeset/hungry-walls-burn.md @@ -4,4 +4,11 @@ '@red-hat-developer-hub/backstage-plugin-scorecard-common': minor --- -Skip scalar aggregation threshold coloring when no successful samples contributed (`total` is 0); return a null display color and keep the card grey fallback. +Skip scalar aggregation threshold coloring when no successful samples contributed (`total` is 0). Return a null display color and keep the card grey fallback. Scalar aggregation responses now include `aggregationChartDisplayColor` (threshold-derived chart color, or `null` when `total` is 0). + +**BREAKING**: Changed types in `scorecard-common` module: + +- `WeightedStatusScoreAggregationResult.aggregationChartDisplayColor` widened from `string` to `string | null`. +- `ScalarAggregationResult` gained a required `aggregationChartDisplayColor: string | null` property. + +These changes are intentional: the API can return `null` when no samples contribute, and scalar KPI results now expose the same display-color field as weighted status score aggregations.