diff --git a/.changeset/entity-tab-not-found.md b/.changeset/entity-tab-not-found.md new file mode 100644 index 0000000000..074c0a9e42 --- /dev/null +++ b/.changeset/entity-tab-not-found.md @@ -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. diff --git a/packages/core-compat-api/src/convertLegacyApp.test.tsx b/packages/core-compat-api/src/convertLegacyApp.test.tsx index 7094eb798c..9c6728321d 100644 --- a/packages/core-compat-api/src/convertLegacyApp.test.tsx +++ b/packages/core-compat-api/src/convertLegacyApp.test.tsx @@ -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(); }); }); diff --git a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.test.tsx b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.test.tsx index a85d795cd8..3145006fa8 100644 --- a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.test.tsx +++ b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.test.tsx @@ -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( @@ -151,10 +151,74 @@ describe('EntityTabs', () => { , ); + 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:
Overview Content
, + }, + { + group: 'default', + path: '/details', + title: 'Details', + children:
Details Content
, + }, + ]; + + render( + + + } + /> + + , + ); + 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:
Overview Content
, + }, + { + group: 'default', + path: '/details', + title: 'Details', + children:
Details Content
, + }, + ]; + + render( + + + } + /> + + , + ); + + 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( @@ -288,10 +352,135 @@ describe('EntityTabs', () => { , ); - 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:
Overview Content
, + }, + { + group: 'default', + path: '/docs/*', + title: 'Docs', + children:
Docs Content
, + }, + ]; + + render( + + + } + /> + + , ); + + 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:
Overview Content
, + }, + { + group: 'overview', + path: '/details', + title: 'Details', + children:
Details Content
, + }, + ]; + + it('renders the matched route content and no not-found page', async () => { + await renderInTestApp( + + + } + /> + , + { 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( + + + } + /> + , + { 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:
Overview Content
, + }, + { + group: 'overview', + path: '/docs', + title: 'Docs', + children:
Docs Content
, + }, + ]; + + await renderInTestApp( + + + } + /> + , + { initialRouteEntries: ['/docs/api/v1'] }, + ); + + expect(await screen.findByText('Docs Content')).toBeInTheDocument(); + expect(screen.queryByTestId('error')).toBeNull(); }); }); }); diff --git a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.tsx b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.tsx index 102df76042..f9deb0dec8 100644 --- a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.tsx +++ b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabs.tsx @@ -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) { /> - {element} + {element ?? } ); diff --git a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabsList.tsx b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabsList.tsx index 9abbfb69d9..4885fac1d7 100644 --- a/plugins/catalog/src/alpha/components/EntityTabs/EntityTabsList.tsx +++ b/plugins/catalog/src/alpha/components/EntityTabs/EntityTabsList.tsx @@ -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]) => (