From 6ca05d4768b744ed05f51eff693f67fc019f7b7c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 18:08:27 +0000 Subject: [PATCH 1/2] fix(nav): the last four producers emit canonical metadata routes, not the deprecated alias (#3660) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The System hub's "Metadata" and "Datasources" cards, and the `sys-datasources` entry in both AppSidebar and UnifiedSidebar, aimed at `component/metadata/{directory,resource?type=datasource}`. app-shell declares those spellings as legacy aliases whose route element is `LegacyMetadataRedirect` — a bare Navigate onto `metadata` and `metadata/datasource`. Each click paid a redundant hop plus a re-render. All four now name the destination directly. Endpoints are byte-identical to what the alias hop computed; only the intermediate hop is gone. The alias routes are untouched and stay reachable for bookmarks and external links. Completes #3639, which fixed the console host's two redirects and enumerated these four as the remainder. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- .../nav-canonical-metadata-routes-3660.md | 12 + .../src/pages/system/SystemHubPage.tsx | 16 +- .../SystemHubPage.metadataCards.test.tsx | 182 ++++++++++++ packages/app-shell/src/layout/AppSidebar.tsx | 11 +- .../app-shell/src/layout/UnifiedSidebar.tsx | 8 +- .../systemNavDatasourcesHop.test.tsx | 261 ++++++++++++++++++ .../systemNavSettingsTarget.test.tsx | 36 ++- 7 files changed, 521 insertions(+), 5 deletions(-) create mode 100644 .changeset/nav-canonical-metadata-routes-3660.md create mode 100644 apps/console/src/pages/system/__tests__/SystemHubPage.metadataCards.test.tsx create mode 100644 packages/app-shell/src/layout/__tests__/systemNavDatasourcesHop.test.tsx diff --git a/.changeset/nav-canonical-metadata-routes-3660.md b/.changeset/nav-canonical-metadata-routes-3660.md new file mode 100644 index 0000000000..ab423eaee3 --- /dev/null +++ b/.changeset/nav-canonical-metadata-routes-3660.md @@ -0,0 +1,12 @@ +--- +'@object-ui/console': patch +'@object-ui/app-shell': patch +--- + +Point the last four navigation producers at the canonical metadata-admin routes instead of the deprecated `component/metadata` alias, removing a redirect hop from each (objectui#3660). + +The System hub's "Metadata" and "Datasources" cards aimed at `…/component/metadata/directory` and `…/component/metadata/resource?type=datasource`, and the `sys-datasources` entry in both `AppSidebar.systemFallbackNavigation` and `UnifiedSidebar.homeNavigation` spelled the latter too. app-shell declares those spellings as legacy *aliases*, not pages: their route element is `LegacyMetadataRedirect`, which immediately navigates on to `…/metadata` and `…/metadata/datasource`. Every click on any of the four therefore paid a redundant hop plus a re-render to reach a destination the navigation could name directly. All four now name it. + +The landing pages are unchanged, byte for byte — the new URLs are exactly what the alias hop was already computing (`datasource` percent-encodes to itself, and neither producer carried a query or hash beyond the `?type=` the alias itself consumed). Only the intermediate hop is gone. + +The alias routes stay declared in both `AppContent` branches, untouched: bookmarks and external links still arrive on them and are still forwarded. This completes objectui#3639, which corrected the console host's two redirects and enumerated these four as the remainder. diff --git a/apps/console/src/pages/system/SystemHubPage.tsx b/apps/console/src/pages/system/SystemHubPage.tsx index b0a16a7c89..320074de01 100644 --- a/apps/console/src/pages/system/SystemHubPage.tsx +++ b/apps/console/src/pages/system/SystemHubPage.tsx @@ -102,6 +102,18 @@ export function SystemHubPage() { // Metadata: single entry point to the server-driven metadata-admin engine. // Per-type cards were removed when the engine started auto-listing every // type registered with the framework (`/api/v1/meta`). + // + // The two metadata cards below name the engine's CANONICAL routes — + // `…/metadata` (directory) and `…/metadata/:type` (one type's list), declared + // by `DefaultAppContent` in `@object-ui/app-shell`. NOT the older + // `…/component/metadata/{directory,resource?type=}` spelling they used to + // carry (objectui#3660): app-shell declares that as a legacy *alias* whose + // route element is `LegacyMetadataRedirect`, i.e. a bare `` onto + // precisely the targets below. Aiming a card at it bought a redundant hop and + // a re-render on every click. The alias routes themselves stay declared — + // bookmarks and external links still land on them — this only stops the hub + // feeding its own traffic through them (same disposition as objectui#3639, + // which corrected the console host's two redirects). const metadataTypeCards: HubCard[] = [ { title: 'Applications', @@ -115,7 +127,7 @@ export function SystemHubPage() { title: 'Metadata', description: 'Browse and edit every metadata type the platform exposes', icon: Database, - href: `${basePath}/component/metadata/directory`, + href: `${basePath}/metadata`, countLabel: '', count: null, }, @@ -123,7 +135,7 @@ export function SystemHubPage() { title: 'Datasources', description: 'Connect external databases and sync their tables in as objects', icon: Boxes, - href: `${basePath}/component/metadata/resource?type=datasource`, + href: `${basePath}/metadata/datasource`, countLabel: '', count: null, }, diff --git a/apps/console/src/pages/system/__tests__/SystemHubPage.metadataCards.test.tsx b/apps/console/src/pages/system/__tests__/SystemHubPage.metadataCards.test.tsx new file mode 100644 index 0000000000..7bfc4e0a30 --- /dev/null +++ b/apps/console/src/pages/system/__tests__/SystemHubPage.metadataCards.test.tsx @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The System hub's two metadata cards land on the CANONICAL metadata-admin + * routes in one hop, never on the deprecated alias (objectui#3660). + * + * ## What was wrong + * + * The "Metadata" card aimed at `…/component/metadata/directory` and the + * "Datasources" card at `…/component/metadata/resource?type=datasource`. + * app-shell declares both spellings as legacy *aliases*, not pages: their route + * element is `LegacyMetadataRedirect`, which immediately ``s onto + * `…/metadata` and `…/metadata/datasource` respectively. So every click on + * either card paid a redundant hop plus a re-render to reach a destination the + * hub could name directly. + * + * These are the third and fourth of the six producers enumerated while fixing + * objectui#3639; that issue's PR corrected the console host's two redirects + * (`ObjectRedirect` / `MetadataRedirect`) but the two cards here were outside + * its declared file surface. The remaining two producers are the `sys-datasources` + * entries in app-shell's two sidebars, pinned by that package's + * `layout/__tests__/systemNavDatasourcesHop.test.tsx`. + * + * ## What these tests measure + * + * `ChainRecorder` records every distinct location the router settles on, so the + * assertion is about the *chain*, not only its endpoint. Endpoint-only + * assertions cannot tell a one-hop click from a two-hop one — both finish at + * `…/metadata/datasource` — and one hop versus two is the whole of this issue. + * The technique is lifted from `__tests__/AppContent.legacyRedirects.test.tsx` + * (objectui#3639), deliberately, so both halves of the same defect are measured + * the same way. + * + * The route table below declares the canonical routes AND the alias routes as + * terminal probes. That is what makes the direction falsifiable: restore either + * card's old href and the alias probe renders, the canonical probe does not, and + * the recorded chain ends on `component/metadata`. + * + * `SystemHubPage` is the real component, mounted at the real `system` path, so + * the hrefs measured are the ones its own render path emits — the card list is + * not transcribed here. A transcribed copy is exactly how the alias spelling + * survived this long. + * + * ## Scope + * + * This measures where the hub AIMS. The alias routes themselves are untouched + * and stay reachable for bookmarks and external links (app-shell's + * `console/__tests__/AppContent.noAppComponentRoutes.test.tsx` drives the real + * alias end to end); nothing here asks for their removal. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Routes, Route, useLocation, useParams } from 'react-router-dom'; + +// The hub's only two data dependencies. `@object-ui/components` stays REAL so +// the cards, their click handlers and their test ids are the ones the hub +// actually renders. +vi.mock('@object-ui/app-shell', () => ({ + useAdapter: () => ({ find: async () => ({ data: [] }) }), +})); +vi.mock('@object-ui/auth', () => ({ + useIsWorkspaceAdmin: () => true, +})); + +import { SystemHubPage } from '../SystemHubPage'; + +/** + * Records every distinct location the router settles on. `` + * re-renders the tree once per hop, so a two-hop chain shows up as three + * entries and a one-hop chain as two. + */ +function ChainRecorder({ sink }: { sink: string[] }) { + const location = useLocation(); + const here = `${location.pathname}${location.search}`; + if (sink[sink.length - 1] !== here) sink.push(here); + return null; +} + +/** Terminal probe: reports which route matched and with which params. */ +function Probe({ id }: { id: string }) { + const params = useParams(); + return
{JSON.stringify(params)}
; +} + +/** + * The destinations both spellings compete for. `metadata/*` mirrors app-shell's + * canonical metadata-admin routes; `component/metadata/*` mirrors the legacy + * aliases it declares alongside them. Both are terminal here — this file asks + * *which one the hub aims at*, not what the alias does afterwards. + */ +function renderHub(): string[] { + const chain: string[] = []; + render( + + + + + } /> + } /> + } /> + } /> + } /> + } /> + + } /> + + , + ); + return chain; +} + +/** No alias route may appear anywhere in the chain, not merely at its end. */ +function expectNoAliasAnywhere(chain: string[]) { + expect(chain.some((entry) => entry.includes('component/metadata'))).toBe(false); + expect(screen.queryByTestId('alias-resource')).not.toBeInTheDocument(); + expect(screen.queryByTestId('alias-directory')).not.toBeInTheDocument(); +} + +/** + * Click a hub card and let the counts effect settle first. `fetchCounts` runs + * from a `useEffect` and setStates; clicking before it resolves produces act() + * noise unrelated to anything measured here. + */ +async function clickCard(testId: string) { + const card = await screen.findByTestId(testId); + await waitFor(() => expect(screen.queryByRole('status')).not.toBeInTheDocument()); + await userEvent.click(card); +} + +describe('System hub metadata cards → canonical metadata routes (objectui#3660)', () => { + it('the "Metadata" card reaches the metadata directory in ONE hop', async () => { + const chain = renderHub(); + + await clickCard('hub-card-metadata'); + + // The whole point: two entries, i.e. a single navigation. Before the fix + // this ended on `component/metadata/directory`, whose route element is a + // second `` onto exactly the URL asserted here. + expect(chain).toEqual(['/apps/setup/system', '/apps/setup/metadata']); + expect(screen.getByTestId('canonical-directory')).toBeInTheDocument(); + expectNoAliasAnywhere(chain); + }); + + it('the "Datasources" card reaches metadata/datasource in ONE hop', async () => { + const chain = renderHub(); + + await clickCard('hub-card-datasources'); + + expect(chain).toEqual(['/apps/setup/system', '/apps/setup/metadata/datasource']); + expect(screen.getByTestId('canonical-list')).toHaveTextContent('"type":"datasource"'); + expectNoAliasAnywhere(chain); + }); + + it('MEASUREMENT: both endpoints are byte-identical to what the alias hop produced', async () => { + // Equivalence, not improvement. `LegacyMetadataRedirect`'s directory arm + // builds `${appBase}/metadata` + search + hash, and its resource arm builds + // `${appBase}/metadata/${encodeURIComponent(type)}` + path tail + hash. + // Neither card carried a query or a hash beyond the `?type=` the alias + // itself consumed, and `datasource` percent-encodes to itself, so the old + // two-hop chain landed on precisely these two URLs. Pinned so a future + // reader can see the equality was measured rather than assumed. + const chain = renderHub(); + + await clickCard('hub-card-datasources'); + + expect(chain[chain.length - 1]).toBe('/apps/setup/metadata/datasource'); + expect(chain).toHaveLength(2); + }); + + it('CONTROL: the sibling "Applications" card is unchanged and still hub-scoped', async () => { + // Without this the suite would also pass on a hub that had lost its cards + // entirely, or on one where every href had been rewritten to `/metadata`. + const chain = renderHub(); + + await clickCard('hub-card-applications'); + + expect(chain).toEqual(['/apps/setup/system', '/apps/setup/system/apps']); + }); +}); diff --git a/packages/app-shell/src/layout/AppSidebar.tsx b/packages/app-shell/src/layout/AppSidebar.tsx index 926dffe9bc..f0e5d76743 100644 --- a/packages/app-shell/src/layout/AppSidebar.tsx +++ b/packages/app-shell/src/layout/AppSidebar.tsx @@ -327,7 +327,16 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri } items.push( { id: 'sys-objects', label: t('layout.systemNav.objectManager', { defaultValue: 'Object Manager' }), type: 'url' as const, url: '/apps/setup/system/metadata/object', icon: 'database' }, - { id: 'sys-datasources', label: t('layout.systemNav.datasources', { defaultValue: 'Datasources' }), type: 'url' as const, url: '/apps/setup/component/metadata/resource?type=datasource', icon: 'database' }, + // #3660 — `sys-datasources` names the metadata-admin engine's CANONICAL + // route `/apps/setup/metadata/datasource`, not the legacy + // `…/component/metadata/resource?type=datasource` alias it used to carry. + // That alias is not a page: its route element is `LegacyMetadataRedirect`, + // which ``s onto exactly the URL spelled here, so every click + // paid a redundant hop plus a re-render. The alias route stays declared in + // BOTH `AppContent` branches (bookmarks and external links still arrive on + // it, and #3610 added it to the zero-app branch precisely because this + // entry fed it) — we simply stop aiming our own navigation at it. + { id: 'sys-datasources', label: t('layout.systemNav.datasources', { defaultValue: 'Datasources' }), type: 'url' as const, url: '/apps/setup/metadata/datasource', icon: 'database' }, { id: 'sys-users', label: t('layout.systemNav.users', { defaultValue: 'Users' }), type: 'url' as const, url: '/apps/setup/system/users', icon: 'users' }, { id: 'sys-orgs', label: t('layout.systemNav.organizations', { defaultValue: 'Organizations' }), type: 'url' as const, url: '/apps/setup/system/organizations', icon: 'building-2' }, { id: 'sys-roles', label: t('layout.systemNav.roles', { defaultValue: 'Roles' }), type: 'url' as const, url: '/apps/setup/system/roles', icon: 'shield' }, diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index e676593ee7..9da813e8ef 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -333,7 +333,13 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { { id: 'sys-apps', label: t('layout.systemNav.applications', { defaultValue: 'Applications' }), type: 'url' as const, url: '/apps/setup/system/apps', icon: 'layout-grid' }, { id: 'sys-marketplace', label: t('layout.systemNav.appMarketplace', { defaultValue: 'App Marketplace' }), type: 'url' as const, url: '/apps/setup/system/marketplace', icon: 'store' }, { id: 'sys-objects', label: t('layout.systemNav.objectManager', { defaultValue: 'Object Manager' }), type: 'url' as const, url: '/apps/setup/system/metadata/object', icon: 'database' }, - { id: 'sys-datasources', label: t('layout.systemNav.datasources', { defaultValue: 'Datasources' }), type: 'url' as const, url: '/apps/setup/component/metadata/resource?type=datasource', icon: 'database' }, + // #3660 — canonical `…/metadata/datasource`, not the legacy + // `…/component/metadata/resource?type=datasource` alias. See the twin + // entry in `AppSidebar.systemFallbackNavigation` for the full note: the + // alias renders `LegacyMetadataRedirect`, a bare `` onto this + // very URL, so pointing here removes a hop without moving the landing + // page. The alias route itself is untouched. + { id: 'sys-datasources', label: t('layout.systemNav.datasources', { defaultValue: 'Datasources' }), type: 'url' as const, url: '/apps/setup/metadata/datasource', icon: 'database' }, { id: 'sys-users', label: t('layout.systemNav.users', { defaultValue: 'Users' }), type: 'url' as const, url: '/apps/setup/system/users', icon: 'users' }, { id: 'sys-orgs', label: t('layout.systemNav.organizations', { defaultValue: 'Organizations' }), type: 'url' as const, url: '/apps/setup/system/organizations', icon: 'building-2' }, { id: 'sys-roles', label: t('layout.systemNav.roles', { defaultValue: 'Roles' }), type: 'url' as const, url: '/apps/setup/system/roles', icon: 'shield' }, diff --git a/packages/app-shell/src/layout/__tests__/systemNavDatasourcesHop.test.tsx b/packages/app-shell/src/layout/__tests__/systemNavDatasourcesHop.test.tsx new file mode 100644 index 0000000000..cce754a47f --- /dev/null +++ b/packages/app-shell/src/layout/__tests__/systemNavDatasourcesHop.test.tsx @@ -0,0 +1,261 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `sys-datasources` entry both sidebars carry arrives at the canonical + * metadata-admin route with ZERO redirects (objectui#3660). + * + * ## What was wrong + * + * `AppSidebar.systemFallbackNavigation` and `UnifiedSidebar.homeNavigation` + * each hold their own literal for this entry, and both spelled it + * `/apps/setup/component/metadata/resource?type=datasource`. app-shell declares + * that spelling as a legacy *alias*, not a page: in both `AppContent` branches + * its route element is `LegacyMetadataRedirect`, which immediately ``s + * onto `/apps/setup/metadata/datasource`. Every click therefore paid a hop plus + * a re-render to reach a URL the nav item could name itself. + * + * These are two of the six producers enumerated while fixing objectui#3639. + * That issue's PR corrected the console host's two redirects; the System hub's + * two metadata cards are the other pair, measured in `apps/console`'s + * `pages/system/__tests__/SystemHubPage.metadataCards.test.tsx`. + * + * ## What this file measures, and how it differs from the sibling pin + * + * `systemNavSettingsTarget.test.tsx` asserts the URL each entry CARRIES — a + * string equality on an `href`. This file asks the next question: what that URL + * costs to resolve. `ChainRecorder` records every distinct location the router + * settles on, so a direct arrival is a one-entry chain and an alias arrival is + * two. Endpoint-only assertions cannot tell those apart — both finish at + * `/apps/setup/metadata/datasource`, which is exactly why the detour survived. + * + * The href is READ OUT of a real sidebar render rather than typed in here, so + * the producer under test is the component's own literal. Both sidebars are + * driven, because both hold a copy. + * + * ## The alias mirror below, and what rests on its fidelity + * + * `AliasResourceRedirect` mirrors `LegacyMetadataRedirect`'s resource arm from + * `console/AppContent.tsx`. Nothing this file ASSERTS depends on that mirror + * being faithful: the green expectation is `chain` equals `[canonical URL]`, + * which holds iff the sidebar's URL matches a canonical route directly. The + * mirror only shapes what the FAILURE looks like — with it, restoring either + * literal produces a two-entry chain whose first entry is the alias, i.e. the + * extra hop printed literally in the diff rather than merely implied. The real + * alias route's own behaviour is pinned separately, against the real route + * table, in `console/__tests__/AppContent.noAppComponentRoutes.test.tsx`. + * + * ## Scope + * + * Where the sidebars AIM. The alias routes are untouched and stay reachable for + * bookmarks and external links; nothing here asks for their removal. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Routes, Route, Navigate, useLocation, useParams } from 'react-router-dom'; + +// --------------------------------------------------------------------------- +// Mocks — providers and console-only chrome, matching the sibling sidebar +// suites (`systemNavSettingsTarget.test.tsx`, `appSidebarSettingsTargets.test.tsx`). +// `react-router-dom` stays REAL: the chain measurement below IS the router. +// --------------------------------------------------------------------------- + +vi.mock('@object-ui/i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useObjectTranslation: () => ({ + t: (key: string, options?: Record) => String(options?.defaultValue ?? key), + }), + useObjectLabel: () => ({ + objectLabel: ({ label }: { label?: string }) => label, + viewLabel: (_o: string, _v: string, fallback?: string) => fallback, + dashboardLabel: ({ label }: { label?: string }) => label, + navGroupLabel: (_a: string, _g: string, fallback?: string) => fallback, + }), +})); + +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: null, signOut: vi.fn(), isAuthEnabled: false, activeOrganization: null }), + useIsWorkspaceAdmin: () => true, + getUserInitials: () => 'U', +})); + +vi.mock('@object-ui/permissions', () => ({ + usePermissions: () => ({ can: () => true, hasCapabilities: () => true }), +})); + +/** The zero-app deployment `systemFallbackNavigation` exists for. */ +vi.mock('../../providers/MetadataProvider', () => ({ + useMetadata: () => ({ apps: [], objects: [] }), +})); + +vi.mock('../../providers/ExpressionProvider', () => ({ + useExpressionContext: () => ({ evaluator: null }), + evaluateVisibility: (expr: unknown) => expr !== false && expr !== 'false', +})); + +vi.mock('../../utils', () => ({ + resolveI18nLabel: (label: unknown) => (typeof label === 'string' ? label : ''), + matchAppBySegment: (apps: Array<{ name?: string }>, segment?: string) => + apps.find((a) => a?.name === segment), + appRouteSegment: (app: { name?: string }) => app?.name, +})); + +vi.mock('../../utils/getIcon', () => ({ getIcon: () => () => null })); +vi.mock('@object-ui/components', async (importOriginal) => ({ + ...(await importOriginal>()), + getLazyIcon: () => () => null, +})); + +vi.mock('../../hooks/useRecentItems', () => ({ useRecentItems: () => ({ recentItems: [] }) })); +vi.mock('../../hooks/useFavorites', () => ({ + useFavorites: () => ({ favorites: [], removeFavorite: vi.fn() }), +})); +vi.mock('../../hooks/useNavPins', () => ({ + useNavPins: () => ({ togglePin: vi.fn(), applyPins: (items: unknown) => items }), +})); +vi.mock('../../hooks/useNavActionDispatch', () => ({ + useNavActionDispatch: () => vi.fn(), +})); +vi.mock('../../context/NavigationContext', () => ({ + useNavigationContext: () => ({ context: 'home', currentAppName: undefined }), +})); +vi.mock('../ContextSelectors', () => ({ + useAppContextSelectors: () => ({ contextValues: {}, element: null }), + contextSelectorQueryKey: (id: string) => (id === 'active_package' ? 'package' : id), + STUDIO_PACKAGE_SELECTOR_ID: 'active_package', +})); +vi.mock('../LocalizedSidebarTrigger', () => ({ + LocalizedSidebarTrigger: () => null, +})); + +import { SidebarProvider } from '@object-ui/components'; +import { AppSidebar } from '../AppSidebar'; +import { UnifiedSidebar } from '../UnifiedSidebar'; + +/** Where the metadata-admin engine really serves the datasource list. */ +const CANONICAL = '/apps/setup/metadata/datasource'; + +/** + * Records every distinct location the router settles on: one entry means the + * URL matched a real route on arrival, two means it was forwarded once. + */ +function ChainRecorder({ sink }: { sink: string[] }) { + const location = useLocation(); + const here = `${location.pathname}${location.search}`; + if (sink[sink.length - 1] !== here) sink.push(here); + return null; +} + +/** Terminal probe: reports which route matched and with which params. */ +function Probe({ id }: { id: string }) { + const params = useParams(); + return
{JSON.stringify(params)}
; +} + +/** + * Mirror of `LegacyMetadataRedirect`'s resource arm (`console/AppContent.tsx`). + * Present so a restored alias literal shows up as a real extra hop rather than + * as a dead end — see the note in this file's header on what does and does not + * rest on its fidelity. + */ +function AliasResourceRedirect() { + const location = useLocation(); + const appBase = location.pathname.replace(/\/component\/metadata\/.*$/, ''); + const type = new URLSearchParams(location.search).get('type') ?? ''; + const tail = location.pathname.match(/\/component\/metadata\/resource(\/.*)?$/)?.[1] ?? ''; + return ( + + ); +} + +/** Renders a sidebar, reads the entry's href, then unmounts it. */ +function datasourcesHrefFrom(ui: React.ReactElement, at: string): string { + const view = render( + + {ui} + , + ); + const href = screen.getByRole('link', { name: 'Datasources' }).getAttribute('href'); + view.unmount(); + expect(href).toBeTruthy(); + return href as string; +} + +/** Drops the emitted URL into a router that knows both spellings. */ +function chainFor(url: string): string[] { + const chain: string[] = []; + render( + + + + + } /> + } /> + } /> + } /> + } /> + + } /> + + , + ); + return chain; +} + +beforeEach(() => { + localStorage.clear(); +}); + +describe('sidebar sys-datasources reaches the canonical route directly (objectui#3660)', () => { + it('AppSidebar: the zero-app fallback cluster arrives with NO redirect', () => { + const href = datasourcesHrefFrom( + {}} />, + '/apps/setup', + ); + + // Precondition: with zero apps this really is the fallback cluster, so the + // href just read is the one `systemFallbackNavigation` declares. + expect(href).toBe(CANONICAL); + + const chain = chainFor(href); + + // One entry = matched on arrival. Before the fix this was two, the alias + // first. + expect(chain).toEqual([CANONICAL]); + expect(screen.getByTestId('canonical-list')).toHaveTextContent('"type":"datasource"'); + expect(chain.some((entry) => entry.includes('component/metadata'))).toBe(false); + }); + + it('UnifiedSidebar: the /home Administration cluster arrives with NO redirect', () => { + const href = datasourcesHrefFrom(, '/home'); + + expect(href).toBe(CANONICAL); + + const chain = chainFor(href); + + expect(chain).toEqual([CANONICAL]); + expect(screen.getByTestId('canonical-list')).toHaveTextContent('"type":"datasource"'); + expect(chain.some((entry) => entry.includes('component/metadata'))).toBe(false); + }); + + it('CONTROL: the alias still resolves, and doing so costs the hop the entries used to pay', () => { + // The alias is deliberately KEPT (bookmarks, external links), so its + // continued reachability is part of the contract, not collateral. This also + // proves the two assertions above are not vacuous: the probe table really + // does forward this spelling, so a sidebar that still emitted it would be + // measured at two entries rather than silently falling to `unmatched`. + const chain = chainFor('/apps/setup/component/metadata/resource?type=datasource'); + + expect(chain).toEqual(['/apps/setup/component/metadata/resource?type=datasource', CANONICAL]); + expect(screen.getByTestId('canonical-list')).toHaveTextContent('"type":"datasource"'); + }); +}); diff --git a/packages/app-shell/src/layout/__tests__/systemNavSettingsTarget.test.tsx b/packages/app-shell/src/layout/__tests__/systemNavSettingsTarget.test.tsx index b719725dac..dc132a897f 100644 --- a/packages/app-shell/src/layout/__tests__/systemNavSettingsTarget.test.tsx +++ b/packages/app-shell/src/layout/__tests__/systemNavSettingsTarget.test.tsx @@ -45,6 +45,17 @@ * `NavigationRenderer` the app arm uses. The pin is therefore GONE, not * duplicated: it is replaced below by the real assertions it was standing in * for, so the repo pins the fix instead of pinning both the bug and the fix. + * + * ## The `Datasources` entry (objectui#3660) + * + * `ADMINISTRATION_ENTRIES` used to spell that entry + * `/apps/setup/component/metadata/resource?type=datasource` — a legacy *alias*, + * not a page, whose route element ``s straight on to + * `/apps/setup/metadata/datasource`. Both sidebars now name that destination + * directly, and this file's expectation is REWRITTEN to match (same discipline + * as above: replace the old shape, do not keep it alongside). The AppSidebar + * test below gained the matching assertion at the same time — that sidebar + * carries its own copy of the literal and had none. */ import '@testing-library/jest-dom/vitest'; @@ -148,6 +159,19 @@ import { UnifiedSidebar } from '../UnifiedSidebar'; /** The system hub — the reachable target, and what every sibling entry prefixes. */ const SYSTEM_HUB = '/apps/setup/system'; +/** + * `Datasources` names the metadata-admin engine's canonical route + * (objectui#3660). This entry used to read + * `/apps/setup/component/metadata/resource?type=datasource` — the legacy alias, + * whose route element is `LegacyMetadataRedirect`, a bare `` onto the + * URL below. That old spelling is REPLACED here, not kept alongside: the repo + * pins the fix, never both the bug and the fix (the #3609 discipline this file + * already applied once, when the #3590 measurement pin was deleted rather than + * duplicated). Whether this URL then resolves in one hop is a different + * question, measured in `systemNavDatasourcesHop.test.tsx`. + */ +const DATASOURCES_TARGET = '/apps/setup/metadata/datasource'; + /** * Every entry of the `/home` Administration cluster, in declaration order. * Asserted whole rather than by sample: the defect was that the group's @@ -159,7 +183,7 @@ const ADMINISTRATION_ENTRIES: ReadonlyArray = [ ['Applications', `${SYSTEM_HUB}/apps`], ['App Marketplace', `${SYSTEM_HUB}/marketplace`], ['Object Manager', `${SYSTEM_HUB}/metadata/object`], - ['Datasources', '/apps/setup/component/metadata/resource?type=datasource'], + ['Datasources', DATASOURCES_TARGET], ['Users', `${SYSTEM_HUB}/users`], ['Organizations', `${SYSTEM_HUB}/organizations`], ['Roles', `${SYSTEM_HUB}/roles`], @@ -211,6 +235,16 @@ describe('sidebar system-settings target (objectui#3590)', () => { 'href', `${SYSTEM_HUB}/apps`, ); + + // objectui#3660 — `sys-datasources`. AppSidebar and UnifiedSidebar each hold + // their OWN literal for this entry, and only UnifiedSidebar's was pinned + // (in ADMINISTRATION_ENTRIES); this half of the twin could drift back to the + // alias with every test in the repo still green. Pinned here now, in the + // same shape, so both copies are covered. + expect(screen.getByRole('link', { name: 'Datasources' })).toHaveAttribute( + 'href', + DATASOURCES_TARGET, + ); }); it('UnifiedSidebar: /home renders the Administration cluster as a GROUP, with all nine entries reachable (objectui#3609)', () => { From bd04651ee62878bb809a376442feeafa8fb466b3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 18:25:55 +0000 Subject: [PATCH 2/2] docs(app-shell): rewrite three docblocks this branch falsified (#3666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two sidebars' `sys-datasources` entry now names the canonical `…/metadata/datasource` route, so three comments claiming the setup left-nav points at the legacy alias became false as of the previous commit. Rewritten as positive statements of what is true now. Per #3656, none of them re-plants the alias URL in a denial sentence — a comment that says "no longer points at X" keeps a grep for X returning the very hit the rewrite was meant to clear. `console/AppContent.tsx` is a comment-only change: the two alias route declarations, and every other line of code, are untouched. Its #3610 history is preserved and extended rather than replaced — #3610 declined to re-point the navigation because a zero-app-only spelling would have given the alias a second canonical destination, and #3660 re-pointed it at the shared route, so that reasoning still holds. Fixes #3666 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt --- packages/app-shell/src/console/AppContent.tsx | 23 +++++++++++-------- .../datasource/DatasourceResourcePage.tsx | 11 +++++---- .../metadata-admin/datasource/register.ts | 9 ++++---- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index d8ac6894f0..8a958ada0c 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -646,16 +646,19 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = branch below. They are NOT a second copy of the page: both render `LegacyMetadataRedirect`, which forwards onto the canonical `metadata/:type…` routes declared just above. Declaring them here - is what makes the zero-app console's own fallback navigation work: - `sys-datasources` points straight at - `…/component/metadata/resource?type=datasource`, and `sys-objects` - arrives via the host's `system/metadata/:type` → same alias - rewrite. Both pass `isMetadataRoute` (a `metadata` path segment — - a substring test until #3638) and so land in THIS branch, which declared no - `component/…` route at all — every one of them rendered a blank - screen. Kept as a mirror rather than re-pointed navigation because - the alias already has exactly one canonical destination; adding a - zero-app-only spelling would create a second. */} + is what stopped the zero-app console rendering a blank screen: the + fallback navigation then aimed `sys-datasources` at an alias, and + carried `sys-objects` onto one via the host's + `system/metadata/:type` rewrite. Both pass `isMetadataRoute` (a + `metadata` path segment — a substring test until #3638) and so land + in THIS branch, which declared no `component/…` route at all — + every one of them rendered a blank screen. #3610 mirrored the + routes rather than re-point that navigation, because inventing a + zero-app-only spelling would have given the alias a second + canonical destination. #3660 re-pointed it anyway — at the shared + `metadata/:type` routes above, so no second spelling was created — + which leaves these two serving bookmarks and external links, the + arrivals that can never be re-pointed. */} } /> } /> {extraRoutesNoApp} diff --git a/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx b/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx index 216c929431..42ef371d99 100644 --- a/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx +++ b/packages/app-shell/src/views/metadata-admin/datasource/DatasourceResourcePage.tsx @@ -5,11 +5,12 @@ * registered into the metadata-admin engine (`registerMetadataResource`) and * reached via the engine route `…/metadata/datasource`. * - * Two legacy spellings still arrive here, both as redirects onto that engine - * route rather than routes of their own (objectui#3639): the setup left-nav - * "Datasources" item points at `…/component/metadata/resource?type=datasource` - * (rewritten by `LegacyMetadataRedirect`), and the console host forwards - * `…/system/metadata/datasource` (rewritten by its own `MetadataRedirect`). + * The setup left-nav "Datasources" item names that engine route directly + * (objectui#3660). Two legacy spellings still arrive here as well, both as + * redirects onto the same engine route rather than routes of their own: the + * console host forwards `…/system/metadata/datasource` (rewritten by its own + * `MetadataRedirect`), and the older aliases `AppContent` declares for + * bookmarks and external links are rewritten by `LegacyMetadataRedirect`. * * datasource is a *side-effectful* metadata type: its records are managed by * the framework `datasource-admin` service (secret encryption + connection-pool diff --git a/packages/app-shell/src/views/metadata-admin/datasource/register.ts b/packages/app-shell/src/views/metadata-admin/datasource/register.ts index 3f3a1a18bc..3ceb0ee2d9 100644 --- a/packages/app-shell/src/views/metadata-admin/datasource/register.ts +++ b/packages/app-shell/src/views/metadata-admin/datasource/register.ts @@ -8,10 +8,11 @@ * (engine route + registry slot + shell), reachable at the engine route * `…/metadata/datasource`, instead of a separate hand-written System page. * - * The setup left-nav "Datasources" item still points at the older - * `…/component/metadata/resource?type=datasource` spelling. That is a legacy - * *alias*, not this engine route: it renders `LegacyMetadataRedirect`, which - * 302s onto `…/metadata/datasource` (objectui#3639). + * The setup left-nav "Datasources" item names that engine route directly + * (objectui#3660). The older alias spellings `AppContent` still declares are + * redirects onto it rather than routes of their own — kept so bookmarks and + * external links keep resolving — but no navigation in this repo is routed + * through them any more. */ import { registerMetadataResource } from '../registry';