Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/no-apps-go-to-settings-target-3590.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@object-ui/app-shell': patch
---

Point the "System Settings" entries at the system hub `/apps/setup/system` instead of the bare `/apps/setup` (objectui#3590).

`AppContent` mounts the system hub only under `isSystemRoute`, which keys on a `/system` path segment. A bare `/apps/setup` therefore matched no pseudo-route except `isSetupRoute` and fell back into the "No Apps Configured" guard — i.e. on a zero-app deployment it *is* that empty state's own URL, so the empty state's `go-to-settings-btn` re-rendered the very screen it sits on. Retargeted three call sites: the empty state's CTA, `AppSidebar`'s no-active-app `sys-settings` fallback entry, and `UnifiedSidebar`'s `/home` Administration `sys-settings` entry. Every sibling entry in both clusters already spelled `/apps/setup/system/...`.
14 changes: 13 additions & 1 deletion packages/app-shell/src/console/AppContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,19 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
<Button onClick={() => navigate(`/apps/${appName}/create-app`)} data-testid="create-first-app-btn">
{t('empty.createFirstApp')}
</Button>
<Button variant="outline" onClick={() => navigate('/apps/setup')} data-testid="go-to-settings-btn">
{/* #3590 — the system hub lives at `/apps/setup/system`, NOT at the
bare `/apps/setup`. Being inside the `/apps/:appName/*` subtree is
necessary but NOT sufficient: `isSystemRoute` keys on a `/system`
segment, so bare `/apps/setup` fails every pseudo-route test above
except `isSetupRoute` and falls straight back into THIS guard —
i.e. it is this empty state's own URL, and the click was a no-op
loop. `/apps/setup/system` flips `isSystemRoute`, which is exactly
the switch that mounts `extraRoutesNoApp` (the branch just below)
where the host declares `system` → SystemHubPage. Spelled the same
absolute way as the sidebar's whole `sys-*` cluster
(`/apps/setup/system/...`); `appName` is provably `setup` here, so
the two forms coincide. */}
<Button variant="outline" onClick={() => navigate('/apps/setup/system')} data-testid="go-to-settings-btn">
{t('empty.systemSettings')}
</Button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
* /apps/setup and /apps/setup/<anything not
* system|metadata|create-app>
*
* which is precisely where the sidebar's no-active-app system navigation and
* the empty state's own `go-to-settings-btn` send a zero-app user
* (`layout/AppSidebar.tsx` systemFallbackNavigation → `/apps/setup`).
* which is where a zero-app user still arrives from the remaining bare
* `/apps/setup` senders (`layout/AppSidebar.tsx`'s no-app sidebar header and
* user-menu "Settings", `console/ConsoleShell.tsx`'s legacy `/system` redirect)
* and from bookmarks. Note both CTA-shaped senders that used to point here have
* since been retargeted at the system hub: the empty state's own
* `go-to-settings-btn` and both sidebars' `sys-settings` entry (objectui#3590) —
* the ENTRY family below is unchanged, only who points at it.
*
* `/` is NOT such a URL: with zero apps `RootLandingRedirect` resolves to
* `/home` and `AppContent` never mounts. So the empty state always renders
Expand Down Expand Up @@ -138,6 +142,19 @@ function LocationProbe() {
return <div data-testid="pathname">{location.pathname}</div>;
}

/**
* The host's system routes, reduced to the one entry this file asserts on.
* `apps/console/src/AppContent.tsx` builds a `systemRoutes` fragment whose FIRST
* entry is `<Route path="system" element={<SystemHubPage />} />`, and passes the
* SAME fragment to both `extraRoutes` and `extraRoutesNoApp`. With zero apps only
* the `extraRoutesNoApp` branch is reachable, so that is the one wired below —
* and it is what makes `/apps/setup/system` a real destination rather than
* another URL that renders nothing (objectui#3590).
*/
const systemRoutesStub = (
<Route path="system" element={<div data-testid="system-hub-page">system hub</div>} />
);

/**
* The reference host's route tree, reduced to the parts that decide this
* question: the `/apps/:appName/*` subtree, the landing route, and the
Expand All @@ -149,7 +166,7 @@ function renderConsoleAt(initialUrl: string) {
<MemoryRouter initialEntries={[initialUrl]}>
<LocationProbe />
<Routes>
<Route path="/apps/:appName/*" element={<AppContent />} />
<Route path="/apps/:appName/*" element={<AppContent extraRoutesNoApp={systemRoutesStub} />} />
<Route path="/" element={<div data-testid="root-landing">landing</div>} />
<Route path="/home" element={<div data-testid="home-launcher">home</div>} />
<Route path="*" element={<Navigate to="/" replace />} />
Expand Down Expand Up @@ -199,18 +216,38 @@ describe('AppContent — no-apps empty state CTA (objectui#3573)', () => {
expect(screen.queryByTestId('root-landing')).not.toBeInTheDocument();
});

it('leaves the sibling go-to-settings CTA on its absolute /apps/setup target', async () => {
renderConsoleAt('/apps/setup/sys_inbox_message');
it('sibling go-to-settings CTA opens the system hub instead of looping onto this same empty state', async () => {
// objectui#3590 — this REPLACES the pin that used to sit here (*"leaves the
// sibling go-to-settings CTA on its absolute /apps/setup target"*: pathname
// `/apps/setup`, `create-first-app-btn` still present). That pin recorded
// current behaviour to prove #3573 had not touched this button; it explicitly
// did not bless the target. The target was wrong: bare `/apps/setup` is this
// empty state's OWN url, so the click re-rendered the same screen.
renderConsoleAt('/apps/setup');
fireEvent.click(await screen.findByTestId('go-to-settings-btn'));

expect(pathname()).toBe('/apps/setup');
expect(await screen.findByTestId('system-hub-page')).toBeInTheDocument();
expect(pathname()).toBe('/apps/setup/system');
// The loop this pins: the empty state must be GONE. Asserting only the URL
// would stay green for a target that merely renders nothing, and asserting
// only the hub would miss a screen that rendered both.
expect(screen.queryByTestId('create-first-app-btn')).not.toBeInTheDocument();
expect(screen.queryByTestId('root-landing')).not.toBeInTheDocument();
// NB: this asserts CURRENT behaviour, it does not bless it. `/apps/setup`
// bare IS this empty state's own URL (`isSystemRoute` needs a `/system`
// segment), so on a zero-app deployment this sibling CTA is a no-op loop —
// filed separately as #3590. Kept here only to prove the #3573 fix did not
// touch it; update this expectation together with #3590.
expect(await screen.findByTestId('create-first-app-btn')).toBeInTheDocument();
});

it('reaches the SAME system hub from a deeper splat URL (absolute target, depth-independent)', async () => {
// The depth the replaced pin used. The `/system` segment is what flips
// `isSystemRoute`, i.e. the switch that mounts `extraRoutesNoApp` — so the
// splat segment must not leak into the target either (a relative `system`
// would build `/apps/setup/sys_inbox_message/system` here: still
// `isSystemRoute`, but matching no route inside that branch — which has no
// catch-all — and therefore rendering blank).
renderConsoleAt('/apps/setup/sys_inbox_message');
fireEvent.click(await screen.findByTestId('go-to-settings-btn'));

expect(await screen.findByTestId('system-hub-page')).toBeInTheDocument();
expect(pathname()).toBe('/apps/setup/system');
expect(screen.queryByTestId('create-first-app-btn')).not.toBeInTheDocument();
});

it('MEASUREMENT: a non-pseudo /apps/:appName URL never reaches this empty state', async () => {
Expand Down
11 changes: 10 additions & 1 deletion packages/app-shell/src/layout/AppSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,18 @@ export function AppSidebar({ activeAppName, onAppChange }: { activeAppName: stri
// Fallback system navigation when no active app exists — routes into the Setup app.
// The marketplace entry is hidden from non-admin members (install is gated to
// owner/admin on the server, so non-admins have no reason to see it).
//
// #3590 — `sys-settings` targets `/apps/setup/system` (the system hub), not the
// bare `/apps/setup`. This whole cluster renders ONLY when `activeApp` is falsy,
// and `activeApp` (above) is `matched || activeApps[0]` — falsy only when the
// deployment has zero active+visible apps. In exactly that case bare
// `/apps/setup` renders `AppContent`'s "No Apps Configured" empty state (its
// `isSystemRoute` guard needs a `/system` segment), so the cluster's head entry
// was a dead link in the one situation the cluster exists for. Every sibling
// below already spells `/apps/setup/system/...`.
const systemFallbackNavigation: NavigationItem[] = React.useMemo(() => {
const items: NavigationItem[] = [
{ id: 'sys-settings', label: t('layout.systemNav.systemSettings', { defaultValue: 'System Settings' }), type: 'url' as const, url: '/apps/setup', icon: 'settings' },
{ id: 'sys-settings', label: t('layout.systemNav.systemSettings', { defaultValue: 'System Settings' }), type: 'url' as const, url: '/apps/setup/system', icon: 'settings' },
{ id: 'sys-apps', label: t('layout.systemNav.applications', { defaultValue: 'Applications' }), type: 'url' as const, url: '/apps/setup/system/apps', icon: 'layout-grid' },
];
if (isWorkspaceAdmin) {
Expand Down
12 changes: 11 additions & 1 deletion packages/app-shell/src/layout/UnifiedSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,18 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) {
{ id: 'docs', label: t('layout.systemNav.documentation', { defaultValue: 'Documentation' }), type: 'url' as const, url: '/docs', icon: 'book-open' },
];
if (isWorkspaceAdmin) {
// #3590 — `sys-settings` targets the system hub `/apps/setup/system`, not
// the bare `/apps/setup`. This cluster exists FOR the fresh env described
// above (no apps yet), and that is precisely where bare `/apps/setup` is a
// dead link: with zero apps it renders `AppContent`'s "No Apps Configured"
// empty state (`isSystemRoute` keys on a `/system` segment), so an admin
// landing on `/home` — `resolveLandingPath([])` — had no way through. The
// `/system` form resolves in BOTH branches (`extraRoutesNoApp` with no
// active app, `extraRoutes` once one exists), so an app-bearing deployment
// now reaches the hub here instead of whatever app `/apps/setup` fell back
// to. Every sibling below already spells `/apps/setup/system/...`.
const adminItems: NavigationItem[] = [
{ id: 'sys-settings', label: t('layout.systemNav.systemSettings', { defaultValue: 'System Settings' }), type: 'url' as const, url: '/apps/setup', icon: 'settings' },
{ id: 'sys-settings', label: t('layout.systemNav.systemSettings', { defaultValue: 'System Settings' }), type: 'url' as const, url: '/apps/setup/system', icon: 'settings' },
{ 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' },
Expand Down
Loading
Loading