Skip to content

Commit 4eed49a

Browse files
committed
fix(settings): answer a closed section with the product's not-found page, not Next's bare default
1 parent 8bdc71c commit 4eed49a

4 files changed

Lines changed: 3 additions & 54 deletions

File tree

‎apps/sim/app/(product)/_components/live-shell/live-shell.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { type ReactNode, Suspense } from 'react'
22
import { cookies } from 'next/headers'
3-
import { notFound, redirect } from 'next/navigation'
3+
import { redirect } from 'next/navigation'
44
import { loginHref, WORKSPACES_PATH } from '@/lib/navigation/paths'
55
import { resolveLiveDestinations } from '@/app/(product)/_components/destinations'
66
import { LiveComposerHost } from '@/app/(product)/_components/live-shell/live-composer-host'
@@ -13,7 +13,6 @@ import { startShellPrefetch } from '@/platform/lib/data/prefetch-shell'
1313
import { LiveProviders } from '@/platform/lib/data/providers'
1414
import {
1515
followProductRoute,
16-
isClosedSettingsSection,
1716
type ProductLocation,
1817
readDocumentLocation,
1918
} from '@/platform/lib/data/route-resolution'
@@ -46,8 +45,7 @@ async function retiredOrganizationHref({
4645
* organization layout's gates, in its order, against data read in process. Each gate decides
4746
* before anything product-shaped renders. The page below streams inside the Suspense boundary, so a
4847
* redirect it raised would paint this shell first; the document's own route resolution
49-
* (`followProductRoute`) runs here instead, and answers with one HTTP redirect, as does a Settings
50-
* section the viewer may not open, with one 404 (`isClosedSettingsSection`).
48+
* (`followProductRoute`) runs here instead, and answers with one HTTP redirect.
5149
*/
5250
export async function LiveShell({
5351
telemetry,
@@ -89,7 +87,6 @@ export async function LiveShell({
8987
location && followProductRoute(organization, location),
9088
shellPrefetch.finish(organization),
9189
])
92-
if (location && isClosedSettingsSection(organization, location)) notFound()
9390

9491
return (
9592
<LiveProviders

‎apps/sim/platform/AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ A sub-view (one record's detail, where it is also edited, or the create flow) op
110110

111111
A read is `SettingsRead`: nothing while pending, `FailureNotice` with Retry when the first read fails, the content under a `FailureNotice` when a later refresh fails. An on-or-off setting is `SwitchRow` (`switch-row`), never a `Switch` placed in a `SettingsRow`. A draft is committed with `SettingsSaveBar` (`save-owner`): EMCN's `SaveActions` plus the draft's registration with the Settings leave guard, so it is the only Save and Discard a settings page renders. A sub-view's back action leaves through `requestSettingsLeave`, which asks the same guard. A create form keeps its own submit button, Create {noun} or Add {noun}, because the bar only says Save. A list of `ActivityRow` rows is `ResourceList`, which bleeds through `PageOverflow`. A titled list that has no rows is `SettingsList` with `empty`: the `EmptyState` takes the surface's place under the section title, never sits inside a surface (`empty-in-surface`), where its own inset would set its text 8px inside the rows'. A create row that lived in that surface moves to the `EmptyState`'s action.
112112

113-
A browser-rendered section's first reads start on the server: `lib/data/prefetch-settings.ts` seeds them under each hook's key and `staleTime`, and the page hands them to the section through a `HydrationBoundary`, as a server section's own `prefetch-*` read does. A read that depends on the browser (the viewer's time zone, a clock-relative range) is not prefetched, since its key would not match. A Settings section the viewer may not open is answered by the shell (`isClosedSettingsSection`) before the page streams, so the response is a 404; a `notFound()` inside the page arrives after the status has been sent.
113+
A browser-rendered section's first reads start on the server: `lib/data/prefetch-settings.ts` seeds them under each hook's key and `staleTime`, and the page hands them to the section through a `HydrationBoundary`, as a server section's own `prefetch-*` read does. A read that depends on the browser (the viewer's time zone, a clock-relative range) is not prefetched, since its key would not match. A Settings section the viewer may not open answers with the product's not-found page from inside the page (`LiveSettingsRoute`): the shell renders in the product root layout, where a `notFound()` would skip that page for Next's bare default.
114114

115115
Every settings page uses these parts, and `save-owner`, `switch-row` and `empty-in-surface` grant no exceptions.
116116

‎apps/sim/platform/lib/data/route-resolution.test.ts‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import type { PlatformOrganization } from '@/platform/lib/data/organization-mode
2727
import {
2828
blockedWorkspaceEntryHref,
2929
deniedSurfaceHref,
30-
isClosedSettingsSection,
3130
type RouteContext,
3231
resolveProductRedirect,
3332
} from '@/platform/lib/data/route-resolution'
@@ -331,27 +330,3 @@ describe('an organization the viewer can no longer open', () => {
331330
expect(deniedSurfaceHref(false)).toBeNull()
332331
})
333332
})
334-
335-
describe('a Settings section the viewer may not open', () => {
336-
const opens = (href: string, settingsSections: PlatformOrganization['settingsSections']) => {
337-
const url = new URL(href, 'https://app.sim.test')
338-
return isClosedSettingsSection(
339-
{ ...organization(), settingsSections },
340-
{ pathname: url.pathname, query: url.searchParams }
341-
)
342-
}
343-
344-
it('is answered by the shell before the page streams, with or without a subpath', () => {
345-
expect(opens('/o/org-1/settings/billing', ['general'])).toBe(true)
346-
expect(opens('/o/org-1/settings/billing/credit-usage', ['general'])).toBe(true)
347-
expect(opens('/o/org-1/settings/admin', ['general', 'admin'])).toBe(true)
348-
expect(opens('/o/org-1/settings/no-such-section', ['general'])).toBe(true)
349-
})
350-
351-
it('leaves General, an open section and the routes beside the sections to the page', () => {
352-
expect(opens('/o/org-1/settings', [])).toBe(false)
353-
expect(opens('/o/org-1/settings/billing', ['general', 'billing'])).toBe(false)
354-
expect(opens('/o/org-1/settings/billing/plans', ['general'])).toBe(false)
355-
expect(opens('/o/org-1/home', [])).toBe(false)
356-
})
357-
})

‎apps/sim/platform/lib/data/route-resolution.ts‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
import { resolveLegacyRequestLink } from '@/platform/lib/data/legacy-request-link'
2222
import { type PlatformOrganization, surfaceHref } from '@/platform/lib/data/organization-model'
2323
import { requireSessionViewer } from '@/platform/lib/data/server-read'
24-
import { resolveLiveSetting } from '@/platform/lib/data/settings-navigation'
2524
import { readWorkspaceShellOrganizationId } from '@/platform/lib/data/shell-scope'
2625
import { workspaceRootHref } from '@/platform/lib/workspace-navigation'
2726

@@ -273,28 +272,6 @@ export async function followProductRoute(
273272
redirect(target.href)
274273
}
275274

276-
/** Settings routes beside the sections' catch-all, which gate themselves. */
277-
const SETTINGS_ROUTES_BESIDE_SECTIONS = ['billing/plans']
278-
279-
/**
280-
* Whether a document request opens a Settings section this viewer may not open, which the shell
281-
* answers as not found. The page streams inside the shell's Suspense boundary after the status has
282-
* been sent, so a section's own `notFound()` would reach the browser as a 200 with the not-found
283-
* body; the shell decides before anything streams, as it does for a redirect. A section's own gate
284-
* and subpaths still answer inside the page, and a client navigation, which has no status, there.
285-
*/
286-
export function isClosedSettingsSection(
287-
organization: PlatformOrganization,
288-
location: ProductLocation
289-
): boolean {
290-
if (!isSettingsPath(organization, location.pathname)) return false
291-
const rest = location.pathname.slice(surfaceHref(organization, '/settings').length + 1)
292-
if (!rest || SETTINGS_ROUTES_BESIDE_SECTIONS.some((route) => `${rest}/`.startsWith(`${route}/`)))
293-
return false
294-
const [key] = decodePathSegments(rest.split('/')) ?? []
295-
return !key || !resolveLiveSetting(key, organization)
296-
}
297-
298275
/**
299276
* The path and query this document request opens, as the proxy names it; `null` outside the proxy.
300277
* The shell renders above the page's Suspense boundary and without route params, so this is how it

0 commit comments

Comments
 (0)