Skip to content

fix(scorecard): add threshold-based status colors to entities table#2640

Open
Eswaraiahsapram wants to merge 13 commits intoredhat-developer:mainfrom
Eswaraiahsapram:fix/scorecard-status-color-threshold
Open

fix(scorecard): add threshold-based status colors to entities table#2640
Eswaraiahsapram wants to merge 13 commits intoredhat-developer:mainfrom
Eswaraiahsapram:fix/scorecard-status-color-threshold

Conversation

@Eswaraiahsapram
Copy link
Copy Markdown
Member

@Eswaraiahsapram Eswaraiahsapram commented Mar 30, 2026

Hey, I just made a Pull Request!

Fix

Description

  • Derive status indicator colors in Status column from the metric's threshold configuration instead of hardcoded MUI palette values
  • Replace react-use/useAsync with @tanstack/react-query in useAggregatedScorecard, useAggregatedScorecardEntities, and useMetric for better caching and request deduplication
  • Add ScorecardQueryProvider and queryClient utility to provide a stable QueryClient for scorecard components rendered outside an existing provider tree
  • Export ScorecardPageWithProvider and ScorecardHomepageCardWithProvider for standalone usage (e.g. on the homepage)
  • Gate the entities fetch in EntitiesTable until useOwnershipEntityRefs has finished resolving to avoid a race condition with an empty refs array
  • ScorecardPage shows customized title if it configured

Screenshots

Customized status colors

image
Screen.Recording.2026-03-30.at.1.34.07.PM.mov

Customized titles

image image

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app bot commented Mar 30, 2026

Changed Packages

Package Name Package Path Changeset Bump Current Version
app-legacy workspaces/scorecard/packages/app-legacy none v0.0.0
@red-hat-developer-hub/backstage-plugin-scorecard workspaces/scorecard/plugins/scorecard patch v2.5.0

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Migrate to react-query and add threshold-based status colors for entities

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Migrate data fetching from react-use to @tanstack/react-query for better caching and state
  management
• Add threshold-based status colors to entities table with new getThresholdRuleColor utility
• Wrap scorecard components with ScorecardQueryProvider for standalone homepage rendering
• Fix ownership loading state handling in entities table to prevent premature data display
Diagram
flowchart LR
  A["react-use hooks"] -->|"migrate to"| B["@tanstack/react-query"]
  B -->|"enables"| C["QueryClientProvider wrapper"]
  C -->|"supports"| D["Standalone component rendering"]
  E["Threshold rules"] -->|"compute"| F["Status colors"]
  F -->|"applied to"| G["MetricStatusCell"]
  H["Ownership loading state"] -->|"fix"| I["Prevent premature rendering"]
Loading

Grey Divider

File Changes

1. workspaces/scorecard/plugins/scorecard/src/hooks/useAggregatedScorecard.tsx ✨ Enhancement +36/-33

Migrate from react-use to react-query

workspaces/scorecard/plugins/scorecard/src/hooks/useAggregatedScorecard.tsx


2. workspaces/scorecard/plugins/scorecard/src/hooks/useAggregatedScorecardEntities.tsx ✨ Enhancement +46/-49

Migrate to react-query with enabled flag support

workspaces/scorecard/plugins/scorecard/src/hooks/useAggregatedScorecardEntities.tsx


3. workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx ✨ Enhancement +29/-34

Migrate to react-query for metric fetching

workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx


View more (13)
4. workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx ✨ Enhancement +37/-0

New provider component for QueryClient

workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx


5. workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ScorecardHomepageCard.tsx ✨ Enhancement +15/-0

Add provider wrapper for standalone rendering

workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/ScorecardHomepageCard.tsx


6. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/ScorecardPage.tsx ✨ Enhancement +11/-0

Add provider wrapper for standalone rendering

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/ScorecardPage.tsx


7. workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/index.ts ✨ Enhancement +4/-1

Export new provider-wrapped component variant

workspaces/scorecard/plugins/scorecard/src/components/ScorecardHomepageSection/index.ts


8. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/index.ts ✨ Enhancement +1/-1

Export new provider-wrapped component variant

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/index.ts


9. workspaces/scorecard/plugins/scorecard/src/plugin.ts ✨ Enhancement +3/-2

