Skip to content

Commit 4a3369f

Browse files
authored
Scorecard: Fix homepage aggregated scorecard widgets (#2201)
* refactor(scorecard): update aggregated metric handling and API response structure Signed-off-by: Ihor Mykhno <imykhno@redhat.com> * fix(scorecard): aggregated widgets view when entities are missing value or metric fetching fails Signed-off-by: Ihor Mykhno <imykhno@redhat.com> * fix(scorecard): e2e tests Signed-off-by: Ihor Mykhno <imykhno@redhat.com> * chore(scorecard): update label and tooltip message for the aggregation failed cards Signed-off-by: Ihor Mykhno <imykhno@redhat.com> * fix(scorecard): homepage error card labels and tooltips Signed-off-by: Ihor Mykhno <imykhno@redhat.com> * chore(scorecard): update report file --------- Signed-off-by: Ihor Mykhno <imykhno@redhat.com>
1 parent c964f4f commit 4a3369f

35 files changed

Lines changed: 894 additions & 579 deletions
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@red-hat-developer-hub/backstage-plugin-scorecard-backend': patch
3+
'@red-hat-developer-hub/backstage-plugin-scorecard-common': patch
4+
'@red-hat-developer-hub/backstage-plugin-scorecard': patch
5+
---
6+
7+
Fix aggregated scorecard widgets view when entities are missing value or metric fetching fails.
8+
9+
Refactor the /metrics/:metricId/catalog/aggregations endpoint to return an object of aggregated metrics instead of an array containing a single object.

workspaces/scorecard/packages/app/e2e-tests/scorecard.test.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import {
2929
invalidThresholdResponse,
3030
githubAggregatedResponse,
3131
jiraAggregatedResponse,
32+
emptyGithubAggregatedResponse,
33+
emptyJiraAggregatedResponse,
3234
} from './utils/scorecardResponseUtils';
3335
import {
3436
ScorecardMessages,
@@ -265,13 +267,24 @@ test.describe('Scorecard Plugin Tests', () => {
265267
await runAccessibilityTests(page, testInfo);
266268
});
267269

268-
test('Verify cards hidden when API returns empty response', async () => {
269-
await mockAggregatedScorecardResponse(page, [], []);
270+
test('Verify cards aggregation data is not found when API returns empty aggregated response', async () => {
271+
await mockAggregatedScorecardResponse(
272+
page,
273+
emptyGithubAggregatedResponse,
274+
emptyJiraAggregatedResponse,
275+
);
270276

271277
await homePage.navigateToHome();
272278
await page.reload();
273-
await homePage.expectCardNotVisible('github.open_prs');
274-
await homePage.expectCardNotVisible('jira.open_issues');
279+
await homePage.expectCardVisible('github.open_prs');
280+
await homePage.expectCardVisible('jira.open_issues');
281+
282+
await expect(page.locator('article')).toContainText(
283+
translations.errors.noDataFound,
284+
);
285+
await expect(page.locator('article')).toContainText(
286+
translations.errors.noDataFoundMessage,
287+
);
275288
});
276289

277290
test('Verify threshold tooltips', async () => {

workspaces/scorecard/packages/app/e2e-tests/utils/scorecardResponseUtils.ts

Lines changed: 80 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -171,48 +171,86 @@ export const invalidThresholdResponse = [
171171
];
172172

173173
// Aggregated scorecard responses (15 GitHub entities, 10 Jira entities)
174-
export const githubAggregatedResponse = [
175-
{
176-
id: 'github.open_prs',
177-
status: 'success',
178-
metadata: {
179-
title: 'GitHub open PRs',
180-
description:
181-
'Current count of open Pull Requests for a given GitHub repository.',
182-
type: 'number',
183-
history: true,
184-
},
185-
result: {
186-
values: [
187-
{ count: 5, name: 'success' },
188-
{ count: 7, name: 'warning' },
189-
{ count: 3, name: 'error' },
190-
],
191-
total: 15,
192-
timestamp: '2026-01-24T14:10:32.858Z',
193-
},
174+
export const githubAggregatedResponse = {
175+
id: 'github.open_prs',
176+
status: 'success',
177+
metadata: {
178+
title: 'GitHub open PRs',
179+
description:
180+
'Current count of open Pull Requests for a given GitHub repository.',
181+
type: 'number',
182+
history: true,
194183
},
195-
];
184+
result: {
185+
values: [
186+
{ count: 5, name: 'success' },
187+
{ count: 7, name: 'warning' },
188+
{ count: 3, name: 'error' },
189+
],
190+
total: 15,
191+
timestamp: '2026-01-24T14:10:32.858Z',
192+
},
193+
};
196194

197-
export const jiraAggregatedResponse = [
198-
{
199-
id: 'jira.open_issues',
200-
status: 'success',
201-
metadata: {
202-
title: 'Jira open blocking tickets',
203-
description:
204-
'Highlights the number of issues that are currently open in Jira.',
205-
type: 'number',
206-
history: true,
207-
},
208-
result: {
209-
values: [
210-
{ count: 6, name: 'success' },
211-
{ count: 3, name: 'warning' },
212-
{ count: 1, name: 'error' },
213-
],
214-
total: 10,
215-
timestamp: '2026-01-24T14:10:32.776Z',
216-
},
195+
export const jiraAggregatedResponse = {
196+
id: 'jira.open_issues',
197+
status: 'success',
198+
metadata: {
199+
title: 'Jira open blocking tickets',
200+
description:
201+
'Highlights the number of issues that are currently open in Jira.',
202+
type: 'number',
203+
history: true,
217204
},
218-
];
205+
result: {
206+
values: [
207+
{ count: 6, name: 'success' },
208+
{ count: 3, name: 'warning' },
209+
{ count: 1, name: 'error' },
210+
],
211+
total: 10,
212+
timestamp: '2026-01-24T14:10:32.776Z',
213+
},
214+
};
215+
216+
export const emptyJiraAggregatedResponse = {
217+
id: 'jira.open_issues',
218+
status: 'success',
219+
metadata: {
220+
title: 'Jira open blocking tickets',
221+
description:
222+
'Highlights the number of critical, blocking issues that are currently open in Jira.',
223+
type: 'number',
224+
history: true,
225+
},
226+
result: {
227+
total: 0,
228+
values: [
229+
{ count: 0, name: 'success' },
230+
{ count: 0, name: 'warning' },
231+
{ count: 0, name: 'error' },
232+
],
233+
timestamp: '2026-01-24T14:10:32.858Z',
234+
},
235+
};
236+
237+
export const emptyGithubAggregatedResponse = {
238+
id: 'github.open_prs',
239+
status: 'success',
240+
metadata: {
241+
title: 'GitHub open PRs',
242+
description:
243+
'Current count of open Pull Requests for a given GitHub repository.',
244+
type: 'number',
245+
history: true,
246+
},
247+
result: {
248+
total: 0,
249+
values: [
250+
{ count: 0, name: 'success' },
251+
{ count: 0, name: 'warning' },
252+
{ count: 0, name: 'error' },
253+
],
254+
timestamp: '2026-01-24T14:10:32.858Z',
255+
},
256+
};

workspaces/scorecard/plugins/scorecard-backend/README.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ Returns a list of available metrics. Supports filtering by metric IDs or datasou
107107

108108
#### Query Parameters
109109

110-
| Parameter | Type | Required | Description |
111-
| ------------ | ------ | -------- | -------------------------------------------------------------------------------------------- |
112-
| `metricIds` | string | No | Comma-separated list of metric IDs to filter by (e.g., `github.open_prs,github.open_issues`) |
113-
| `datasource` | string | No | Filter metrics by datasource ID (e.g., `github`, `jira`, `sonar`) |
110+
| Parameter | Type | Required | Description |
111+
| ------------ | ------ | -------- | ------------------------------------------------------------------------------------------ |
112+
| `metricIds` | string | No | Comma-separated list of metric IDs to filter by (e.g., `github.open_prs,jira.open_issues`) |
113+
| `datasource` | string | No | Filter metrics by datasource ID (e.g., `github`, `jira`, `sonar`) |
114114

115115
#### Behavior
116116

@@ -127,7 +127,7 @@ curl -X GET "{{url}}/api/scorecard/metrics" \
127127
-H "Authorization: Bearer <token>"
128128
129129
# Get specific metrics by IDs
130-
curl -X GET "{{url}}/api/scorecard/metrics?metricIds=github.open_prs,github.open_issues" \
130+
curl -X GET "{{url}}/api/scorecard/metrics?metricIds=github.open_prs,jira.open_issues" \
131131
-H "Authorization: Bearer <token>"
132132
133133
# Get all metrics from a specific datasource
@@ -149,9 +149,9 @@ Returns the latest metric values for a specific catalog entity.
149149

150150
#### Query Parameters
151151

152-
| Parameter | Type | Required | Description |
153-
| ----------- | ------ | -------- | -------------------------------------------------------------------------------------------- |
154-
| `metricIds` | string | No | Comma-separated list of metric IDs to filter by (e.g., `github.open_prs,github.open_issues`) |
152+
| Parameter | Type | Required | Description |
153+
| ----------- | ------ | -------- | ------------------------------------------------------------------------------------------ |
154+
| `metricIds` | string | No | Comma-separated list of metric IDs to filter by (e.g., `github.open_prs,jira.open_issues`) |
155155

156156
#### Permissions
157157

workspaces/scorecard/plugins/scorecard-backend/__fixtures__/mockDatabaseMetricValues.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,21 @@ type BuildMockDatabaseMetricValuesParams = {
2121
metricValues?: DbMetricValue[];
2222
latestEntityMetric?: DbMetricValue[];
2323
countOfExpiredMetrics?: number;
24-
aggregatedMetrics?: DbAggregatedMetric[];
24+
aggregatedMetric?: DbAggregatedMetric;
2525
};
2626

2727
export const mockDatabaseMetricValues = {
2828
createMetricValues: jest.fn(),
2929
readLatestEntityMetricValues: jest.fn(),
3030
cleanupExpiredMetrics: jest.fn(),
31-
readAggregatedMetricsByEntityRefs: jest.fn(),
31+
readAggregatedMetricByEntityRefs: jest.fn(),
3232
} as unknown as jest.Mocked<DatabaseMetricValues>;
3333

3434
export const buildMockDatabaseMetricValues = ({
3535
metricValues,
3636
latestEntityMetric,
3737
countOfExpiredMetrics,
38-
aggregatedMetrics,
38+
aggregatedMetric,
3939
}: BuildMockDatabaseMetricValuesParams) => {
4040
const createMetricValues = metricValues
4141
? jest.fn().mockResolvedValue(metricValues)
@@ -49,14 +49,14 @@ export const buildMockDatabaseMetricValues = ({
4949
? jest.fn().mockResolvedValue(countOfExpiredMetrics)
5050
: mockDatabaseMetricValues.cleanupExpiredMetrics;
5151

52-
const readAggregatedMetricsByEntityRefs = aggregatedMetrics
53-
? jest.fn().mockResolvedValue(aggregatedMetrics)
54-
: mockDatabaseMetricValues.readAggregatedMetricsByEntityRefs;
52+
const readAggregatedMetricByEntityRefs = aggregatedMetric
53+
? jest.fn().mockResolvedValue(aggregatedMetric)
54+
: mockDatabaseMetricValues.readAggregatedMetricByEntityRefs;
5555

5656
return {
5757
createMetricValues,
5858
readLatestEntityMetricValues,
5959
cleanupExpiredMetrics,
60-
readAggregatedMetricsByEntityRefs,
60+
readAggregatedMetricByEntityRefs,
6161
} as unknown as jest.Mocked<DatabaseMetricValues>;
6262
};

workspaces/scorecard/plugins/scorecard-backend/docs/aggregation.md

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -66,16 +66,23 @@ curl -X GET "{{url}}/api/scorecard/metrics/github.open_prs/catalog/aggregations"
6666
#### Key Features
6767

6868
- **Metric Access Validation**: This endpoint explicitly validates that the user has access to the specified metric and returns `403 Forbidden` if access is denied
69-
- **Empty Results Handling**: Returns an empty array `[]` when the user owns no entities, avoiding errors when filtering by a single metric
69+
- **Empty Results Handling**: Returns an empty aggregation object (zero counts with a timestamp) when the user owns no entities
7070

7171
## Error Handling
7272

7373
### Missing User Entity Reference
7474

7575
If the authenticated user doesn't have an entity reference in the catalog:
7676

77+
- **Status Code**: `401 Unauthorized`
78+
- **Error**: `AuthenticationError: User entity reference not found`
79+
80+
### User Entity Not Found in the Catalog
81+
82+
If the user entity doesn't exist in the catalog.
83+
7784
- **Status Code**: `404 Not Found`
78-
- **Error**: `NotFoundError: User entity reference not found`
85+
- **Error**: `NotFoundError: User entity not found in catalog`
7986

8087
### Permission Denied
8188

@@ -91,16 +98,9 @@ If the user doesn't have access to the specified metric:
9198
- **Status Code**: `403 Forbidden`
9299
- **Error**: `NotAllowedError: To view the scorecard metrics, your administrator must grant you the required permission.`
93100

94-
### Invalid Query Parameters
95-
96-
If invalid query parameters are provided:
97-
98-
- **Status Code**: `400 Bad Request`
99-
- **Error**: Validation error details
100-
101101
## Best Practices
102102

103-
1. **Handle Empty Results**: Always check for empty arrays when the user owns no entities
103+
1. **Handle Empty Results**: Always handle empty aggregations (zero counts) when the user owns no entities
104104

105105
2. **Group Structure**: Be aware of the direct parent group limitation when designing your group hierarchy. You currently receive scorecard results only for entities you own and those of your immediate parent group. To include results from _all_ parent
106106
groups, you can either implement custom logic, restructure your groups, or (if using RHDH), enable transitive parent groups ([see transitive parent group enablement documentation](https://docs.redhat.com/en/documentation/red_hat_developer_hub/1.5/html-single/authorization_in_red_hat_developer_hub/index#enabling-transitive-parent-groups)).

workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.test.ts

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,9 @@ describe('DatabaseMetricValues', () => {
204204
);
205205
});
206206

207-
describe('readAggregatedMetricsByEntityRefs', () => {
207+
describe('readAggregatedMetricByEntityRefs', () => {
208208
it.each(databases.eachSupportedId())(
209-
'should return aggregated metrics by status for multiple entities and metrics - %p',
209+
'should return aggregated metrics by status for multiple entities - %p',
210210
async databaseId => {
211211
const { client, db } = await createDatabase(databaseId);
212212

@@ -240,34 +240,21 @@ describe('DatabaseMetricValues', () => {
240240
},
241241
]);
242242

243-
const result = await db.readAggregatedMetricsByEntityRefs(
243+
const result = await db.readAggregatedMetricByEntityRefs(
244244
[
245245
'component:default/test-service',
246246
'component:default/another-service',
247247
],
248-
['github.metric1', 'github.metric2'],
249-
);
250-
251-
expect(result).toHaveLength(2);
252-
253-
const metric1Result = result.find(
254-
r => r.metric_id === 'github.metric1',
255-
);
256-
const metric2Result = result.find(
257-
r => r.metric_id === 'github.metric2',
248+
'github.metric2',
258249
);
259250

260-
expect(metric1Result).toBeDefined();
261-
expect(metric1Result?.total).toBe(2);
262-
expect(metric1Result?.success).toBe(2);
263-
expect(metric1Result?.warning).toBe(0);
264-
expect(metric1Result?.error).toBe(0);
265-
266-
expect(metric2Result).toBeDefined();
267-
expect(metric2Result?.total).toBe(2);
268-
expect(metric2Result?.success).toBe(0);
269-
expect(metric2Result?.warning).toBe(1);
270-
expect(metric2Result?.error).toBe(1);
251+
expect(result).toBeDefined();
252+
expect(result?.metric_id).toBe('github.metric2');
253+
expect(result?.total).toBe(2);
254+
expect(result?.success).toBe(0);
255+
expect(result?.warning).toBe(1);
256+
expect(result?.error).toBe(1);
257+
expect(result?.max_timestamp).toEqual(laterTime);
271258
},
272259
);
273260
});

workspaces/scorecard/plugins/scorecard-backend/src/database/DatabaseMetricValues.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,17 +67,17 @@ export class DatabaseMetricValues {
6767
/**
6868
* Get aggregated metrics by status for multiple entities and metrics.
6969
*/
70-
async readAggregatedMetricsByEntityRefs(
70+
async readAggregatedMetricByEntityRefs(
7171
catalog_entity_refs: string[],
72-
metric_ids: string[],
73-
): Promise<DbAggregatedMetric[]> {
72+
metric_id: string,
73+
): Promise<DbAggregatedMetric | undefined> {
7474
const latestIdsSubquery = this.dbClient(this.tableName)
7575
.max('id')
76-
.whereIn('metric_id', metric_ids)
76+
.where('metric_id', metric_id)
7777
.whereIn('catalog_entity_ref', catalog_entity_refs)
7878
.groupBy('metric_id', 'catalog_entity_ref');
7979

80-
const results = await this.dbClient(this.tableName)
80+
const [row] = await this.dbClient(this.tableName)
8181
.select('metric_id')
8282
.count('* as total')
8383
.max('timestamp as max_timestamp')
@@ -104,7 +104,7 @@ export class DatabaseMetricValues {
104104
// Normalize types for cross-database compatibility
105105
// PostgreSQL returns COUNT/SUM as strings, SQLite returns numbers
106106
// PostgreSQL returns MAX(timestamp) as Date, SQLite returns number (milliseconds)
107-
return results.map(row => {
107+
if (row) {
108108
let maxTimestamp: Date;
109109
if (row.max_timestamp instanceof Date) {
110110
maxTimestamp = row.max_timestamp;
@@ -125,6 +125,8 @@ export class DatabaseMetricValues {
125125
warning: Number(row.warning),
126126
error: Number(row.error),
127127
};
128-
});
128+
}
129+
130+
return undefined;
129131
}
130132
}

0 commit comments

Comments
 (0)