feat(scorecard): add entities page for drilling down aggregated KPIs#2509
feat(scorecard): add entities page for drilling down aggregated KPIs#2509Eswaraiahsapram wants to merge 9 commits intoredhat-developer:mainfrom
Conversation
Eswaraiahsapram
requested review from
a team,
christoph-jerolimov and
its-mitesh-kumar
as code owners
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Review Summary by QodoAdd Scorecard Entities page with drill-down capability from aggregated KPIs
WalkthroughsDescription• Introduces a new Scorecard Entities page enabling drilling down from aggregated KPIs to underlying entities • Adds API methods getMetrics() and getAggregatedScorecardEntities() for fetching metric and entity-level data with pagination and sorting • Implements comprehensive entities table component with pagination controls, sortable columns, and row visibility management • Adds info icon on scorecard cards displaying metric last-updated timestamps with navigation to entities page • Creates specialized cell components for entity name (with catalog links), owner (with entity linking), and metric status display • Adds custom hooks for fetching metrics, aggregated entities, entity metadata caching, and user ownership refs • Includes extensive test coverage with 15+ test suites covering table components, pagination, state management, and utility functions • Adds translations for 6 languages (Japanese, German, Spanish, Italian, French, and reference translations) with 30-40+ keys per language • Registers new route /scorecard/metrics/:metricId for entities page navigation • Updates API documentation and adds changeset for feature release Diagramflowchart LR
A["Scorecard Homepage Card"] -->|"click info icon"| B["Entities Page"]
A -->|"click card"| B
B -->|"displays"| C["Entities Table"]
C -->|"shows"| D["Entity Details"]
C -->|"with"| E["Pagination & Sorting"]
B -->|"displays"| F["Metric Status Chart"]
D -->|"links to"| G["Catalog Entity"]
D -->|"shows"| H["Owner Info"]
File Changes1. workspaces/scorecard/plugins/scorecard/src/api/index.ts
|
Code Review by Qodo
1.
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
...orecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTableHeader.tsx
Show resolved
Hide resolved
...paces/scorecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesRow.tsx
Show resolved
Hide resolved
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
from
802be24 to
9bdd4b1
Compare
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
2 times, most recently
from
64f8520 to
769d38d
Compare
|
@Eswaraiahsapram looks great to me! The only thing is that we need to fix the "Metric ID not found" screen, maybe we can use this UI? |
There was a problem hiding this comment.
@Eswaraiahsapram UI scaling issues have been observed when the resolution changes.
Screen.Recording.2026-03-12.at.4.47.32.PM.mov
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
The entity names in the aggregated scorecard drill-down change after a few seconds. Is this expected behavior?
Also, the API response initially shows "entityName": "red-hat-developer-hub", but changes to "Red Hat Developer Hub" shortly after. See the attached recording and screenshots.
Screen.Recording.2026-03-12.at.11.21.39.PM.mov
Hi @HusneShabbir , this is expected. Initially, we receive the entity names. And then we fetch the entity titles using Catalog API (similar to what we did in Adoption insights #2311). |
...card/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/MetricStatusCell.tsx
Show resolved
Hide resolved
...ard/plugins/scorecard/src/components/ScorecardEntitiesPage/EntitiesTable/cells/OwnerCell.tsx
Outdated
Show resolved
Hide resolved
...orecard/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/EntitiesTableFooter.tsx
Show resolved
Hide resolved
...ins/scorecard/src/components/ScorecardEntitiesPage/EntitiesTable/EntitiesTablePagination.tsx
Outdated
Show resolved
Hide resolved
...ins/scorecard/src/components/ScorecardEntitiesPage/EntitiesTable/EntitiesTablePagination.tsx
Outdated
Show resolved
Hide resolved
workspaces/scorecard/plugins/scorecard/src/hooks/useOwnershipEntityRefs.ts
Show resolved
Hide resolved
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
3 times, most recently
from
170e861 to
873419d
Compare
|
Thank you for implementation new logic and applying changes after review. I would like to ask you to check Point regarding displaying entities which have failed statusFollowing the Figma design, the status should be displayed in grey; however, in our current implementation, it appears as green. Question about warning iconShould this icon be hidden when all entities are displayed without errors, or should it always be visible? Question about "Last updated" valueFor my opinion displaying Today for last updated may confuse user as it's not clear to understand when actually this happened. Can we display something like Question about success synced metrics versus total number metricsThe Figma design displays a ratio between successfully synced entity metrics and the total number of scorecard-supported entities (e.g., 37/45 entities). Are we expected to implement this logic in this PR, or will it be handled in a separate one? |
|
Thanks, @imykhno, for pointing out these items
For this one, we have a Jira - https://redhat.atlassian.net/browse/RHIDP-12633. I'm planning to work on it as separate PR. I'm also integrating
We are currently using pagination in this table. Because of that, it’s not possible to determine whether all entities have values or a valid status across the full dataset. While filtering could help, it only applies to the current page. This may lead to cases where the first page looks fine, but missing data on subsequent pages causes the warning icon to toggle inconsistently.
I have created a story for this - https://redhat.atlassian.net/browse/RHIDP-12712
Once the backend changes are ready, I’ll update this. If needed, I’ll create a separate task for it. |
...card/plugins/scorecard/src/components/ScorecardPage/EntitiesTable/cells/MetricStatusCell.tsx
Show resolved
Hide resolved
...gins/scorecard/src/components/ScorecardEntitiesPage/EntitiesTable/cells/MetricStatusCell.tsx
Show resolved
Hide resolved
Definitely agree on consistent display. I'm just not convinced the warning icon should always be visible. Maybe @PatAKnight could have the backend send back this information when it fetches all that entities anyway.
cc @PatAKnight |
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
from
47f632c to
e86b10f
Compare
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
from
e86b10f to
cda6f67
Compare
workspaces/scorecard/plugins/scorecard/src/hooks/useAggregatedScorecardEntities.tsx
Show resolved
Hide resolved
imykhno
left a comment
imykhno
left a comment
There was a problem hiding this comment.
Thank you @Eswaraiahsapram for this PR. I have tested it locally, and it works as expected. All the questions I raised have been addressed, and we have agreed that the remaining items will be fixed or implemented in separate PRs (tasks).
/lgtm
christoph-jerolimov
left a comment
christoph-jerolimov
left a comment
There was a problem hiding this comment.
Some more or less small change requests. Please take a look and I will try to review it tomorrow evening again.
| // Ignore button-name for icon-only buttons that have a tooltip (e.g. scorecard "Last updated" info icon) | ||
| const filteredViolations = accessibilityScanResults.violations.filter( | ||
| v => v.id !== 'button-name', | ||
| ); |
|
|
||
| test('Verify threshold tooltips', async () => { | ||
| test('Verify threshold and last updated tooltips', async () => { | ||
| const lastUpdatedFormatted = '24 Jan 2026'; |
There was a problem hiding this comment.
Are we running the tests with different languages? Shouldn't this string be depending on that language?
There was a problem hiding this comment.
Good point 👍
Currently, lastupdated is not localized, and we're using a fixed format in English. As part of this task https://redhat.atlassian.net/browse/RHIDP-12712 we can address localization and update the tests.
| sx={{ | ||
| width: 520, | ||
| height: 480, | ||
| '& > *': { | ||
| height: '100%', | ||
| minHeight: 0, | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| }, |
There was a problem hiding this comment.
Why do we need this "magic code" here? Magic means for me that numbers comes from "somewhere" that I coudln't understand without more information.
width: 420, height: 480? 🤷♂️
Can you explain it here or maybe better add a comment that this is aligned with a common box in the homepage? When that's the reason? But shouldn't this card work anyway on different layouts?
Maybe its worth to have here multiple ScorecardHomepageCards with different sizes so that its easy to verify how they look look like in different cases?
There was a problem hiding this comment.
I can add multiple ScorecardHomepageCard variants with different sizes so it's easier to verify how it behaves across layouts. Ideally, the card should adapt to the parent’s width and height. That’s the intention behind these styles.
I'll also add a comment
| getAggregatedScorecardEntities(options: { | ||
| metricId: string; | ||
| page: number; | ||
| pageSize: number; | ||
| ownershipEntityRefs?: string[]; | ||
| orderBy?: string | null; | ||
| order?: 'asc' | 'desc'; | ||
| }): Promise<EntityMetricDetailResponse>; |
There was a problem hiding this comment.
Can you please extract the options type as GetAggregatedScorecardEntitiesOptions (with or without prefix Get) so that the same typedefinition isn't needed in the implementation?
| * @param metricId - The ID of the metric to get aggregated entities for | ||
| * @param page - The page number to retrieve | ||
| * @param pageSize - The number of entities per page | ||
| * @param ownershipEntityRefs - Optional array of ownership entity refs to filter entities by | ||
| * @param orderBy - Optional column to sort by | ||
| * @param order - Optional sort order |
There was a problem hiding this comment.
That documentation isn't right. The param is called options and its an object. The documentation says there are 6 params. Please move that documentation to a new type for the options.
|
|
||
| import Box from '@mui/material/Box'; | ||
| import { useTheme } from '@mui/material/styles'; | ||
| import MuiTooltip from '@mui/material/Tooltip'; |
There was a problem hiding this comment.
| import MuiTooltip from '@mui/material/Tooltip'; | |
| import Tooltip from '@mui/material/Tooltip'; |
| const [page, setPage] = useState<number>(1); | ||
| const [rowsPerPage, setRowsPerPage] = useState<number>(5); | ||
| const { t } = useTranslation(); | ||
|
|
||
| const [sortState, setSortState] = useState<{ | ||
| orderBy: string | null; | ||
| order: 'asc' | 'desc'; | ||
| }>({ | ||
| orderBy: null, | ||
| order: 'asc', | ||
| }); | ||
|
|
||
| const { orderBy, order } = sortState; | ||
|
|
||
| const ownershipEntityRefs = useOwnershipEntityRefs(); | ||
|
|
||
| const { | ||
| aggregatedScorecardEntities, | ||
| loadingData: loadingDataEntities, | ||
| error: entitiesError, | ||
| } = useAggregatedScorecardEntities({ | ||
| metricId: metricId as string, | ||
| page, | ||
| pageSize: rowsPerPage, | ||
| ownershipEntityRefs, | ||
| orderBy, | ||
| order, | ||
| }); | ||
|
|
||
| const isNotFound = entitiesError?.message?.includes('NotFoundError'); | ||
| if (isNotFound) { | ||
| setMetricNotFound?.(true); | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| setMetricTitle(aggregatedScorecardEntities?.metricMetadata?.title ?? ''); | ||
| }, [aggregatedScorecardEntities?.metricMetadata?.title, setMetricTitle]); | ||
|
|
||
| const handleChangeRowsPerPage = useCallback( | ||
| (event: ChangeEvent<HTMLInputElement>) => { | ||
| setRowsPerPage(Number(event.target.value)); | ||
| }, | ||
| [], | ||
| ); | ||
|
|
||
| const handleSortRequest = useCallback((columnId: string) => { | ||
| setSortState(prev => | ||
| prev.orderBy !== columnId | ||
| ? { orderBy: columnId, order: 'asc' } | ||
| : { ...prev, order: prev.order === 'asc' ? 'desc' : 'asc' }, | ||
| ); | ||
| }, []); |
There was a problem hiding this comment.
It would be nice if we would use Backstage Core Components Table here. But as we discussed, we can keep that for now and clean it up when we migrate to Backstage UI.
There was a problem hiding this comment.
Sure, thanks a lot 😊
| return ( | ||
| <Box | ||
| component={Paper} | ||
| elevation={0} | ||
| sx={{ | ||
| display: 'flex', | ||
| justifyContent: 'flex-end', | ||
| padding: 1, | ||
| }} | ||
| > | ||
| <TablePagination | ||
| labelRowsPerPage={null} | ||
| count={count} | ||
| page={page} | ||
| rowsPerPageOptions={rowsPerPageOptions} | ||
| rowsPerPage={rowsPerPage} | ||
| onPageChange={handleChangePage} | ||
| onRowsPerPageChange={handleChangeRowsPerPage} | ||
| ActionsComponent={EntitiesTablePagination} | ||
| component="div" | ||
| sx={{ | ||
| '& .v5-MuiTablePagination-select:focus': { | ||
| backgroundColor: 'transparent', | ||
| }, | ||
| border: 'none', | ||
| '& .v5-MuiTablePagination-input': { | ||
| mr: 1, | ||
| }, | ||
|
|
||
| '& .v5-MuiTablePagination-displayedRows': { | ||
| display: 'none', | ||
| }, |
There was a problem hiding this comment.
The complete header wouldn't be needed with Backstage Core Components Table or Backstage UI Table.
I'm also not a fan of all these extra styles here! We should stay more on the detaults otherwise this will create a big tech debt for the future and the risk that our plugins a) looks different then others and b) doesn't look with other themes!
cc @rohitkrai03 @imykhno @redhat-developer/rhdh-ui
There was a problem hiding this comment.
Btw, did you have tested these extra styles just with yarn start or also on a cluster or rhdh-local with a build version. I remember that some styles are minified in MUI and works only in development mode.
Please double check with a final build version as well.
There was a problem hiding this comment.
I've only tested it with yarn start so far. I'll verify it with a final build version as well
| sx={{ | ||
| p: 3, | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| fontWeight: 'bold', | ||
| fontSize: '1.5rem', |
There was a problem hiding this comment.
Why is a custom font size needed here? Couldn't we go with one of the default headlines?
| export const ScorecardEntitiesPage = scorecardPlugin.provide( | ||
| createRoutableExtension({ | ||
| name: 'ScorecardEntitiesPage', | ||
| component: () => | ||
| import('./components/ScorecardHomepageSection').then( | ||
| m => m.ScorecardEntitiesPage, | ||
| ), |
There was a problem hiding this comment.
ScorecardEntitiesPage is a named export ScorecardEntitiesPage in the file ScorecardHomepageSection. That's confusing.
Can you please please clean this up and create a dedicated components/ScorecardEntitiesPage or pages/ScorecardEntitiesPage and exports with the same name just the page.
Personally I would call the page ScorecardPage -- and when we add NFS support if can split it in subpages and call this one ScorecardEntitiesSubpage (as subpage).
So for now I would prefer pages/ScorecardPage.tsx. Wdyt?
There was a problem hiding this comment.
I agree the current naming is confusing. I’ll refactor this by moving it to pages/ScorecardPage.tsx and keep the export aligned with the file name.
…, a11y (#4) - Use homePage.getCard() for ARIA snapshot assertions (scope to scorecard article) - Add getThresholdsSnapshot link with /url for drill-down; keep getMissingPermissionSnapshot without link - Add getLastUpdatedLabel and verifyLastUpdatedTooltip in HomePage (POM, translation-based) - Rename test to 'Verify threshold and last updated tooltips' - Filter button-name violations in accessibility helper for icon-only tooltip buttons Made-with: Cursor Co-authored-by: HusneShabbir <husneshabbir447@gmail.com>
Eswaraiahsapram
force-pushed
the
feat/scorecard-entities-page-ui
branch
from
12550e9 to
3d36564
Compare
|
New changes are detected. LGTM label has been removed. |
|
6 participants
Hey, I just made a Pull Request!
Fixes:
Description
This PR introduces the Scorecard Entities page and enhances the homepage scorecard to support drilling down from aggregated KPIs to the underlying entities contributing to those metrics.
The goal is to provide better visibility into aggregated scorecard data so that Engineering Managers and Platform Engineers can identify specific services impacting metrics and investigate issues at the entity level.
What’s Included
Aggregated Scorecard Enhancements
Scorecard Entities Page
Introduces a new page that displays entity-level details for a selected scorecard metric.
Screenshots
✔️ Checklist