Use provider-wrapped component exports

workspaces/scorecard/plugins/scorecard/src/plugin.ts


10. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx ✨ Enhancement +22/-2

Add threshold-based status color computation

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx


11. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/MetricStatusCell.tsx ✨ Enhancement +11/-4

Use threshold-based color for status indicator

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/MetricStatusCell.tsx


12. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx 🐞 Bug fix +19/-12

Fix ownership loading state and pass metricId to rows

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx


13. workspaces/scorecard/plugins/scorecard/package.json Dependencies +1/-1

Add react-query dependency and remove unused ui package

workspaces/scorecard/plugins/scorecard/package.json


14. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/__tests__/EntitiesRow.test.tsx 🧪 Tests +14/-0

Update tests for threshold color and metricId prop

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/tests/EntitiesRow.test.tsx


15. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/__tests__/EntitiesTable.test.tsx 🧪 Tests +5/-1

Update tests for ownership loading state handling

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/tests/EntitiesTable.test.tsx


16. workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/__tests__/MetricStatusCell.test.tsx 🧪 Tests +16/-4

Update tests for statusColor prop parameter

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/tests/MetricStatusCell.test.tsx


Grey Divider

Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge bot commented Mar 30, 2026

Code Review by Qodo

🐞 Bugs (3)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ≡ Correctness (1) ☼ Reliability (1) ➹ Performance (1)

Grey Divider


Action required

1. Missing queryClient module 🐞
Description
ScorecardQueryProvider imports "../utils/queryClient", but the scorecard plugin source tree does not
contain that module, so the package will fail module resolution/compilation.
Code

workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx[R17-21]

+import type { ReactNode } from 'react';
+import { QueryClientProvider } from '@tanstack/react-query';
+
+import queryClient from '../utils/queryClient';
+
Evidence
The new provider hard-depends on a default export named queryClient from a path that is not defined
anywhere else in this plugin; without adding that module (or changing the import), bundling will
fail.

workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx[17-36]
workspaces/scorecard/plugins/scorecard/src/utils/index.ts[17-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ScorecardQueryProvider` imports `../utils/queryClient`, but there is no such module in the scorecard plugin. This will break module resolution and prevent the plugin from compiling/running.

### Issue Context
The PR migrated multiple hooks to `@tanstack/react-query`, so a stable `QueryClient` instance is required.

### Fix Focus Areas
- Add a `queryClient` module (e.g., `QueryClient` singleton) and export it as the default export expected by the provider.
- Optionally configure sensible defaults (commonly: `queries.retry: false` for UI, depending on project conventions).

- workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx[17-36]
- workspaces/scorecard/plugins/scorecard/src/utils/queryClient.ts[1-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Ownership hook return mismatch🐞
Description
EntitiesTable destructures useOwnershipEntityRefs() as an object with {ownershipEntityRefs,
loading}, but the hook currently returns only a string[]; this breaks the intended loading gating
and will fail TypeScript compilation.
Code

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx[R65-69]

+  const { ownershipEntityRefs, loading: ownershipLoading } =
+    useOwnershipEntityRefs();

  const {
    aggregatedScorecardEntities,
Evidence
EntitiesTable expects useOwnershipEntityRefs() to return an object containing both
ownershipEntityRefs and loading, but the hook implementation returns only the array. This makes
ownershipEntityRefs/ownershipLoading incorrect (and should be a TS error).

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx[63-80]
workspaces/scorecard/plugins/scorecard/src/hooks/useOwnershipEntityRefs.ts[20-31]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EntitiesTable` now assumes `useOwnershipEntityRefs()` returns `{ ownershipEntityRefs, loading }`, but the hook still returns only `string[]`. This breaks compilation and prevents the new `enabled: !ownershipLoading` gating from working.

### Issue Context
The PR added `enabled: !ownershipLoading` to avoid firing scorecard entity queries before identity ownership refs are loaded.

### Fix Focus Areas
- Update `useOwnershipEntityRefs` to return an object `{ ownershipEntityRefs, loading, error? }`.
- Set `loading=true` initially, then `false` after `getBackstageIdentity()` resolves (and handle rejection).
- Ensure `EntitiesTable` logic continues to work with the updated hook shape.

- workspaces/scorecard/plugins/scorecard/src/hooks/useOwnershipEntityRefs.ts[17-31]
- workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx[65-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. useMetric tests incompatible🐞
Description
useMetric now uses react-query useQuery, but its unit test still mocks react-use/useAsync and does
not provide a QueryClientProvider, so the test will fail (and no longer validates the hook
behavior).
Code

workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx[R17-20]

+import { useQuery } from '@tanstack/react-query';
import { useApi } from '@backstage/core-plugin-api';
import { Metric } from '@red-hat-developer-hub/backstage-plugin-scorecard-common';
Evidence
The hook implementation was migrated to useQuery, while the test suite still imports/mocks
useAsync and asserts useAsync-driven behavior; additionally, useQuery requires a
QueryClientProvider in tests.

workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx[17-61]
workspaces/scorecard/plugins/scorecard/src/hooks/tests/useMetric.test.tsx[17-32]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`useMetric` was migrated from `react-use/lib/useAsync` to `@tanstack/react-query`'s `useQuery`, but `useMetric.test.tsx` still mocks `useAsync` and asserts `useAsync` states. The test also needs a `QueryClientProvider` wrapper.

### Issue Context
Without updating tests, CI/unit tests will fail and the hook behavior will be unverified.

### Fix Focus Areas
- Rewrite the tests to:
 - create a `QueryClient` with retries disabled
 - wrap `renderHook` with `QueryClientProvider`
 - mock `useApi()` to return a scorecardApi stub and assert calls to `getMetrics`
 - use `waitFor` to assert eventual `metric` and `error` states
- Remove `useAsync` mocks/imports.

- workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx[17-61]
- workspaces/scorecard/plugins/scorecard/src/hooks/__tests__/useMetric.test.tsx[17-127]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. ScorecardPageWithProvider not exported🐞
Description
plugin.ts lazy-loads ScorecardPageWithProvider from "./pages/ScorecardPage", but that module only
exports ScorecardPage, so navigation will attempt to render an undefined component.
Code

workspaces/scorecard/plugins/scorecard/src/plugin.ts[R88-94]

export const ScorecardPage = scorecardPlugin.provide(
  createRoutableExtension({
    name: 'ScorecardPage',
-    component: () => import('./pages/ScorecardPage').then(m => m.ScorecardPage),
+    component: () =>
+      import('./pages/ScorecardPage').then(m => m.ScorecardPageWithProvider),
    mountPoint: rootRouteRef,
  }),
Evidence
The routable extension expects ScorecardPageWithProvider to be exported from
src/pages/ScorecardPage.tsx, but the file only re-exports ScorecardPage. That makes the lazy
import selector return undefined.

workspaces/scorecard/plugins/scorecard/src/plugin.ts[88-95]
workspaces/scorecard/plugins/scorecard/src/pages/ScorecardPage.tsx[17-17]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`plugin.ts` does `import('./pages/ScorecardPage').then(m => m.ScorecardPageWithProvider)`, but `src/pages/ScorecardPage.tsx` does not export `ScorecardPageWithProvider`. This will break the ScorecardPage route at runtime.

### Issue Context
The PR added `ScorecardPageWithProvider` in the components layer to ensure a `QueryClientProvider` exists when using react-query hooks.

### Fix Focus Areas
- Re-export `ScorecardPageWithProvider` from `src/pages/ScorecardPage.tsx`.
 - e.g., `export { ScorecardPage, ScorecardPageWithProvider } from '../components/ScorecardPage';`
- Alternatively, update the lazy import in `plugin.ts` to import from a module that already exports `ScorecardPageWithProvider`.

- workspaces/scorecard/plugins/scorecard/src/pages/ScorecardPage.tsx[17-17]
- workspaces/scorecard/plugins/scorecard/src/plugin.ts[88-95]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Removed dev dependency still imported 🐞
Description
@backstage/ui was removed from devDependencies, but the dev entrypoints still import
"@backstage/ui/css/styles.css", which will break dev start/build for this package.
Code

workspaces/scorecard/plugins/scorecard/package.json[R79-83]

    "@backstage/plugin-catalog": "^1.33.1",
    "@backstage/plugin-user-settings": "^0.9.0",
    "@backstage/test-utils": "^1.7.13",
-    "@backstage/ui": "^0.9.1",
    "@red-hat-developer-hub/backstage-plugin-theme": "^0.13.0",
    "@testing-library/jest-dom": "^6.0.0",
Evidence
The package no longer declares @backstage/ui as a dependency, while dev entrypoints still import
its CSS, so module resolution will fail when running the plugin in dev mode.

workspaces/scorecard/plugins/scorecard/package.json[73-89]
workspaces/scorecard/plugins/scorecard/dev/index.tsx[23-26]
workspaces/scorecard/plugins/scorecard/dev/legacy.tsx[17-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The package removes `@backstage/ui` from `devDependencies`, but `dev/index.tsx` and `dev/legacy.tsx` still import `@backstage/ui/css/styles.css`. This will break `yarn start` / dev workflows for the plugin.

### Issue Context
This appears unrelated to the react-query migration and looks like an accidental dependency drop.

### Fix Focus Areas
- Either add `@backstage/ui` back to `devDependencies`, OR remove/replace the CSS import with the correct styling entrypoint.

- workspaces/scorecard/plugins/scorecard/package.json[73-89]
- workspaces/scorecard/plugins/scorecard/dev/index.tsx[23-26]
- workspaces/scorecard/plugins/scorecard/dev/legacy.tsx[17-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Per-row thresholds query 🐞
Description
EntitiesRow calls useAggregatedScorecard per rendered row to obtain threshold rules, creating many
react-query observers/subscriptions for the same queryKey and avoidable render overhead.
Code

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx[R50-57]

+  const { aggregatedScorecard } = useAggregatedScorecard({
+    metricId: metricId as string,
+  });
+  const thresholdRules = aggregatedScorecard?.result?.thresholds?.rules ?? [];
+
+  const statusColor =
+    getThresholdRuleColor(thresholdRules, entity?.status ?? '') ??
+    SCORECARD_ERROR_STATE_COLOR;
Evidence
The row component itself performs the aggregated scorecard query and derives threshold colors, and
EntitiesTable renders one EntitiesRow per entity; moving the query to the table level would avoid N
hook instances for the same metricId.

workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx[37-58]
workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx[166-176]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EntitiesRow` calls `useAggregatedScorecard` for each row. Even though react-query caching prevents repeated network calls, it still creates many query observers and repeated derived computations.

### Issue Context
All rows share the same `metricId`, so threshold rules can be fetched once and passed down.

### Fix Focus Areas
- Move `useAggregatedScorecard({ metricId })` into `EntitiesTable` (or a table wrapper) and pass `thresholdRules` (or a `statusColorResolver`) into `EntitiesRow`.
- Keep `EntitiesRow` as a pure row renderer using props.

- workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx[37-58]
- workspaces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTable.tsx[166-176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread workspaces/scorecard/plugins/scorecard/src/components/ScorecardQueryProvider.tsx Outdated
Comment thread workspaces/scorecard/plugins/scorecard/src/plugin.ts
Comment thread workspaces/scorecard/plugins/scorecard/src/hooks/useMetric.tsx
@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch 2 times, most recently from c07674e to ee734b5 Compare March 30, 2026 08:57
@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch from ee734b5 to d7fa700 Compare March 31, 2026 16:32
@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch 3 times, most recently from f0f9874 to a96704f Compare April 7, 2026 05:43
@rohitratannagar
Copy link
Copy Markdown
Contributor

Thanks @Eswaraiahsapram , tested the pr locally in both legacy and nfs mode. Looks good to me. 🎉
/lgtm

Screen.Recording.2026-04-07.at.11.49.33.AM.mov

Copy link
Copy Markdown
Contributor

@HusneShabbir HusneShabbir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

/lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 8, 2026

New changes are detected. LGTM label has been removed.

@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch 4 times, most recently from 419f3b4 to 65e82cd Compare April 14, 2026 14:47
@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch 4 times, most recently from 6a72aad to 0a67320 Compare April 15, 2026 14:34
@Eswaraiahsapram Eswaraiahsapram force-pushed the fix/scorecard-status-color-threshold branch from d38e5bf to a264dc9 Compare April 15, 2026 16:17
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants