paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(catalog): show not-found instead of falling back to first route on unknown entity sub-paths (#34081)

Browse Source 
* fix(catalog): show not-found instead of falling back to first route on unknown entity sub-paths

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(catalog): address PR review feedback

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(core-compat-api): update entity page conversion test for new not-found behavior

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(catalog): use NotFoundErrorPage and drop redundant route sort

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(catalog): split slash trimming into two replacements for clarity

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(catalog): comment normalizeRoutePath regex and memoize routes

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(catalog): preserve explicit trailing wildcards in route paths

Signed-off-by: benjdlambert <ben@blam.sh>

---------

Signed-off-by: benjdlambert <ben@blam.sh>
This commit is contained in:
Ben Lambert
2026-05-12 09:57:28 +02:00
committed by GitHub
parent cbac98d9e7
commit 728629cf64
5 changed files with 239 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/entity-tab-not-found.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog': patch
---
Fixed an issue where navigating to an unknown sub-path on an entity page (for example `/catalog/default/component/foo/blob`) would silently render the first available route. Unknown paths now show the standard not-found page instead.
packages/core-compat-api/src/convertLegacyApp.test.tsx
+3 -2
View File
@@ -317,9 +317,10 @@ describe('convertLegacyApp', () => {
features: [catalogOverride, ...converted],
initialRouteEntries: ['/catalog/default/other/x/bar'],
});
// /bar does not exist on the "other" entity layout, expect the not-found page.
await expect(
renderBarOther.findByText('other overview content', {}, findOptions),
).resolves.toBeInTheDocument(); // /bar does not exist, fall back to rendering overview
renderBarOther.findByTestId('error', {}, findOptions),
).resolves.toBeInTheDocument();
renderBarOther.unmount();
});
});
plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.test.tsx
+195 -6
View File
@@ -15,7 +15,7 @@
*/
import { screen } from '@testing-library/react';
import { useSelectedSubRoute } from './EntityTabs';
import { EntityTabs, useSelectedSubRoute } from './EntityTabs';
import { EntityTabsList } from './EntityTabsList';
import { MemoryRouter, Route, Routes } from 'react-router-dom';
import { render } from '@testing-library/react';
@@ -139,7 +139,7 @@ describe('EntityTabs', () => {
];
describe('useSelectedSubRoute', () => {
it('should render the first route at root path', () => {
it('should not match when no route is registered for the root path', () => {
render(
<MemoryRouter initialEntries={['/']}>
<Routes>
@@ -151,10 +151,74 @@ describe('EntityTabs', () => {
</MemoryRouter>,
);
expect(screen.getByTestId('selected-index')).toHaveTextContent('-1');
expect(screen.getByTestId('element-container')).toBeEmptyDOMElement();
});
it('should match the index route when path is "/"', () => {
const subRoutesWithIndex = [
{
group: 'default',
path: '/',
title: 'Overview',
children: <div>Overview Content</div>,
},
{
group: 'default',
path: '/details',
title: 'Details',
children: <div>Details Content</div>,
},
];
render(
<MemoryRouter initialEntries={['/']}>
<Routes>
<Route
path="/*"
element={<TestSubRouteHook subRoutes={subRoutesWithIndex} />}
/>
</Routes>
</MemoryRouter>,
);
expect(screen.getByTestId('selected-index')).toHaveTextContent('0');
expect(screen.getByTestId('selected-route-title')).toHaveTextContent(
'Overview',
);
expect(screen.getByText('Overview Content')).toBeInTheDocument();
});
it('should not let the index route swallow unknown sub-paths', () => {
const subRoutesWithIndex = [
{
group: 'default',
path: '/',
title: 'Overview',
children: <div>Overview Content</div>,
},
{
group: 'default',
path: '/details',
title: 'Details',
children: <div>Details Content</div>,
},
];
render(
<MemoryRouter initialEntries={['/blob']}>
<Routes>
<Route
path="/*"
element={<TestSubRouteHook subRoutes={subRoutesWithIndex} />}
/>
</Routes>
</MemoryRouter>,
);
expect(screen.getByTestId('selected-index')).toHaveTextContent('-1');
expect(screen.queryByText('Overview Content')).not.toBeInTheDocument();
expect(screen.queryByText('Details Content')).not.toBeInTheDocument();
});
it('should render a route at non-root path', () => {
@@ -276,7 +340,7 @@ describe('EntityTabs', () => {
);
});
it('should fall back to first route for unknown paths', () => {
it('should not match unknown paths', () => {
render(
<MemoryRouter initialEntries={['/unknown-path']}>
<Routes>
@@ -288,10 +352,135 @@ describe('EntityTabs', () => {
</MemoryRouter>,
);
expect(screen.getByTestId('selected-index')).toHaveTextContent('0');
expect(screen.getByTestId('selected-route-title')).toHaveTextContent(
'Overview',
expect(screen.getByTestId('selected-index')).toHaveTextContent('-1');
expect(screen.getByTestId('element-container')).toBeEmptyDOMElement();
});
it('should accept paths that already include an explicit wildcard', () => {
const wildcardSubRoutes = [
{
group: 'default',
path: '/',
title: 'Overview',
children: <div>Overview Content</div>,
},
{
group: 'default',
path: '/docs/*',
title: 'Docs',
children: <div>Docs Content</div>,
},
];
render(
<MemoryRouter initialEntries={['/docs/api/v1']}>
<Routes>
<Route
path="/*"
element={<TestSubRouteHook subRoutes={wildcardSubRoutes} />}
/>
</Routes>
</MemoryRouter>,
);
expect(screen.getByTestId('selected-index')).toHaveTextContent('1');
expect(screen.getByTestId('selected-route-title')).toHaveTextContent(
'Docs',
);
expect(screen.getByText('Docs Content')).toBeInTheDocument();
});
});
describe('rendering', () => {
const tabRoutes = [
{
group: 'overview',
path: '/',
title: 'Overview',
children: <div>Overview Content</div>,
},
{
group: 'overview',
path: '/details',
title: 'Details',
children: <div>Details Content</div>,
},
];
it('renders the matched route content and no not-found page', async () => {
await renderInTestApp(
<Routes>
<Route
path="/*"
element={
<EntityTabs
routes={tabRoutes}
groupDefinitions={{ overview: { title: 'Overview' } }}
/>
}
/>
</Routes>,
{ initialRouteEntries: ['/details'] },
);
expect(await screen.findByText('Details Content')).toBeInTheDocument();
expect(screen.queryByTestId('error')).toBeNull();
});
it('renders the not-found page for unknown sub-paths', async () => {
await renderInTestApp(
<Routes>
<Route
path="/*"
element={
<EntityTabs
routes={tabRoutes}
groupDefinitions={{ overview: { title: 'Overview' } }}
/>
}
/>
</Routes>,
{ initialRouteEntries: ['/blob'] },
);
expect(await screen.findByTestId('error')).toBeInTheDocument();
expect(screen.queryByText('Overview Content')).not.toBeInTheDocument();
expect(screen.queryByText('Details Content')).not.toBeInTheDocument();
});
it('still routes nested sub-paths to the matching tab content', async () => {
const nestedRoutes = [
{
group: 'overview',
path: '/',
title: 'Overview',
children: <div>Overview Content</div>,
},
{
group: 'overview',
path: '/docs',
title: 'Docs',
children: <div>Docs Content</div>,
},
];
await renderInTestApp(
<Routes>
<Route
path="/*"
element={
<EntityTabs
routes={nestedRoutes}
groupDefinitions={{ overview: { title: 'Overview' } }}
/>
}
/>
</Routes>,
{ initialRouteEntries: ['/docs/api/v1'] },
);
expect(await screen.findByText('Docs Content')).toBeInTheDocument();
expect(screen.queryByTestId('error')).toBeNull();
});
});
});
plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.tsx
+34 -22
View File
@@ -16,6 +16,7 @@
import { ReactElement, useMemo } from 'react';
import { Helmet } from 'react-helmet';
import { matchRoutes, useParams, useRoutes } from 'react-router-dom';
import { NotFoundErrorPage } from '@backstage/frontend-plugin-api';
import { EntityTabsPanel } from './EntityTabsPanel';
import { EntityTabsList } from './EntityTabsList';
import { EntityContentGroupDefinitions } from '@backstage/plugin-catalog-react/alpha';
@@ -28,6 +29,22 @@ type SubRoute = {
children: JSX.Element;
};
// Normalize a route path so it can be matched correctly:
// - strip leading slashes
// - if the path already ends with a `*`, keep it as-is so explicit wildcards
// like `/*` or `/foo/*` aren't double-suffixed into `*/*` / `foo/*/*`
// - otherwise strip trailing slashes and append `/*` for nested matching;
// a bare `/` collapses to the empty string so it acts as an index route
// rather than a wildcard that would swallow every sub-path
function normalizeRoutePath(path: string): string {
const withoutLeading = path.replace(/^\/+/, '');
if (withoutLeading.endsWith('*')) {
return withoutLeading;
}
const trimmed = withoutLeading.replace(/\/+$/, '');
return trimmed ? `${trimmed}/*` : '';
}
export function useSelectedSubRoute(subRoutes: SubRoute[]): {
index: number;
route?: SubRoute;
@@ -35,37 +52,32 @@ export function useSelectedSubRoute(subRoutes: SubRoute[]): {
} {
const params = useParams();
const routes = subRoutes.map(({ path, children }) => ({
caseSensitive: false,
path: `${path}/*`,
element: children,
}));
// TODO: remove once react-router updated
const sortedRoutes = routes.sort((a, b) =>
// remove "/*" symbols from path end before comparing
b.path.replace(/\/\*$/, '').localeCompare(a.path.replace(/\/\*$/, '')),
const routes = useMemo(
() =>
subRoutes.map(({ path, children }) => ({
caseSensitive: false,
path: normalizeRoutePath(path),
element: children,
})),
[subRoutes],
);
const element = useRoutes(sortedRoutes) ?? subRoutes[0]?.children;
const element = useRoutes(routes) ?? undefined;
// TODO(Rugvip): Once we only support v6 stable we can always prefix
// This avoids having a double / prefix for react-router v6 beta, which in turn breaks
// the tab highlighting when using relative paths for the tabs.
let currentRoute = params['*'] ?? '';
if (!currentRoute.startsWith('/')) {
currentRoute = `/${currentRoute}`;
}
const [matchedRoute] = matchRoutes(sortedRoutes, currentRoute) ?? [];
const [matchedRoute] = matchRoutes(routes, currentRoute) ?? [];
const foundIndex = matchedRoute
? subRoutes.findIndex(t => `${t.path}/*` === matchedRoute.route.path)
: 0;
? routes.findIndex(r => r.path === matchedRoute.route.path)
: -1;
return {
index: foundIndex === -1 ? 0 : foundIndex,
index: foundIndex,
element,
route: subRoutes[foundIndex] ?? subRoutes[0],
route: subRoutes[foundIndex],
};
}
@@ -83,8 +95,8 @@ export function EntityTabs(props: EntityTabsProps) {
const tabs = useMemo(
() =>
routes.map(t => {
const { path, title, group, icon } = t;
routes.map(r => {
const { path, title, group, icon } = r;
let to = path;
// Remove trailing /*
to = to.replace(/\/\*$/, '');
@@ -112,7 +124,7 @@ export function EntityTabs(props: EntityTabsProps) {
/>
<EntityTabsPanel>
<Helmet title={route?.title} />
{element}
{element ?? <NotFoundErrorPage />}
</EntityTabsPanel>
</>
);
plugins/catalog/src/alpha/components/EntityTabs/EntityTabsList.tsx
+2 -2
View File
@@ -170,7 +170,7 @@ export function EntityTabsList(props: EntityTabsListProps) {
return sorted;
}, [items, groupDefinitions, aliasToGroup, defaultContentOrder]);
const selectedItem = items[selectedIndex];
const selectedItem = selectedIndex >= 0 ? items[selectedIndex] : undefined;
const selectedGroup = resolveGroupId(
selectedItem?.group,
groupDefinitions,
@@ -185,7 +185,7 @@ export function EntityTabsList(props: EntityTabsListProps) {
variant="scrollable"
scrollButtons="auto"
aria-label={t('entityTabs.tabsAriaLabel')}
value={selectedGroup ?? selectedItem?.id}
value={selectedGroup ?? selectedItem?.id ?? false}
>
{groups.map(([id, tabGroup]) => (
<EntityTabsGroup