From c689d5d9a33aa77947fb4775e45da8a456c5d105 Mon Sep 17 00:00:00 2001 From: Ihor Mykhno Date: Tue, 23 Dec 2025 21:36:42 +0100 Subject: [PATCH 01/18] feat(scorecard): implement endpoint to aggregate metrics for scorecard --- .../scorecard/.changeset/cold-experts-stop.md | 6 + .../src/index.ts | 11 + .../src/service/CatalogMetricService.test.ts | 23 ++ .../src/service/CatalogMetricService.ts | 10 + .../utils/aggregateMetricsByStatus.test.ts | 379 ++++++++++++++++++ .../src/utils/aggregateMetricsByStatus.ts | 44 ++ 6 files changed, 473 insertions(+) create mode 100644 workspaces/scorecard/.changeset/cold-experts-stop.md create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts create mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts diff --git a/workspaces/scorecard/.changeset/cold-experts-stop.md b/workspaces/scorecard/.changeset/cold-experts-stop.md new file mode 100644 index 00000000000..b70089144df --- /dev/null +++ b/workspaces/scorecard/.changeset/cold-experts-stop.md @@ -0,0 +1,6 @@ +--- +'@red-hat-developer-hub/backstage-plugin-scorecard-backend': minor +'@red-hat-developer-hub/backstage-plugin-scorecard-common': minor +--- + +Implemented endpoint to aggregate metrics for scorecard diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts index 934f1f3d6a3..baaec70e85e 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts @@ -15,9 +15,20 @@ */ /** +<<<<<<<< HEAD:workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts * The openssf backend module for the scorecard plugin. * * @packageDocumentation */ export { scorecardOpenSFFModule as default } from './module'; +======== + * Parse a comma separated string into an array of strings + * + * @param value - The comma separated string to parse + * @returns The array of strings + */ +export function parseCommaSeparatedString(value: string): string[] { + return value.split(',').map(id => id.trim()); +} +>>>>>>>> 84afdf20 (feat(scorecard): implement endpoint to aggregate metrics for scorecard):workspaces/scorecard/plugins/scorecard-backend/src/utils/parseCommaSeparatedString.ts diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts index e0e77f23e34..f9b1bfb2ca0 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -40,6 +40,7 @@ import { jest.mock('../utils/mergeEntityAndProviderThresholds'); jest.mock('../permissions/permissionUtils'); +jest.mock('../utils/aggregateMetricsByStatus'); const provider = new MockNumberProvider('github.important_metric', 'github'); @@ -87,6 +88,9 @@ describe('CatalogMetricService', () => { let mockedRegistry: jest.Mocked; let mockedDatabase: jest.Mocked; let service: CatalogMetricService; + let aggregateMetricsByStatusSpy: jest.SpyInstance; + + const mockEntity = new MockEntityBuilder().build(); const mockEntity = new MockEntityBuilder().build(); @@ -120,6 +124,19 @@ describe('CatalogMetricService', () => { rules: mockThresholdRules, }); + aggregateMetricsByStatusSpy = jest + .spyOn(aggregateMetricsByStatusModule, 'aggregateMetricsByStatus') + .mockReturnValue({ + 'github.important_metric': { + values: { + success: 1, + warning: 1, + error: 0, + }, + total: 2, + }, + }); + service = new CatalogMetricService({ catalog: mockedCatalog, auth: mockedAuth, @@ -143,6 +160,12 @@ describe('CatalogMetricService', () => { }); }); + describe('getCatalogService', () => { + it('should return the catalog service', () => { + expect(service.getCatalogService()).toBe(mockedCatalog); + }); + }); + describe('getLatestEntityMetrics', () => { it('should handle multiple metrics correctly', async () => { const secondProvider = new MockNumberProvider( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index 16ebc51163a..2c03308aa76 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -31,6 +31,7 @@ import { import { CatalogService } from '@backstage/plugin-catalog-node'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; +import { aggregateMetricsByStatus } from '../utils/aggregateMetricsByStatus'; type CatalogMetricServiceOptions = { catalog: CatalogService; @@ -57,6 +58,15 @@ export class CatalogMetricService { this.database = options.database; } + /** + * Get the catalog service + * + * @returns CatalogService + */ + getCatalogService(): CatalogService { + return this.catalog; + } + /** * Get latest metric results for a specific catalog entity and metric providers. * diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts new file mode 100644 index 00000000000..aa75f73edeb --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts @@ -0,0 +1,379 @@ +/* + * 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 { aggregateMetricsByStatus } from './aggregateMetricsByStatus'; +import { DbMetricValue } from '../database/types'; + +describe('aggregateMetricsByStatus', () => { + const createMetric = ( + metricId: string, + status: 'success' | 'warning' | 'error', + value: any = { count: 1 }, + ): DbMetricValue => ({ + id: 1, + catalog_entity_ref: 'component:default/test', + metric_id: metricId, + value, + timestamp: new Date(), + status, + }); + + it('should return empty object when metrics array is empty', () => { + const result = aggregateMetricsByStatus([]); + expect(result).toEqual({}); + }); + + describe('when metrics have valid status and value', () => { + it('should aggregate single success metric', () => { + const metrics = [createMetric('metric1', 'success')]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should aggregate single warning metric', () => { + const metrics = [createMetric('metric1', 'warning')]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 0, + warning: 1, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should aggregate single error metric', () => { + const metrics = [createMetric('metric1', 'error')]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 0, + warning: 0, + error: 1, + }, + total: 1, + }, + }); + }); + + it('should aggregate multiple metrics with same metric_id', () => { + const metrics = [ + createMetric('metric1', 'success'), + createMetric('metric1', 'success'), + createMetric('metric1', 'warning'), + createMetric('metric1', 'error'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 2, + warning: 1, + error: 1, + }, + total: 4, + }, + }); + }); + + it('should aggregate multiple metrics with different metric_ids', () => { + const metrics = [ + createMetric('metric1', 'success'), + createMetric('metric2', 'warning'), + createMetric('metric3', 'error'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + metric2: { + values: { + success: 0, + warning: 1, + error: 0, + }, + total: 1, + }, + metric3: { + values: { + success: 0, + warning: 0, + error: 1, + }, + total: 1, + }, + }); + }); + + it('should aggregate complex scenario with multiple metric_ids and statuses', () => { + const metrics = [ + createMetric('metric1', 'success'), + createMetric('metric1', 'success'), + createMetric('metric1', 'warning'), + createMetric('metric2', 'error'), + createMetric('metric2', 'error'), + createMetric('metric2', 'success'), + createMetric('metric3', 'warning'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 2, + warning: 1, + error: 0, + }, + total: 3, + }, + metric2: { + values: { + success: 1, + warning: 0, + error: 2, + }, + total: 3, + }, + metric3: { + values: { + success: 0, + warning: 1, + error: 0, + }, + total: 1, + }, + }); + }); + }); + + describe('when metrics have invalid status or value', () => { + it('should skip metrics with null value', () => { + const metrics: DbMetricValue[] = [ + createMetric('metric1', 'success', null), + createMetric('metric1', 'warning', { count: 1 }), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 0, + warning: 1, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should skip metrics without status', () => { + const metrics: DbMetricValue[] = [ + // @ts-expect-error - for testing + createMetric('metric1', undefined, { count: 1 }), + createMetric('metric1', 'success'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should skip metrics with both null value and no status', () => { + const metrics: DbMetricValue[] = [ + { + id: 1, + catalog_entity_ref: 'component:default/test', + metric_id: 'metric1', + value: undefined, + timestamp: new Date(), + }, + createMetric('metric1', 'success'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should handle undefined value as valid (not null)', () => { + const metrics: DbMetricValue[] = [ + createMetric('metric1', 'success', undefined), + ]; + const result = aggregateMetricsByStatus(metrics); + + // undefined !== null, so it should be included + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should handle mixed valid and invalid metrics', () => { + const metrics: DbMetricValue[] = [ + createMetric('metric1', 'success'), + createMetric('metric1', 'warning', null), + // @ts-expect-error - for testing + createMetric('metric1', undefined, { count: 1 }), + createMetric('metric1', 'error'), + createMetric('metric2', 'success', null), + createMetric('metric2', 'warning'), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 1, + }, + total: 2, + }, + metric2: { + values: { + success: 0, + warning: 1, + error: 0, + }, + total: 1, + }, + }); + }); + }); + + describe('when all metrics are invalid', () => { + it('should return empty object when all metrics have null value', () => { + const metrics: DbMetricValue[] = [ + createMetric('metric1', 'success', null), + createMetric('metric2', 'warning', null), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({}); + }); + + it('should return empty object when all metrics have no status', () => { + const metrics: DbMetricValue[] = [ + // @ts-expect-error - for testing + createMetric('metric2', undefined, { count: 2 }), + // @ts-expect-error - for testing + createMetric('metric2', undefined, { count: 2 }), + ]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({}); + }); + }); + + describe('edge cases', () => { + it('should handle zero value as valid (not null)', () => { + const metrics: DbMetricValue[] = [createMetric('metric1', 'success', 0)]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should handle empty string value as valid (not null)', () => { + const metrics: DbMetricValue[] = [createMetric('metric1', 'success', '')]; + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + + it('should handle false value as valid (not null)', () => { + const metrics: DbMetricValue[] = [ + createMetric('metric1', 'success', false), + ]; + + const result = aggregateMetricsByStatus(metrics); + + expect(result).toEqual({ + metric1: { + values: { + success: 1, + warning: 0, + error: 0, + }, + total: 1, + }, + }); + }); + }); +}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts new file mode 100644 index 00000000000..ca1424b0f92 --- /dev/null +++ b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts @@ -0,0 +1,44 @@ +/* + * 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 { DbMetricValue } from '../database/types'; +import { AggregatedMetricsByStatus } from '../service/CatalogMetricService'; + +export function aggregateMetricsByStatus( + metrics: DbMetricValue[], +): AggregatedMetricsByStatus { + const aggregatedMetrics: AggregatedMetricsByStatus = {}; + + for (const metric of metrics) { + if (metric.status && metric.value !== null) { + if (!Object.hasOwn(aggregatedMetrics, metric.metric_id)) { + aggregatedMetrics[metric.metric_id] = { + values: { + success: 0, + warning: 0, + error: 0, + }, + total: 0, + }; + } + + aggregatedMetrics[metric.metric_id].values[metric.status]++; + aggregatedMetrics[metric.metric_id].total++; + } + } + + return aggregatedMetrics; +} From 459b99b5e4f79270fc5ac587324d1c9ff2013d1b Mon Sep 17 00:00:00 2001 From: Eswaraiahsapram Date: Tue, 6 Jan 2026 15:53:10 +0530 Subject: [PATCH 02/18] fix(scorecard): integration UI with aggregated backend api --- .../scorecard/packages/app/package.json | 2 +- workspaces/scorecard/packages/app/src/App.tsx | 28 +- .../src/index.ts | 11 - .../src/service/CatalogMetricService.test.ts | 23 - .../src/service/CatalogMetricService.ts | 1 - .../utils/aggregateMetricsByStatus.test.ts | 379 ------------ .../src/utils/aggregateMetricsByStatus.ts | 44 -- .../__fixtures__/aggregatedScorecardData.ts | 28 +- .../scorecard/plugins/scorecard/dev/index.tsx | 7 +- .../plugins/scorecard/src/api/index.ts | 43 +- .../src/components/Common/CardWrapper.tsx | 79 ++- .../Common/PermissionRequiredState.tsx | 2 +- .../Common/__tests__/CardWrapper.test.tsx | 6 +- .../PermissionRequiredState.test.tsx | 2 +- .../src/components/Scorecard/CustomLegend.tsx | 104 ++++ .../Scorecard/EntityScorecardContent.tsx | 1 - .../src/components/Scorecard/Scorecard.tsx | 426 ++++++------- .../Scorecard/__tests__/Scorecard.test.tsx | 31 +- .../ScorecardHomepageSection/CustomLegend.tsx | 17 +- .../CustomTooltip.tsx | 29 +- .../PermissionRequiredHomepageCard.tsx | 136 +++++ .../ResponsivePieChart.tsx | 84 +++ .../ScorecardHomepageCard.tsx | 109 +--- .../ScorecardHomepageSection.tsx | 72 ++- .../__tests__/CustomLegend.test.tsx | 105 +++- .../__tests__/CustomTooltip.test.tsx | 37 +- .../__tests__/ScorecardHomepageCard.test.tsx | 255 ++++---- .../ScorecardHomepageSection.test.tsx | 175 +++--- .../ScorecardHomepageSection/index.ts | 5 +- ...recards.tsx => useAggregatedScorecard.tsx} | 30 +- .../scorecard/plugins/scorecard/src/plugin.ts | 22 +- .../plugins/scorecard/src/translations/de.ts | 6 + .../plugins/scorecard/src/translations/es.ts | 6 + .../plugins/scorecard/src/translations/fr.ts | 8 + .../plugins/scorecard/src/translations/ref.ts | 6 + .../plugins/scorecard/src/utils/utils.ts | 13 + workspaces/scorecard/yarn.lock | 559 +++++++++++++++--- 37 files changed, 1687 insertions(+), 1204 deletions(-) delete mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts delete mode 100644 workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts create mode 100644 workspaces/scorecard/plugins/scorecard/src/components/Scorecard/CustomLegend.tsx create mode 100644 workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/PermissionRequiredHomepageCard.tsx create mode 100644 workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ResponsivePieChart.tsx rename workspaces/scorecard/plugins/scorecard/src/hooks/{useAggregatedScorecards.tsx => useAggregatedScorecard.tsx} (64%) diff --git a/workspaces/scorecard/packages/app/package.json b/workspaces/scorecard/packages/app/package.json index 680b4b33d94..c7e25f532c6 100644 --- a/workspaces/scorecard/packages/app/package.json +++ b/workspaces/scorecard/packages/app/package.json @@ -49,7 +49,7 @@ "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@openshift/dynamic-plugin-sdk": "^5.0.1", - "@red-hat-developer-hub/backstage-plugin-dynamic-home-page": "^1.9.2", + "@red-hat-developer-hub/backstage-plugin-dynamic-home-page": "^1.10.2", "@red-hat-developer-hub/backstage-plugin-scorecard": "workspace:^", "@red-hat-developer-hub/backstage-plugin-theme": "^0.12.0", "@roadiehq/backstage-plugin-github-pull-requests": "^3.5.1", diff --git a/workspaces/scorecard/packages/app/src/App.tsx b/workspaces/scorecard/packages/app/src/App.tsx index 5c2c39c481e..0a88db9293a 100644 --- a/workspaces/scorecard/packages/app/src/App.tsx +++ b/workspaces/scorecard/packages/app/src/App.tsx @@ -54,14 +54,16 @@ import { catalogEntityCreatePermission } from '@backstage/plugin-catalog-common/ import { scorecardTranslations } from '@red-hat-developer-hub/backstage-plugin-scorecard/alpha'; import { githubAuthApiRef } from '@backstage/core-plugin-api'; import { getThemes } from '@red-hat-developer-hub/backstage-plugin-theme'; -import { ScorecardHomepageSection } from '@red-hat-developer-hub/backstage-plugin-scorecard'; +import { + ScorecardJiraHomepageCard, + ScorecardGitHubHomepageCard, +} from '@red-hat-developer-hub/backstage-plugin-scorecard'; import { ScalprumContext, ScalprumState } from '@scalprum/react-core'; import { PluginStore } from '@openshift/dynamic-plugin-sdk'; import { DynamicCustomizableHomePage, OnboardingSection, - defaultLayouts, HomePageCardMountPoint, homepageTranslations, } from '@red-hat-developer-hub/backstage-plugin-dynamic-home-page'; @@ -70,20 +72,22 @@ const mountPoints: HomePageCardMountPoint[] = [ { Component: OnboardingSection, config: { - layouts: defaultLayouts.onboarding, + id: 'onboarding-section', + title: 'Onboarding section', }, }, { - Component: ScorecardHomepageSection, + Component: ScorecardJiraHomepageCard, config: { - layouts: { - xl: { w: 12, h: 6 }, - lg: { w: 12, h: 6 }, - md: { w: 12, h: 7 }, - sm: { w: 12, h: 8 }, - xs: { w: 12, h: 9 }, - xxs: { w: 12, h: 10 }, - }, + id: 'scorecard-jira-homepage-section', + title: 'Scorecard Jira homepage section', + }, + }, + { + Component: ScorecardGitHubHomepageCard, + config: { + id: 'scorecard-github-homepage-section', + title: 'Scorecard GitHub homepage section', }, }, ]; diff --git a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts index baaec70e85e..934f1f3d6a3 100644 --- a/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts +++ b/workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts @@ -15,20 +15,9 @@ */ /** -<<<<<<<< HEAD:workspaces/scorecard/plugins/scorecard-backend-module-openssf/src/index.ts * The openssf backend module for the scorecard plugin. * * @packageDocumentation */ export { scorecardOpenSFFModule as default } from './module'; -======== - * Parse a comma separated string into an array of strings - * - * @param value - The comma separated string to parse - * @returns The array of strings - */ -export function parseCommaSeparatedString(value: string): string[] { - return value.split(',').map(id => id.trim()); -} ->>>>>>>> 84afdf20 (feat(scorecard): implement endpoint to aggregate metrics for scorecard):workspaces/scorecard/plugins/scorecard-backend/src/utils/parseCommaSeparatedString.ts diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts index f9b1bfb2ca0..e0e77f23e34 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.test.ts @@ -40,7 +40,6 @@ import { jest.mock('../utils/mergeEntityAndProviderThresholds'); jest.mock('../permissions/permissionUtils'); -jest.mock('../utils/aggregateMetricsByStatus'); const provider = new MockNumberProvider('github.important_metric', 'github'); @@ -88,9 +87,6 @@ describe('CatalogMetricService', () => { let mockedRegistry: jest.Mocked; let mockedDatabase: jest.Mocked; let service: CatalogMetricService; - let aggregateMetricsByStatusSpy: jest.SpyInstance; - - const mockEntity = new MockEntityBuilder().build(); const mockEntity = new MockEntityBuilder().build(); @@ -124,19 +120,6 @@ describe('CatalogMetricService', () => { rules: mockThresholdRules, }); - aggregateMetricsByStatusSpy = jest - .spyOn(aggregateMetricsByStatusModule, 'aggregateMetricsByStatus') - .mockReturnValue({ - 'github.important_metric': { - values: { - success: 1, - warning: 1, - error: 0, - }, - total: 2, - }, - }); - service = new CatalogMetricService({ catalog: mockedCatalog, auth: mockedAuth, @@ -160,12 +143,6 @@ describe('CatalogMetricService', () => { }); }); - describe('getCatalogService', () => { - it('should return the catalog service', () => { - expect(service.getCatalogService()).toBe(mockedCatalog); - }); - }); - describe('getLatestEntityMetrics', () => { it('should handle multiple metrics correctly', async () => { const secondProvider = new MockNumberProvider( diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts index 2c03308aa76..ed1cd6ef279 100644 --- a/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts +++ b/workspaces/scorecard/plugins/scorecard-backend/src/service/CatalogMetricService.ts @@ -31,7 +31,6 @@ import { import { CatalogService } from '@backstage/plugin-catalog-node'; import { DatabaseMetricValues } from '../database/DatabaseMetricValues'; import { mergeEntityAndProviderThresholds } from '../utils/mergeEntityAndProviderThresholds'; -import { aggregateMetricsByStatus } from '../utils/aggregateMetricsByStatus'; type CatalogMetricServiceOptions = { catalog: CatalogService; diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts deleted file mode 100644 index aa75f73edeb..00000000000 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.test.ts +++ /dev/null @@ -1,379 +0,0 @@ -/* - * 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 { aggregateMetricsByStatus } from './aggregateMetricsByStatus'; -import { DbMetricValue } from '../database/types'; - -describe('aggregateMetricsByStatus', () => { - const createMetric = ( - metricId: string, - status: 'success' | 'warning' | 'error', - value: any = { count: 1 }, - ): DbMetricValue => ({ - id: 1, - catalog_entity_ref: 'component:default/test', - metric_id: metricId, - value, - timestamp: new Date(), - status, - }); - - it('should return empty object when metrics array is empty', () => { - const result = aggregateMetricsByStatus([]); - expect(result).toEqual({}); - }); - - describe('when metrics have valid status and value', () => { - it('should aggregate single success metric', () => { - const metrics = [createMetric('metric1', 'success')]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should aggregate single warning metric', () => { - const metrics = [createMetric('metric1', 'warning')]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 0, - warning: 1, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should aggregate single error metric', () => { - const metrics = [createMetric('metric1', 'error')]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 0, - warning: 0, - error: 1, - }, - total: 1, - }, - }); - }); - - it('should aggregate multiple metrics with same metric_id', () => { - const metrics = [ - createMetric('metric1', 'success'), - createMetric('metric1', 'success'), - createMetric('metric1', 'warning'), - createMetric('metric1', 'error'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 2, - warning: 1, - error: 1, - }, - total: 4, - }, - }); - }); - - it('should aggregate multiple metrics with different metric_ids', () => { - const metrics = [ - createMetric('metric1', 'success'), - createMetric('metric2', 'warning'), - createMetric('metric3', 'error'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - metric2: { - values: { - success: 0, - warning: 1, - error: 0, - }, - total: 1, - }, - metric3: { - values: { - success: 0, - warning: 0, - error: 1, - }, - total: 1, - }, - }); - }); - - it('should aggregate complex scenario with multiple metric_ids and statuses', () => { - const metrics = [ - createMetric('metric1', 'success'), - createMetric('metric1', 'success'), - createMetric('metric1', 'warning'), - createMetric('metric2', 'error'), - createMetric('metric2', 'error'), - createMetric('metric2', 'success'), - createMetric('metric3', 'warning'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 2, - warning: 1, - error: 0, - }, - total: 3, - }, - metric2: { - values: { - success: 1, - warning: 0, - error: 2, - }, - total: 3, - }, - metric3: { - values: { - success: 0, - warning: 1, - error: 0, - }, - total: 1, - }, - }); - }); - }); - - describe('when metrics have invalid status or value', () => { - it('should skip metrics with null value', () => { - const metrics: DbMetricValue[] = [ - createMetric('metric1', 'success', null), - createMetric('metric1', 'warning', { count: 1 }), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 0, - warning: 1, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should skip metrics without status', () => { - const metrics: DbMetricValue[] = [ - // @ts-expect-error - for testing - createMetric('metric1', undefined, { count: 1 }), - createMetric('metric1', 'success'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should skip metrics with both null value and no status', () => { - const metrics: DbMetricValue[] = [ - { - id: 1, - catalog_entity_ref: 'component:default/test', - metric_id: 'metric1', - value: undefined, - timestamp: new Date(), - }, - createMetric('metric1', 'success'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should handle undefined value as valid (not null)', () => { - const metrics: DbMetricValue[] = [ - createMetric('metric1', 'success', undefined), - ]; - const result = aggregateMetricsByStatus(metrics); - - // undefined !== null, so it should be included - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should handle mixed valid and invalid metrics', () => { - const metrics: DbMetricValue[] = [ - createMetric('metric1', 'success'), - createMetric('metric1', 'warning', null), - // @ts-expect-error - for testing - createMetric('metric1', undefined, { count: 1 }), - createMetric('metric1', 'error'), - createMetric('metric2', 'success', null), - createMetric('metric2', 'warning'), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 1, - }, - total: 2, - }, - metric2: { - values: { - success: 0, - warning: 1, - error: 0, - }, - total: 1, - }, - }); - }); - }); - - describe('when all metrics are invalid', () => { - it('should return empty object when all metrics have null value', () => { - const metrics: DbMetricValue[] = [ - createMetric('metric1', 'success', null), - createMetric('metric2', 'warning', null), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({}); - }); - - it('should return empty object when all metrics have no status', () => { - const metrics: DbMetricValue[] = [ - // @ts-expect-error - for testing - createMetric('metric2', undefined, { count: 2 }), - // @ts-expect-error - for testing - createMetric('metric2', undefined, { count: 2 }), - ]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({}); - }); - }); - - describe('edge cases', () => { - it('should handle zero value as valid (not null)', () => { - const metrics: DbMetricValue[] = [createMetric('metric1', 'success', 0)]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should handle empty string value as valid (not null)', () => { - const metrics: DbMetricValue[] = [createMetric('metric1', 'success', '')]; - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - - it('should handle false value as valid (not null)', () => { - const metrics: DbMetricValue[] = [ - createMetric('metric1', 'success', false), - ]; - - const result = aggregateMetricsByStatus(metrics); - - expect(result).toEqual({ - metric1: { - values: { - success: 1, - warning: 0, - error: 0, - }, - total: 1, - }, - }); - }); - }); -}); diff --git a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts b/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts deleted file mode 100644 index ca1424b0f92..00000000000 --- a/workspaces/scorecard/plugins/scorecard-backend/src/utils/aggregateMetricsByStatus.ts +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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 { DbMetricValue } from '../database/types'; -import { AggregatedMetricsByStatus } from '../service/CatalogMetricService'; - -export function aggregateMetricsByStatus( - metrics: DbMetricValue[], -): AggregatedMetricsByStatus { - const aggregatedMetrics: AggregatedMetricsByStatus = {}; - - for (const metric of metrics) { - if (metric.status && metric.value !== null) { - if (!Object.hasOwn(aggregatedMetrics, metric.metric_id)) { - aggregatedMetrics[metric.metric_id] = { - values: { - success: 0, - warning: 0, - error: 0, - }, - total: 0, - }; - } - - aggregatedMetrics[metric.metric_id].values[metric.status]++; - aggregatedMetrics[metric.metric_id].total++; - } - } - - return aggregatedMetrics; -} diff --git a/workspaces/scorecard/plugins/scorecard/__fixtures__/aggregatedScorecardData.ts b/workspaces/scorecard/plugins/scorecard/__fixtures__/aggregatedScorecardData.ts index d881bee2bee..e072cf25603 100644 --- a/workspaces/scorecard/plugins/scorecard/__fixtures__/aggregatedScorecardData.ts +++ b/workspaces/scorecard/plugins/scorecard/__fixtures__/aggregatedScorecardData.ts @@ -41,7 +41,7 @@ export const mockAggregatedScorecardSuccessData: AggregatedMetricResult[] = [ id: 'jira.issues_open', status: 'success', metadata: { - title: 'Jira open blocking tickets', + title: 'Open Jira Issues', description: 'Highlights the number of critical, blocking issues that are currently open in Jira.', type: 'number', @@ -49,11 +49,31 @@ export const mockAggregatedScorecardSuccessData: AggregatedMetricResult[] = [ }, result: { values: [ - { count: 0, name: 'success' }, + { count: 4, name: 'success' }, { count: 1, name: 'warning' }, - { count: 3, name: 'error' }, + { count: 6, name: 'error' }, ], - total: 4, + total: 11, + timestamp: '2024-01-15T10:30:00Z', + }, + }, + { + id: 'github.open_prs', + status: 'success', + metadata: { + title: 'GitHub Open PRs', + description: + 'Current count of open Pull Requests for a given GitHub repository.', + type: 'number', + history: true, + }, + result: { + values: [ + { count: 1, name: 'success' }, + { count: 3, name: 'warning' }, + { count: 10, name: 'error' }, + ], + total: 14, timestamp: '2024-01-15T10:30:00Z', }, }, diff --git a/workspaces/scorecard/plugins/scorecard/dev/index.tsx b/workspaces/scorecard/plugins/scorecard/dev/index.tsx index 8e7bdc82d5c..61c4182406b 100644 --- a/workspaces/scorecard/plugins/scorecard/dev/index.tsx +++ b/workspaces/scorecard/plugins/scorecard/dev/index.tsx @@ -13,7 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + +// eslint-disable-next-line import '@backstage/ui/css/styles.css'; + import { createDevApp } from '@backstage/dev-utils'; import { EntityProvider } from '@backstage/plugin-catalog-react'; import { Page, Header, TabbedLayout } from '@backstage/core-components'; @@ -52,8 +55,8 @@ class MockScorecardApi implements ScorecardApi { async getScorecards(_entity: Entity): Promise { return [...mockScorecardSuccessData, ...mockScorecardErrorData]; } - async getAggregatedScorecards( - _metricIds?: string[], + async getAggregatedScorecard( + _metricId: string, ): Promise { return mockAggregatedScorecardSuccessData; } diff --git a/workspaces/scorecard/plugins/scorecard/src/api/index.ts b/workspaces/scorecard/plugins/scorecard/src/api/index.ts index abe1eb4d69f..e74939872df 100644 --- a/workspaces/scorecard/plugins/scorecard/src/api/index.ts +++ b/workspaces/scorecard/plugins/scorecard/src/api/index.ts @@ -24,7 +24,6 @@ import type { MetricResult, AggregatedMetricResult, } from '@red-hat-developer-hub/backstage-plugin-scorecard-common'; -import { mockAggregatedScorecardSuccessData } from '../../__fixtures__/aggregatedScorecardData'; export interface ScorecardApi { /** @@ -34,7 +33,7 @@ export interface ScorecardApi { * @returns Promise resolving to an array of metric results */ getScorecards(entity: Entity, metricIds?: string[]): Promise; - getAggregatedScorecards(): Promise; + getAggregatedScorecard(metricId: string): Promise; } export const scorecardApiRef = createApiRef({ @@ -122,8 +121,42 @@ export class ScorecardApiClient implements ScorecardApi { } } - async getAggregatedScorecards(): Promise { - // Return mock data instead of making an API call - return mockAggregatedScorecardSuccessData; + async getAggregatedScorecard( + metricId: string, + ): Promise { + if (!metricId) { + throw new Error('Metric ID is required for aggregated scorecards'); + } + + const baseUrl = await this.getBaseUrl(); + const url = new URL(`${baseUrl}/metrics/${metricId}/catalog/aggregation`); + + try { + const response = await this.fetchApi.fetch(url.toString()); + + if (!response.ok) { + const errorText = await response.text(); + throw new Error( + `Failed to fetch aggregated scorecards: ${response.status} ${response.statusText}. ${errorText}`, + ); + } + + const data = await response.json(); + + if (!Array.isArray(data)) { + throw new Error( + 'Invalid response format from aggregated scorecard API', + ); + } + + return data; + } catch (error) { + if (error instanceof Error) { + throw error; + } + throw new Error( + `Unexpected error fetching aggregated scorecards: ${String(error)}`, + ); + } } } diff --git a/workspaces/scorecard/plugins/scorecard/src/components/Common/CardWrapper.tsx b/workspaces/scorecard/plugins/scorecard/src/components/Common/CardWrapper.tsx index 550f6155f02..d4b09082a5c 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/Common/CardWrapper.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/Common/CardWrapper.tsx @@ -20,34 +20,101 @@ import Card from '@mui/material/Card'; import CardHeader from '@mui/material/CardHeader'; import CardContent from '@mui/material/CardContent'; import Divider from '@mui/material/Divider'; +import Box from '@mui/material/Box'; +import Typography from '@mui/material/Typography'; interface CardWrapperProps extends HTMLProps { children: ReactNode; title: string; - subtitle?: ReactNode; + subheader?: ReactNode; + description?: string; width?: string; + childrenWidth?: string | number; + childrenHeight?: string | number; } export const CardWrapper = ({ children, title, - subtitle, - width = '371px', + subheader, + description, + width, + childrenWidth = '100%', + childrenHeight = '100%', }: CardWrapperProps) => { return ( - + `1px solid ${muiTheme.palette.grey[300]}`, + overflow: 'auto', + }} + > - {children} + + {description && ( + + + {description} + + + )} + + + + {children} + + + ); }; diff --git a/workspaces/scorecard/plugins/scorecard/src/components/Common/PermissionRequiredState.tsx b/workspaces/scorecard/plugins/scorecard/src/components/Common/PermissionRequiredState.tsx index 0edc6c138dc..9bb70f0983a 100644 --- a/workspaces/scorecard/plugins/scorecard/src/components/Common/PermissionRequiredState.tsx +++ b/workspaces/scorecard/plugins/scorecard/src/components/Common/PermissionRequiredState.tsx @@ -73,7 +73,7 @@ const PermissionRequiredState = () => {