diff --git a/client/src/App.jsx b/client/src/App.jsx index ed3c6be..6310104 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -6,6 +6,7 @@ import RequireAuth from './components/RequireAuth.jsx' import RequirePlayer from './components/RequirePlayer.jsx' import RoleGate from './components/RoleGate.jsx' import { routesFor } from './modules/registry.js' +import { ModuleFeaturesProvider } from './modules/features.jsx' // Public import Portal from './routes/public/Portal.jsx' @@ -80,190 +81,198 @@ export default function App() { return ( - - {/* Landing hero — always public, even in maintenance mode. The hero is - itself the pre-launch "coming soon" page, so it sits outside the - MaintenanceGate and every visitor sees it regardless of auth/site mode. */} - } /> + {/* Inside the auth and site contexts, because a feature provider is a + hook that may well read either — the shard one does, indirectly, by + asking an endpoint whose answer depends on the session. Outside the + routes, so the nav in every layout is filtered by the same gate and + the provider hooks are called once for the whole app rather than + once per screen. */} + + + {/* Landing hero — always public, even in maintenance mode. The hero is + itself the pre-launch "coming soon" page, so it sits outside the + MaintenanceGate and every visitor sees it regardless of auth/site mode. */} + } /> - {/* Rest of the public site — gated by maintenance mode (admins preview through it) */} - - - - } - > - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - {/* Installed modules' public pages, namespaced `//…` — the - registry prefixes the segment, so a module cannot spell its way - out of it (docs/website/MODULE_API.md §3.3). Declared before the - CMS catch-all below: React Router ranks a static segment over a - dynamic one, so the order is not what saves us, but keeping the - two adjacent makes the relationship visible to whoever adds the - next route here. */} - {routesFor('public').map((r) => ( - - ))} - {/* CMS pages: top-level /:slug, matched only after the named routes - above (React Router ranks static routes over this dynamic one). */} - } /> - - - {/* Draft-preview link (token-gated). Outside the maintenance gate so a - preview link works regardless of site mode. */} - } /> - - {/* Admin */} - } /> - - - - } - > - } /> - } /> - } /> - } /> - } /> - } /> - } /> - {/* Theme editing writes an admin-only settings key; the route sits - behind the same RoleGate as the sidebar entry that reaches it, - and PUT/DELETE /admin/settings is admin-only server-side too. */} + {/* Rest of the public site — gated by maintenance mode (admins preview through it) */} - - - } - /> - {/* Same reasoning as Appearance: the nav overrides are an admin-only - settings key, so the route carries the same RoleGate as the - sidebar entry that reaches it. */} - - - - } - /> - } /> - + - + } > - } /> - } /> - } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + {/* Installed modules' public pages, namespaced `//…` — the + registry prefixes the segment, so a module cannot spell its way + out of it (docs/website/MODULE_API.md §3.3). Declared before the + CMS catch-all below: React Router ranks a static segment over a + dynamic one, so the order is not what saves us, but keeping the + two adjacent makes the relationship visible to whoever adds the + next route here. */} + {routesFor('public').map((r) => ( + + ))} + {/* CMS pages: top-level /:slug, matched only after the named routes + above (React Router ranks static routes over this dynamic one). */} + } /> - } /> - } /> - } /> - } /> - } /> - } /> - - - - } - /> - - - - } - /> - } /> - } /> - } /> - } /> - } /> - } /> - } /> - {/* Installed modules' admin pages, at /admin//…, already inside - RequireAuth + AdminLayout. A module cannot supply its own auth - wrapper — only an optional { roles }, which core applies as the - same RoleGate its own routes above use, so the sidebar and the - route table cannot disagree about who may see what. Before the - `*` redirect, which would otherwise swallow every one of them. */} - {routesFor('admin').map((r) => ( - {r.element} : r.element} - /> - ))} - } /> - - {/* Player portal */} - } /> - } /> - } /> - } /> - } /> - - - - } - > - } /> - } /> - } /> - } /> - {/* Installed modules' player-portal pages, at /player//…. This - group's own routes are absolute (its layout route has no path), - so the prefix is written here rather than inherited — the one - place the three areas do not read alike. */} - {routesFor('player').map((r) => ( - {r.element} : r.element} - /> - ))} - + {/* Draft-preview link (token-gated). Outside the maintenance gate so a + preview link works regardless of site mode. */} + } /> - } /> - + {/* Admin */} + } /> + + + + } + > + } /> + } /> + } /> + } /> + } /> + } /> + } /> + {/* Theme editing writes an admin-only settings key; the route sits + behind the same RoleGate as the sidebar entry that reaches it, + and PUT/DELETE /admin/settings is admin-only server-side too. */} + + + + } + /> + {/* Same reasoning as Appearance: the nav overrides are an admin-only + settings key, so the route carries the same RoleGate as the + sidebar entry that reaches it. */} + + + + } + /> + } /> + + + + } + > + } /> + } /> + } /> + + } /> + } /> + } /> + } /> + } /> + } /> + + + + } + /> + + + + } + /> + } /> + } /> + } /> + } /> + } /> + } /> + } /> + {/* Installed modules' admin pages, at /admin//…, already inside + RequireAuth + AdminLayout. A module cannot supply its own auth + wrapper — only an optional { roles }, which core applies as the + same RoleGate its own routes above use, so the sidebar and the + route table cannot disagree about who may see what. Before the + `*` redirect, which would otherwise swallow every one of them. */} + {routesFor('admin').map((r) => ( + {r.element} : r.element} + /> + ))} + } /> + + + {/* Player portal */} + } /> + } /> + } /> + } /> + } /> + + + + } + > + } /> + } /> + } /> + } /> + {/* Installed modules' player-portal pages, at /player//…. This + group's own routes are absolute (its layout route has no path), + so the prefix is written here rather than inherited — the one + place the three areas do not read alike. */} + {routesFor('player').map((r) => ( + {r.element} : r.element} + /> + ))} + + + } /> + + ) diff --git a/client/src/components/SiteHeader.jsx b/client/src/components/SiteHeader.jsx index e9e142d..3cc52bf 100644 --- a/client/src/components/SiteHeader.jsx +++ b/client/src/components/SiteHeader.jsx @@ -4,22 +4,28 @@ import MoonDot from './MoonDot.jsx' import BrandLogo from './BrandLogo.jsx' import { useAuth } from '../contexts/AuthContext.jsx' import { useSite } from '../contexts/SiteContext.jsx' -import { useShardFeatures, canSee } from '../lib/useShardFeatures.js' import NavDropdown from './NavDropdown.jsx' import { buildPublicNav, pruneNav } from '../lib/navOverrides.js' import { parseJsonSetting } from '../lib/settingsJson.js' +import { withModuleNav } from '../modules/nav.js' +import { useFeatureGate } from '../modules/features.jsx' // One consistent top nav for the whole public site. Every page gets the same // main links plus an auth-aware entry on the right (Sign in / My Account / Admin). // -// Entries carrying a `feature` are shard surfaces an admin can disable or gate -// to a higher audience (Admin -> Shard Visibility). They are hidden when this -// viewer can't reach them, so we never render a link that would 403. The gate -// itself is server-side; this is only about not advertising a dead end. +// Entries carrying a `feature` are surfaces an admin can disable or gate to a +// higher audience (Admin -> Shard Visibility). They are hidden when this viewer +// can't reach them, so we never render a link that would 403. The gate itself is +// server-side; this is only about not advertising a dead end. Which module +// answers for a given flag is the registry's business now, not this file's — +// core registers `useShardFlags` for the ten below and Phase 3 hands them over +// (modules/featureGate.js). // // Exported because Admin -> Navigation edits this list. It stays declared here, // with this component as its owner: the editor may only relabel, reorder and -// hide what it finds, and `to`/`feature` are never its to change (§7). +// hide what it finds, and `to`/`feature` are never its to change (§7). An +// installed module's rows join it in `withModuleNav` below — before the override +// merge, so an admin can edit those rows exactly as they edit these. export const NAV = [ { label: 'Home', to: '/', end: true }, { label: 'News', to: '/site/news' }, @@ -48,7 +54,12 @@ const linkStyle = ({ isActive }) => ({ export default function SiteHeader() { const { user, loading } = useAuth() const { siteTitle, settings } = useSite() - const shardFeatures = useShardFeatures() + const isVisible = useFeatureGate() + + // Core's rows plus every installed module's. Computed once: the registry is + // fixed before the first render and there is no unregistering, so this cannot + // change during a session (modules/nav.js). + const baseNav = useMemo(() => withModuleNav(NAV, 'public'), []) // An admin may relabel, reorder and hide these entries from Admin → // Navigation, and may group them into dropdown sections alongside links of @@ -62,9 +73,9 @@ export default function SiteHeader() { // • with no stored row this is the coded NAV, in code order, so an // untouched instance renders exactly what it renders today. const nav = useMemo(() => { - const tree = buildPublicNav(NAV, parseJsonSetting(settings.nav_public)) - return pruneNav(tree, (item) => !item.feature || canSee(shardFeatures, item.feature)) - }, [settings.nav_public, shardFeatures]) + const tree = buildPublicNav(baseNav, parseJsonSetting(settings.nav_public)) + return pruneNav(tree, isVisible) + }, [baseNav, settings.nav_public, isVisible]) // Where the auth entry points: staff → admin, player → portal, else sign in. let account diff --git a/client/src/lib/adminNav.js b/client/src/lib/adminNav.js new file mode 100644 index 0000000..b6c6fb7 --- /dev/null +++ b/client/src/lib/adminNav.js @@ -0,0 +1,64 @@ +// Who may see a row of the admin sidebar, and where that lets them go. +// +// Plain JS, in its own file, for two reasons. It is shared — AdminLayout renders +// by it and Admin -> Navigation builds its palette by it (THEMING_AND_NAV.md +// §8.1), and a second copy of this answer is exactly the thing this file exists +// to abolish. And it is the closest thing in the client to an authorization +// decision, so it belongs somewhere the test runner can reach, which a .jsx file +// is not. +// +// **A row's own `roles` is the whole answer.** Until Phase 2 PR 8 this was +// `roles` AND a hardcoded `MOD_PATHS` list of five paths that confined +// moderators, AND a third prefix list in the redirect effect that disagreed with +// both (docs/website/MODULE_SYSTEM.md §1.4). A module's rows could never be +// added to a list core hardcodes, which is what forced the derivation — but the +// lists had already drifted from each other without a module in sight. + +/** + * Can a viewer with this role see this row? + * + * Applied AFTER the override merge in both callers: an override is presentation + * and this is the boundary, so an override saying `hidden: false` on a row this + * role cannot see still shows nothing (THEMING_AND_NAV.md §7). + * + * A row with no `roles` is visible to everyone who reached the admin area at + * all — that is the self-service case (Account, My Characters), and staff are a + * superset of players. + */ +export function navItemVisibleTo(item, role) { + return !item.roles || item.roles.includes(role) +} + +/** + * The paths a viewer with this role may reach, derived from the rows they see. + * + * Takes the BASE nav, never the override-merged one: an override must not be + * able to move this boundary in either direction. Hiding a row from a + * moderator's sidebar must not also bar them from the page behind it, and + * un-hiding one must not admit them to a page their role does not carry. + * + * @param {Array<{items: Array}>} baseNav the grouped admin nav + * @param {string} role + * @returns {Array<{to: string, exact: boolean}>} + */ +export function allowedPathsFor(baseNav, role) { + return (Array.isArray(baseNav) ? baseNav : []) + .flatMap((g) => g.items || []) + .filter((item) => navItemVisibleTo(item, role)) + .map((item) => ({ to: item.to, exact: item.end === true })) +} + +/** + * Is this pathname one of them? + * + * A row carrying `end` matches exactly — `/admin` is the dashboard, not a prefix + * of the whole admin area, and treating it as one would let every path through. + * Every other row also covers its sub-routes, which is what keeps + * `/admin/moderation/appeals/12` and a module's detail pages reachable without + * anyone listing them. + */ +export function isAllowedPath(pathname, allowed) { + return (allowed || []).some(({ to, exact }) => + exact ? pathname === to : pathname === to || pathname.startsWith(`${to}/`), + ) +} diff --git a/client/src/lib/navOverrides.js b/client/src/lib/navOverrides.js index c82dc72..a83382e 100644 --- a/client/src/lib/navOverrides.js +++ b/client/src/lib/navOverrides.js @@ -20,7 +20,10 @@ // Two shapes are supported, because two exist: // flat [{ to, label, ... }] — public header, player portal // grouped [{ title?, items: [{ to, label, ... }] }] — admin sidebar -function isGrouped(nav) { +// Exported for modules/nav.js, which has to answer the same question about the +// same array a moment earlier — one implementation, so the interleave and the +// merge can never disagree about which shape they are looking at. +export function isGrouped(nav) { return nav.length > 0 && nav.every((g) => g && Array.isArray(g.items)) } diff --git a/client/src/lib/useShardFeatures.js b/client/src/lib/useShardFeatures.js index f1c8533..6e1469a 100644 --- a/client/src/lib/useShardFeatures.js +++ b/client/src/lib/useShardFeatures.js @@ -56,3 +56,15 @@ export function useShardFeatures() { export function canSee(features, name) { return !features || features.set.has(name) } + +// The same answer in the shape core's generic feature seam takes: a Set-like of +// the flags this viewer may see, or null while we do not know yet +// (modules/featureGate.js). Core registers THIS as the provider for the `uo` +// namespace (main.jsx), so the ten shard-gated rows in the public header are +// already resolved through the module seam rather than beside it — when Phase 3 +// moves those rows into the module, the registration moves with this file and +// core is left with nothing to delete. +export function useShardFlags() { + const features = useShardFeatures() + return features ? features.set : null +} diff --git a/client/src/main.jsx b/client/src/main.jsx index dd70f9a..de0fa13 100644 --- a/client/src/main.jsx +++ b/client/src/main.jsx @@ -3,6 +3,8 @@ import { createRoot } from 'react-dom/client' import { BrowserRouter } from 'react-router-dom' import App from './App.jsx' import { publishSharedDependencies } from './modules/shared.js' +import { registerFeatureProvider } from './modules/registry.js' +import { useShardFlags } from './lib/useShardFeatures.js' import './styles/theme.css' // Publish window.__rg BEFORE rendering and before any module chunk evaluates. @@ -13,6 +15,19 @@ import './styles/theme.css' // (docs/website/MODULE_API.md §3.2). publishSharedDependencies() +// Core registers through the same seam a module uses, and registers FIRST — the +// client twin of the server's `registries.registerCore()` (MODULE_SYSTEM.md +// §1.9). The ten shard-gated rows in the public header are core's only because +// Phase 3 has not moved them yet; routing them through the registry now means +// SiteHeader holds one mechanism instead of two, and the extraction becomes a +// deletion rather than a rewrite made under extraction pressure. +// +// The owner id is `core`, which is what a nav row with no `moduleId` resolves +// against (modules/featureGate.js). The namespace is `uo`, so a module that +// wants to read these flags — the `uo` module itself, once it owns them — asks +// for them by the name they will always have had. +registerFeatureProvider('core', 'uo', useShardFlags) + // Render on DOMContentLoaded rather than immediately, and that is the one line // of core's boot the module system changes. // diff --git a/client/src/modules/featureGate.js b/client/src/modules/featureGate.js new file mode 100644 index 0000000..dc30912 --- /dev/null +++ b/client/src/modules/featureGate.js @@ -0,0 +1,56 @@ +// Which nav rows a viewer may see, when the answer belongs to a module. +// +// Phase 2, PR 8 of docs/website/MODULE_SYSTEM.md §2.7 (§1.5 states the problem); +// the contract is docs/website/MODULE_API.md §3.3. +// +// Ten of the sixteen rows in the public header carry a `feature`, and every one +// of them is a shard surface an admin can disable or gate to a higher audience. +// The provider that answers those questions — `useShardFeatures` — moves out +// with the module, so core cannot keep calling it directly and still be a core. +// It keeps a generic seam instead, and the module fills it. +// +// **The namespace comes from the registration, not from the string.** A row's +// `feature` is resolved by the provider its OWN module registered, so a module +// author writes `feature: 'status'` exactly as it reads today: nothing parses a +// prefix, and a typo'd namespace is not a thing that can exist. Core's own rows +// carry no `moduleId` and resolve against the owner id `core`, which is what +// core registers `useShardFeatures` under until Phase 3 moves those rows into +// the module and they arrive stamped `uo` instead. +// +// Everything here fails OPEN, and that is deliberate and unchanged from +// useShardFeatures' own posture: this is presentation, the gate is server-side +// (a disabled feature 404s and an out-of-rung one 403s whether or not a link was +// rendered), so an unknown answer shows the link rather than blanking the nav. +// The one thing a UI mistake must never do here is hide a page from someone +// entitled to it. + +/** + * The predicate the layouts filter their nav with. + * + * @param {Map boolean} | null | undefined>} flagsByOwner + * one entry per registered provider, keyed by the id of the module that + * registered it. The value is whatever that provider's hook returned this + * render: a Set-like of the flags this viewer may see, or `null` while the + * answer is still in flight. + * @returns {(item: object) => boolean} + */ +export function buildFeatureGate(flagsByOwner) { + return function isVisible(item) { + if (!item || !item.feature) return true + const owner = item.moduleId ?? 'core' + // No provider for this owner: the row names a flag nothing answers for. That + // is the no-module-installed case — no core row carries a `feature` once the + // module is out — and it is a correct no-op rather than a hidden row. + if (!flagsByOwner || !flagsByOwner.has(owner)) return true + const flags = flagsByOwner.get(owner) + // Still loading, or a provider that returned something unusable. Both are + // "we do not know yet", and both show the link. + if (!flags || typeof flags.has !== 'function') return true + return flags.has(item.feature) + } +} + +/** The gate an area with no providers gets: everything is visible. */ +export const OPEN_GATE = () => true + +export default buildFeatureGate diff --git a/client/src/modules/features.jsx b/client/src/modules/features.jsx new file mode 100644 index 0000000..c3eab51 --- /dev/null +++ b/client/src/modules/features.jsx @@ -0,0 +1,65 @@ +import { createContext, useContext, useMemo, useState } from 'react' +import { featureProviders } from './registry.js' +import { buildFeatureGate, OPEN_GATE } from './featureGate.js' + +// The React half of the feature seam. The decision logic is featureGate.js, +// which is plain JS and therefore testable in a runner with no DOM; this file is +// wiring, the same split registry.js and shared.js already use. +// +// **Calling a hook per provider inside a loop is the point, and it is legal +// here.** The rules of hooks require the same hooks in the same order on every +// render of a component — not a statically known list. The provider list is +// fixed before the first render (registration happens while module chunks +// evaluate, and main.jsx does not mount until DOMContentLoaded), there is no +// unregistering, and the snapshot below freezes it per component instance +// anyway. So the loop's length cannot change between renders of this provider, +// which is the actual requirement. +// +// A provider hook returns a Set-like of the flags this viewer may see, or `null` +// while it is still fetching. Core knows nothing else about it: what a flag +// means, how it is fetched, and what it is gated on are all the module's. + +const FeatureGateContext = createContext(OPEN_GATE) + +export function ModuleFeaturesProvider({ children }) { + // Snapshotted once. useState's initialiser runs on the first render only, so + // even a provider that somehow registered late cannot change this instance's + // hook count mid-life — it would be ignored until the next mount, which is a + // far better failure than a crashed render. + const [providers] = useState(featureProviders) + + // eslint-disable-next-line react-hooks/rules-of-hooks -- fixed-length list, see above + const values = providers.map((provider) => provider.hook()) + + const gate = useMemo( + () => { + const byOwner = new Map() + // First registration wins for a given owner: a module that registers two + // namespaces answers its own nav rows from the first, rather than from + // whichever happened to be stored last. + providers.forEach((provider, i) => { + if (!byOwner.has(provider.id)) byOwner.set(provider.id, values[i]) + }) + return buildFeatureGate(byOwner) + }, + // One dependency per provider — a fixed-length list, for the same reason the + // hook loop above is fixed-length. + // eslint-disable-next-line react-hooks/exhaustive-deps + [providers, ...values], + ) + + return {children} +} + +/** + * The predicate to filter nav rows with: `(item) => boolean`, true when the row + * carries no `feature` or when its module says this viewer may see it. + * + * Outside a provider it is the open gate, so a component rendered in isolation + * (a test, a preview) shows its whole nav rather than none of it. + */ +export function useFeatureGate() { + return useContext(FeatureGateContext) +} + +export default ModuleFeaturesProvider diff --git a/client/src/modules/nav.js b/client/src/modules/nav.js new file mode 100644 index 0000000..2549011 --- /dev/null +++ b/client/src/modules/nav.js @@ -0,0 +1,174 @@ +// The interleave of module nav items into core's nav. +// +// Phase 2, PR 8 of docs/website/MODULE_SYSTEM.md §2.7 (§1.4 states the problem); +// the normative contract is docs/website/MODULE_API.md §3.3. +// +// **Module items join the BASE array, before anything else happens to it.** That +// is the whole design of this file and the override merge next door forces it: +// `applyNavOverrides` / `buildPublicNav` are keyed by `to` and drop any key the +// base array does not declare (lib/navOverrides.js — deliberately, so a deleted +// route cannot leave a stale row doing something unexpected later). Append +// module items *after* that merge and they are unreachable to Admin → +// Navigation: unorderable, unrelabellable, unhideable. Today's UO rows are all +// three of those things, so appending would make the extraction a visible +// regression for every operator who has ever touched their nav. +// +// So the pipeline gains one step at the front and nothing else changes: +// +// withModuleNav(NAV, area) → admin overrides → role/feature filter → rendered +// +// and the filter stays last, which is what keeps it the boundary an override +// cannot cross (THEMING_AND_NAV.md §7). MODULE_API.md §3.3 wrote those last two +// the other way round; the code is right and the contract was amended. +// +// The result is that a module row is, to everything downstream, an ordinary row. +// Nothing in navOverrides.js, NavEditor.jsx or the layouts knows a module exists. + +import { navFor } from './registry.js' +import { isGrouped } from '../lib/navOverrides.js' + +// Rows with no group of their own are collected under this key. A Symbol rather +// than a string so it cannot collide with a group an admin or a module names. +const UNGROUPED = Symbol('ungrouped') + +/** + * Sort by effective position, where a row that asked for nothing keeps the index + * it already had. Three tie-breaks, in this order: an explicit `order` beats a + * coincidental index (the module said "third", so third), and two explicit + * orders keep registration order, which `navFor` has already put in scan order. + * + * The same rule byOrder/place use in lib/navOverrides.js, and it has to be — an + * admin who then drags that row is editing the position this produced. + */ +function place(entries) { + return entries + .map((entry, index) => ({ ...entry, index })) + .sort((a, b) => a.key - b.key || Number(b.explicit) - Number(a.explicit) || a.index - b.index) + .map(({ item }) => item) +} + +function entryFor(item, fallbackKey) { + return { item, key: item.order ?? fallbackKey, explicit: item.order !== undefined } +} + +function coreEntries(items) { + return items.map((item, index) => ({ item, key: index, explicit: false })) +} + +/** The `to`s a base nav already claims, flat or grouped. */ +function claimedPaths(baseNav, grouped) { + return new Set(grouped ? baseNav.flatMap((g) => g.items.map((i) => i.to)) : baseNav.map((i) => i.to)) +} + +/** + * Drop a module row whose `to` is already on the nav, and say so. + * + * Not a policy about where a module may link — it is that `to` is the KEY the + * override layer stores under and React renders by. Two rows sharing one would + * give an admin a single editor row that silently moves both, and a duplicate + * key in the rendered list. Dropping the newcomer keeps core's row, which is the + * one any existing override was written against. + * + * Fail-safe like every other read in this area: the offending row goes, its + * neighbours stay. + */ +function withoutCollisions(items, claimed) { + const out = [] + for (const item of items) { + if (!item || typeof item.to !== 'string' || !item.to) continue + if (claimed.has(item.to)) { + console.warn( + `[modules] nav item "${item.to}" from module "${item.moduleId}" collides with an existing row and was dropped`, + ) + continue + } + claimed.add(item.to) + out.push(item) + } + return out +} + +// The flat navs — the public header and the player portal. +// +// No groups, so `order` is a position in the one list: core rows are keyed by +// their index and a module row by the `order` it asked for. A module row with no +// order appends after the coded ones, in registration order, rather than jumping +// to the front on a 0 default — the same choice buildPublicNav makes for an +// admin-created link. +function mergeFlat(baseNav, items) { + return place([...coreEntries(baseNav), ...items.map((item, i) => entryFor(item, baseNav.length + i))]) +} + +// The grouped nav — the admin sidebar. +// +// `group` names an existing core group and the row lands inside it: Moderation +// and System, where today's UO rows already sit (§1.4). An unknown group name +// creates a group at the end rather than dropping the row — a typo must cost a +// position, never a link. A row with no `group` at all lands in a trailing +// untitled group, which renders as ungrouped links; core does not invent a +// display title out of a module id. +// +// An ungrouped row is NOT folded into one of core's own untitled groups +// (Dashboard's, Account's): those are furniture pinned to the top and bottom of +// the sidebar, and a module page does not belong beside "Account". +// +// A group created here is a group as far as everything downstream is concerned, +// including as a destination in Admin → Navigation's "move to section" control: +// `readOverrides` builds its set of legal destinations from the base nav it is +// handed, which is this one. +function mergeGrouped(baseNav, items) { + const titles = new Set(baseNav.map((g) => g.title).filter((t) => typeof t === 'string')) + const into = new Map() // existing group title → rows + const fresh = new Map() // new group title (or UNGROUPED) → rows, first-seen order + + for (const item of items) { + const named = typeof item.group === 'string' && item.group ? item.group : null + const key = named ?? UNGROUPED + const bucket = named !== null && titles.has(named) ? into : fresh + if (!bucket.has(key)) bucket.set(key, []) + bucket.get(key).push(item) + } + + const kept = baseNav.map((g) => { + const incoming = into.get(g.title) + if (!incoming) return g + return { + ...g, + items: place([...coreEntries(g.items), ...incoming.map((item, i) => entryFor(item, g.items.length + i))]), + } + }) + + const created = [...fresh.entries()].map(([key, rows]) => { + const items_ = place(rows.map((item, i) => entryFor(item, i))) + return key === UNGROUPED ? { items: items_ } : { title: key, items: items_ } + }) + + return [...kept, ...created] +} + +/** + * The base nav a layout should render: core's coded array with every installed + * module's rows for this area interleaved into it. + * + * Returns `baseNav` ITSELF when no module registered anything for this area, so + * an instance with no modules installed renders the identical array it renders + * today — the same "untouched path" guarantee applyNavOverrides makes, and what + * makes a `useMemo` with an empty dependency list around this call honest. + * + * Safe to call once per component and cache: registration completes before the + * first render (main.jsx waits for DOMContentLoaded — MODULE_API.md §3.1) and + * there is no unregistering, so this answer cannot change during a session. + * + * @param {Array} baseNav the coded NAV, flat or grouped + * @param {'public'|'admin'|'player'} area + * @returns {Array} a nav of the same shape + */ +export function withModuleNav(baseNav, area) { + if (!Array.isArray(baseNav)) return [] + const grouped = isGrouped(baseNav) + const items = withoutCollisions(navFor(area), claimedPaths(baseNav, grouped)) + if (items.length === 0) return baseNav + return grouped ? mergeGrouped(baseNav, items) : mergeFlat(baseNav, items) +} + +export default withModuleNav diff --git a/client/src/modules/registry.js b/client/src/modules/registry.js index fb5d9d4..bdc7892 100644 --- a/client/src/modules/registry.js +++ b/client/src/modules/registry.js @@ -31,7 +31,7 @@ const routes = { public: [], admin: [], player: [] } const nav = { public: [], admin: [], player: [] } -const featureProviders = new Map() +const providers = new Map() const registered = new Set() const AREAS = ['public', 'admin', 'player'] @@ -96,7 +96,7 @@ export function registerNav(id, spec) { * carries a `feature` today. */ export function registerFeatureProvider(id, namespace, hook) { - featureProviders.set(namespace, { id, hook }) + providers.set(namespace, { id, hook }) registered.add(id) } @@ -108,7 +108,19 @@ export const routesFor = (area) => routes[area] || [] export const navFor = (area) => [...(nav[area] || [])].sort((a, b) => (a.order ?? 100) - (b.order ?? 100)) -export const featureProviderFor = (namespace) => featureProviders.get(namespace) +export const featureProviderFor = (namespace) => providers.get(namespace) + +/** + * Every registered provider, for core's feature context to call. + * + * Exported from the module but deliberately NOT a member of the `registry` + * object below: a module asks for a namespace it knows the name of, and has no + * business enumerating what everyone else registered. Core needs the list + * because it has to call each hook — unconditionally, in a fixed order, at the + * top of a component (modules/features.jsx). + */ +export const featureProviders = () => + [...providers.entries()].map(([namespace, { id, hook }]) => ({ id, namespace, hook })) export const registeredIds = () => [...registered] @@ -118,7 +130,7 @@ export function _reset() { routes[area].length = 0 nav[area].length = 0 } - featureProviders.clear() + providers.clear() registered.clear() } diff --git a/client/src/routes/admin/AdminLayout.jsx b/client/src/routes/admin/AdminLayout.jsx index f7cbb7f..e8e13f0 100644 --- a/client/src/routes/admin/AdminLayout.jsx +++ b/client/src/routes/admin/AdminLayout.jsx @@ -6,6 +6,9 @@ import { useAuth } from '../../contexts/AuthContext.jsx' import { useSite } from '../../contexts/SiteContext.jsx' import { applyNavOverrides } from '../../lib/navOverrides.js' import { useNavOverrides } from '../../lib/useNavOverrides.js' +import { withModuleNav } from '../../modules/nav.js' +import { useFeatureGate } from '../../modules/features.jsx' +import { navItemVisibleTo, allowedPathsFor, isAllowedPath } from '../../lib/adminNav.js' // Small inline stroke icons (16px, currentColor) — same style as ProviderIcon. // One shared frame keeps them terse; each item just supplies its path(s). @@ -104,10 +107,6 @@ export const NAV = [ const COLLAPSE_KEY = 'admin.nav.collapsed' -// Moderators only get the moderation section (Discord + in-game ops) + their -// own account security. -const MOD_PATHS = ['/admin/moderation', '/admin/moderation/appeals', '/admin/shard-ops', '/admin/houses', '/admin/account'] - // The one row an override may never hide: the nav editor itself, which is the // only screen that can un-hide anything. The write path already refuses it // (server/src/utils/navOverrides.js) and the editor's own toggle is disabled — @@ -123,15 +122,11 @@ function keepEditorReachable(overrides) { return { ...overrides, [UNHIDEABLE]: rest } } -// Who may see a sidebar row. The single authority for that question: the layout -// applies it after the override merge (overrides are presentation, this is the -// boundary — §7), and Admin -> Navigation applies it to build its palette, so an -// admin is never offered a row they cannot themselves see (§8.1). -export function navItemVisibleTo(item, role) { - if (item.roles && !item.roles.includes(role)) return false - if (role === 'moderator') return MOD_PATHS.includes(item.to) - return true -} +// Who may see a sidebar row, and where that lets them go, both derived from the +// row's own `roles` — lib/adminNav.js, which is where the two hardcoded path +// lists this component used to carry went (MODULE_SYSTEM.md §1.4). Re-exported +// because Admin -> Navigation has always imported it from here. +export { navItemVisibleTo } const TITLES = { '/admin': 'Dashboard', @@ -159,6 +154,19 @@ const TITLES = { '/admin/account': 'Account Security', } +// An installed module's admin pages are not in TITLES and cannot be — core does +// not know what they are called. Their nav row does, so the row is the title: +// the longest matching module row wins, so a detail page under a section titles +// as that section rather than falling through to a bare "Admin". Restricted to +// rows a module registered, which is what keeps every core path resolving +// through TITLES and sectionTitle exactly as it does today. +function moduleTitle(baseNav, pathname) { + return baseNav + .flatMap((g) => g.items) + .filter((i) => i.moduleId && (pathname === i.to || pathname.startsWith(`${i.to}/`))) + .sort((a, b) => b.to.length - a.to.length)[0]?.label +} + // Fallback page title for dynamic sub-routes not in the exact-match TITLES map. function sectionTitle(pathname) { if (pathname.startsWith('/admin/moderation')) return 'Moderation' @@ -186,7 +194,15 @@ export default function AdminLayout() { const navOverrides = useNavOverrides() const navigate = useNavigate() const location = useLocation() - const title = TITLES[location.pathname] || sectionTitle(location.pathname) + const isVisible = useFeatureGate() + + // Core's rows plus every installed module's, before the override merge sees + // them — so a module row is editable in Admin -> Navigation like any other + // (modules/nav.js). Computed once: the registry is fixed before the first + // render and nothing unregisters. + const baseNav = useMemo(() => withModuleNav(NAV, 'admin'), []) + const title = + TITLES[location.pathname] || moduleTitle(baseNav, location.pathname) || sectionTitle(location.pathname) // The hero canvas editor needs room — let it use the full content width. const wide = location.pathname === '/admin/hero' const modeDot = mode === 'live' ? 'var(--mode-live)' : 'var(--mode-maint)' @@ -200,11 +216,17 @@ export default function AdminLayout() { // NAV itself and this is exactly the code that ran before the feature. const navGroups = useMemo( () => - applyNavOverrides(NAV, keepEditorReachable(navOverrides.nav_admin)) - .map((g) => ({ ...g, items: g.items.filter((item) => navItemVisibleTo(item, user?.role)) })) + applyNavOverrides(baseNav, keepEditorReachable(navOverrides.nav_admin)) + .map((g) => ({ + ...g, + // `isVisible` is a no-op for every core row — none carries a `feature` + // — and is applied here so that a module row which does carry one is + // gated on the sidebar rather than silently advertised. + items: g.items.filter((item) => navItemVisibleTo(item, user?.role) && isVisible(item)), + })) // Drop any now-empty group so an empty category header never renders. .filter((g) => g.items.length > 0), - [navOverrides.nav_admin, user?.role], + [baseNav, navOverrides.nav_admin, user?.role, isVisible], ) // Accordion: track which titled categories are collapsed. Persist across @@ -231,17 +253,22 @@ export default function AdminLayout() { g.title && g.items.some((i) => (i.end ? location.pathname === i.to : location.pathname.startsWith(i.to))) )?.title + // Where a moderator may go, from the same `roles` that decide what they see. + // It used to be a third hardcoded list — a prefix check over three paths — + // which disagreed with the sidebar's own five-path allowlist: `/admin/houses` + // was on the sidebar and not in the redirect, so a moderator who clicked + // Houses in their own nav was bounced straight back to Moderation. One + // derivation cannot disagree with itself, which is the point of deriving it. + const allowed = useMemo(() => allowedPathsFor(baseNav, user?.role), [baseNav, user?.role]) + // Confine a moderator who deep-links (or is redirected to the index) to a page // outside their remit — the API would 403 anyway, so send them to their home. useEffect(() => { if (!isModerator) return - const p = location.pathname - const allowed = - p.startsWith('/admin/moderation') || p.startsWith('/admin/shard-ops') || p === '/admin/account' - if (!allowed) { + if (!isAllowedPath(location.pathname, allowed)) { navigate('/admin/moderation', { replace: true }) } - }, [isModerator, location.pathname, navigate]) + }, [isModerator, location.pathname, navigate, allowed]) // Keep the admin out of search indexes (belt-and-suspenders with robots.txt). useEffect(() => { diff --git a/client/src/routes/admin/views/NavEditor.jsx b/client/src/routes/admin/views/NavEditor.jsx index 7861af5..3f8e2e9 100644 --- a/client/src/routes/admin/views/NavEditor.jsx +++ b/client/src/routes/admin/views/NavEditor.jsx @@ -13,7 +13,8 @@ import { Loading, ErrorState } from '../../../components/PageState.jsx' import { api } from '../../../api/client.js' import { useAuth } from '../../../contexts/AuthContext.jsx' import { useSite } from '../../../contexts/SiteContext.jsx' -import { useShardFeatures, canSee } from '../../../lib/useShardFeatures.js' +import { withModuleNav } from '../../../modules/nav.js' +import { useFeatureGate } from '../../../modules/features.jsx' import { buildNavRows, buildNavOverrides, buildPublicNav, buildPublicNavOverrides } from '../../../lib/navOverrides.js' import PublicNavTree from './PublicNavTree.jsx' import { parseJsonSetting } from '../../../lib/settingsJson.js' @@ -244,7 +245,7 @@ export function Row({ row, id, destinations, destination, onDestination, onChang export default function NavEditor() { const { user } = useAuth() const { refresh: refreshSite } = useSite() - const shardFeatures = useShardFeatures() + const isVisible = useFeatureGate() const [tab, setTab] = useState('nav_public') // Per nav: the editable groups, the overrides as loaded (so a row this admin // cannot see survives their save), and whether a settings row exists at all. @@ -255,25 +256,39 @@ export default function NavEditor() { const [saved, setSaved] = useState('') const [dirty, setDirty] = useState({}) - // The palette: each base nav, filtered to what THIS admin can see (§8.1). The - // public nav's gates are the shard-feature ones; the admin nav's are roles. - // The player portal has no gates at all. // The nav as coded, unfiltered. The palette below is what this admin may EDIT; // this is what still EXISTS, and the two are different questions. Saving needs // both: an entry for a row their palette filtered out must be carried through // rather than reset, and only an entry for a route the code no longer declares // at all should be dropped. - const fullNavs = { nav_public: PUBLIC_NAV, nav_admin: ADMIN_NAV, nav_player: PLAYER_NAV } + // + // Each nav is the coded array with every installed module's rows already + // interleaved (modules/nav.js) — the same array the layout renders, which is + // what makes a module row editable here at all: the override merge is keyed by + // `to` and drops a key the base it is handed does not declare, so a nav built + // from core alone would silently discard every stored override on a module row + // the moment it was saved. + const fullNavs = useMemo( + () => ({ + nav_public: withModuleNav(PUBLIC_NAV, 'public'), + nav_admin: withModuleNav(ADMIN_NAV, 'admin'), + nav_player: withModuleNav(PLAYER_NAV, 'player'), + }), + [], + ) + // The palette: each base nav, filtered to what THIS admin can see (§8.1). Two + // gates, and neither is core's own opinion any more — `roles` on a row, and + // the owning module's answer for a row that names a `feature`. const palettes = useMemo( () => ({ - nav_public: PUBLIC_NAV.filter((item) => !item.feature || canSee(shardFeatures, item.feature)), - nav_admin: ADMIN_NAV.map((g) => ({ ...g, items: g.items.filter((i) => navItemVisibleTo(i, user?.role)) })).filter( - (g) => g.items.length > 0, - ), - nav_player: PLAYER_NAV, + nav_public: fullNavs.nav_public.filter(isVisible), + nav_admin: fullNavs.nav_admin + .map((g) => ({ ...g, items: g.items.filter((i) => navItemVisibleTo(i, user?.role) && isVisible(i)) })) + .filter((g) => g.items.length > 0), + nav_player: fullNavs.nav_player.filter(isVisible), }), - [shardFeatures, user?.role], + [fullNavs, isVisible, user?.role], ) useEffect(() => { diff --git a/client/src/routes/player/PlayerPortalLayout.jsx b/client/src/routes/player/PlayerPortalLayout.jsx index 7536abd..589b95d 100644 --- a/client/src/routes/player/PlayerPortalLayout.jsx +++ b/client/src/routes/player/PlayerPortalLayout.jsx @@ -6,6 +6,8 @@ import { useAuth } from '../../contexts/AuthContext.jsx' import { useSite } from '../../contexts/SiteContext.jsx' import { applyNavOverrides } from '../../lib/navOverrides.js' import { useNavOverrides } from '../../lib/useNavOverrides.js' +import { withModuleNav } from '../../modules/nav.js' +import { useFeatureGate } from '../../modules/features.jsx' // Shared shell for the logged-in player portal. Uses the same sidebar shell as // Admin (icon nav, sticky content header, footer sign-out) so the two logged-in @@ -35,9 +37,10 @@ const IconGear = () => // Exported because Admin -> Navigation edits this list. It stays declared here; -// the editor may only relabel, reorder and hide what it finds (§7). No row -// carries a gate — every player sees all three — so the merged result is what -// renders, with no filter after it. +// the editor may only relabel, reorder and hide what it finds (§7). No CORE row +// carries a gate — every player sees all three — but an installed module's rows +// join this list before the merge and may carry a `feature`, so the filter after +// it is not dead code. export const NAV = [ { to: '/player', label: 'Characters', end: true, icon: IconUser }, { to: '/account/appeals', label: 'Appeals', icon: IconShield }, @@ -69,7 +72,12 @@ export default function PlayerPortalLayout() { const { user, logout } = useAuth() const { siteTitle } = useSite() const navOverrides = useNavOverrides() - const nav = useMemo(() => applyNavOverrides(NAV, navOverrides.nav_player), [navOverrides.nav_player]) + const isVisible = useFeatureGate() + const baseNav = useMemo(() => withModuleNav(NAV, 'player'), []) + const nav = useMemo( + () => applyNavOverrides(baseNav, navOverrides.nav_player).filter(isVisible), + [baseNav, navOverrides.nav_player, isVisible], + ) const navigate = useNavigate() const location = useLocation() const title = diff --git a/client/test/adminNav.test.js b/client/test/adminNav.test.js new file mode 100644 index 0000000..89f46bb --- /dev/null +++ b/client/test/adminNav.test.js @@ -0,0 +1,125 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' + +import { navItemVisibleTo, allowedPathsFor, isAllowedPath } from '../src/lib/adminNav.js' + +// Moderator confinement, derived from each row's `roles` (Phase 2 PR 8 — +// MODULE_SYSTEM.md §1.4). This replaced two hardcoded path lists that had +// drifted apart from each other, so the tests worth having are the ones that +// pin what a moderator may now see and reach, and the shape of the match. + +// The real sidebar, trimmed to the rows that decide something here. +const NAV = [ + { items: [{ to: '/admin', label: 'Dashboard', end: true, roles: ['admin', 'editor', 'moderator'] }] }, + { + title: 'Moderation', + items: [ + { to: '/admin/moderation', label: 'Moderation', roles: ['admin', 'moderator'] }, + { to: '/admin/moderation/appeals', label: 'Appeals', roles: ['admin', 'moderator'] }, + { to: '/admin/shard-ops', label: 'In-Game Ops', roles: ['admin', 'moderator'] }, + { to: '/admin/houses', label: 'Houses', roles: ['admin', 'moderator'] }, + ], + }, + { + title: 'System', + items: [ + { to: '/admin/users', label: 'Users', roles: ['admin'] }, + { to: '/admin/settings', label: 'Settings', roles: ['admin'] }, + ], + }, + { + items: [ + { to: '/admin/characters', label: 'My Characters' }, + { to: '/admin/account', label: 'Account' }, + ], + }, +] + +const visibleTo = (role) => + NAV.flatMap((g) => g.items) + .filter((i) => navItemVisibleTo(i, role)) + .map((i) => i.to) + +test('a row with no roles is visible to every staff role', () => { + // Self-service: staff are a superset of players, so a moderator reaching their + // own characters is not a privilege, it is the thing every account has. + for (const role of ['admin', 'editor', 'moderator']) { + assert.equal(navItemVisibleTo({ to: '/admin/account' }, role), true) + } +}) + +test('a role not named on the row cannot see it', () => { + assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, 'moderator'), false) + assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, 'admin'), true) + // An unknown or absent role sees only the ungated rows. + assert.equal(navItemVisibleTo({ to: '/admin/users', roles: ['admin'] }, undefined), false) + assert.equal(navItemVisibleTo({ to: '/admin/account' }, undefined), true) +}) + +test('what a moderator sees is exactly the moderation section, plus self-service', () => { + // The two additions the derivation makes over the old MOD_PATHS list are + // Dashboard — whose roles have always named moderator, so the two lists + // disagreed — and My Characters. Both are already permitted server-side. + assert.deepEqual(visibleTo('moderator'), [ + '/admin', + '/admin/moderation', + '/admin/moderation/appeals', + '/admin/shard-ops', + '/admin/houses', + '/admin/characters', + '/admin/account', + ]) +}) + +test('an admin still sees everything and an editor still sees nothing extra', () => { + assert.equal(visibleTo('admin').length, NAV.flatMap((g) => g.items).length) + assert.deepEqual(visibleTo('editor'), ['/admin', '/admin/characters', '/admin/account']) +}) + +test('a row with `end` matches exactly — the dashboard is not a prefix', () => { + // The bug this shape exists to prevent: treating `/admin` as a prefix would + // make every path in the admin area allowed for anyone who can see Dashboard. + const allowed = allowedPathsFor(NAV, 'moderator') + assert.equal(isAllowedPath('/admin', allowed), true) + assert.equal(isAllowedPath('/admin/users', allowed), false) + assert.equal(isAllowedPath('/admin/users/12', allowed), false) +}) + +test('every other row covers its own sub-routes', () => { + const allowed = allowedPathsFor(NAV, 'moderator') + assert.equal(isAllowedPath('/admin/moderation/appeals/12', allowed), true) + assert.equal(isAllowedPath('/admin/characters/0x4001', allowed), true) +}) + +test('a sibling path that merely shares a prefix is NOT covered', () => { + const allowed = allowedPathsFor(NAV, 'moderator') + // `/admin/houses-secret` starts with `/admin/houses` as a string; the match is + // on path segments, so it does not start with `/admin/houses/`. + assert.equal(isAllowedPath('/admin/houses-secret', allowed), false) + assert.equal(isAllowedPath('/admin/houses/42', allowed), true) +}) + +test('Houses is reachable, which is the defect the derivation fixed', () => { + // The redirect used to allow only /admin/moderation*, /admin/shard-ops* and + // /admin/account, while the sidebar showed Houses — so a moderator clicking a + // row in their own nav was bounced back to Moderation. + const allowed = allowedPathsFor(NAV, 'moderator') + assert.equal(isAllowedPath('/admin/houses', allowed), true) +}) + +test('a module row a moderator may see is reachable without core listing it', () => { + // The reason this is derived at all: core cannot hardcode a path it has never + // heard of, and a module row arrives with `roles` like any other. + const withModule = [ + ...NAV, + { title: 'Shard', items: [{ to: '/admin/uo/shard-ops', label: 'Ops', roles: ['admin', 'moderator'], moduleId: 'uo' }] }, + ] + const allowed = allowedPathsFor(withModule, 'moderator') + assert.equal(isAllowedPath('/admin/uo/shard-ops', allowed), true) + assert.equal(isAllowedPath('/admin/uo/shard-ops/queue', allowed), true) +}) + +test('a nav that is not there does not throw', () => { + assert.deepEqual(allowedPathsFor(null, 'moderator'), []) + assert.equal(isAllowedPath('/admin', undefined), false) +}) diff --git a/client/test/featureGate.test.js b/client/test/featureGate.test.js new file mode 100644 index 0000000..10931f6 --- /dev/null +++ b/client/test/featureGate.test.js @@ -0,0 +1,85 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' + +import { buildFeatureGate, OPEN_GATE } from '../src/modules/featureGate.js' + +// The feature seam's decision logic (MODULE_SYSTEM.md §1.5, MODULE_API.md §3.3). +// Every branch here fails OPEN, and that is the property under test as much as +// the happy path: this is presentation, the server is the gate, and a UI mistake +// that hides a page from someone entitled to it is worse in every case than one +// that shows a link which then 403s. + +const flags = (...names) => new Set(names) + +test('a row with no feature is always visible', () => { + const gate = buildFeatureGate(new Map([['uo', flags()]])) + assert.equal(gate({ to: '/site/news' }), true) +}) + +test('a core row resolves against the owner id `core`', () => { + // Core's ten shard-gated rows carry no moduleId, and core registers its + // provider under `core` (main.jsx) precisely so they resolve without one. + const gate = buildFeatureGate(new Map([['core', flags('atlas')]])) + assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true) + assert.equal(gate({ to: '/site/market', feature: 'market' }), false) +}) + +test('a module row resolves against ITS module, not another one', () => { + const gate = buildFeatureGate( + new Map([ + ['uo', flags('atlas')], + ['rust', flags('market')], + ]), + ) + assert.equal(gate({ to: '/uo/atlas', feature: 'atlas', moduleId: 'uo' }), true) + // `market` is a flag the OTHER module grants. Resolution is by registration, + // so there is no string a module can write to borrow it. + assert.equal(gate({ to: '/uo/market', feature: 'market', moduleId: 'uo' }), false) + assert.equal(gate({ to: '/rust/market', feature: 'market', moduleId: 'rust' }), true) +}) + +test('no provider for the owner shows the row', () => { + // The no-module-installed case, and the reason the filter is a correct no-op + // on a bare core rather than a nav that renders nothing. + const gate = buildFeatureGate(new Map()) + assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true) + assert.equal(gate({ to: '/uo/atlas', feature: 'atlas', moduleId: 'uo' }), true) +}) + +test('a provider still loading shows the row', () => { + // useShardFlags returns null until its fetch lands. Blanking the nav on every + // page load and filling it in a moment later is the behaviour this avoids. + const gate = buildFeatureGate(new Map([['core', null]])) + assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true) +}) + +test('a provider that returned something unusable shows the row', () => { + for (const bad of [undefined, 42, 'atlas', {}, []]) { + const gate = buildFeatureGate(new Map([['core', bad]])) + assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true, `failed closed on ${JSON.stringify(bad)}`) + } +}) + +test('an array-backed provider is not silently treated as a Set', () => { + // `[].has` does not exist, so this is the unusable case above rather than a + // membership test that quietly always fails. Asserted so that a future + // "helpful" normalisation knows it changed a documented behaviour. + const gate = buildFeatureGate(new Map([['core', ['atlas']]])) + assert.equal(gate({ to: '/site/atlas', feature: 'atlas' }), true) +}) + +test('a missing map, or a junk row, shows rather than throws', () => { + assert.equal(buildFeatureGate(null)({ feature: 'atlas' }), true) + assert.equal(buildFeatureGate(new Map())(null), true) + assert.equal(buildFeatureGate(new Map())(undefined), true) +}) + +test('any Set-like satisfies a provider — core does not require a Set', () => { + const gate = buildFeatureGate(new Map([['uo', { has: (name) => name === 'ruleset' }]])) + assert.equal(gate({ feature: 'ruleset', moduleId: 'uo' }), true) + assert.equal(gate({ feature: 'champs', moduleId: 'uo' }), false) +}) + +test('the open gate is what a component outside the provider gets', () => { + assert.equal(OPEN_GATE({ feature: 'anything' }), true) +}) diff --git a/client/test/moduleNav.test.js b/client/test/moduleNav.test.js new file mode 100644 index 0000000..011dd02 --- /dev/null +++ b/client/test/moduleNav.test.js @@ -0,0 +1,187 @@ +import { test, beforeEach } from 'node:test' +import assert from 'node:assert/strict' + +import { withModuleNav } from '../src/modules/nav.js' +import { registerNav, _reset } from '../src/modules/registry.js' +import { applyNavOverrides, buildPublicNav } from '../src/lib/navOverrides.js' + +// The interleave of module nav rows into core's nav (MODULE_API.md §3.3, Phase 2 +// PR 8). Tested against the real merge next door rather than in isolation, +// because the property that matters is a relationship between the two: a module +// row has to be indistinguishable from a core row to everything downstream, and +// the way to prove that is to run the downstream thing on it. + +const PUBLIC = [ + { label: 'Home', to: '/', end: true }, + { label: 'News', to: '/site/news' }, + { label: 'About', to: '/site/about' }, +] + +const ADMIN = [ + { items: [{ to: '/admin', label: 'Dashboard', end: true, roles: ['admin', 'moderator'] }] }, + { title: 'Moderation', items: [{ to: '/admin/moderation', label: 'Moderation' }] }, + { title: 'System', items: [{ to: '/admin/users', label: 'Users' }, { to: '/admin/settings', label: 'Settings' }] }, + { items: [{ to: '/admin/account', label: 'Account' }] }, +] + +beforeEach(() => _reset()) + +test('with no module installed the base array is returned unchanged', () => { + // Identity, not a copy: this is what makes the useMemo in each layout honest, + // and what guarantees an instance with no modules renders what it renders now. + assert.equal(withModuleNav(PUBLIC, 'public'), PUBLIC) + assert.equal(withModuleNav(ADMIN, 'admin'), ADMIN) +}) + +test('a flat nav places a module row by the order it asked for', () => { + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', order: 1 }] }) + assert.deepEqual( + withModuleNav(PUBLIC, 'public').map((i) => i.label), + ['Home', 'Atlas', 'News', 'About'], + ) +}) + +test('a flat row with no order appends rather than jumping to the front', () => { + // The 0-default trap: `order ?? 0` would put an unordered row first, which is + // the one place a module could take over the nav without asking for anything. + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }] }) + assert.deepEqual( + withModuleNav(PUBLIC, 'public').map((i) => i.label), + ['Home', 'News', 'About', 'Atlas'], + ) +}) + +test('an explicit order beats a core row that merely sits at that index', () => { + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', order: 2 }] }) + const labels = withModuleNav(PUBLIC, 'public').map((i) => i.label) + assert.deepEqual(labels, ['Home', 'News', 'Atlas', 'About']) +}) + +test('an admin row lands INSIDE the core group it names', () => { + registerNav('uo', { + area: 'admin', + items: [ + { label: 'In-Game Ops', to: '/admin/uo/shard-ops', group: 'Moderation', order: 30 }, + { label: 'Shard', to: '/admin/uo/link', group: 'System', order: 0 }, + ], + }) + const nav = withModuleNav(ADMIN, 'admin') + assert.deepEqual(nav.map((g) => g.title), [undefined, 'Moderation', 'System', undefined]) + assert.deepEqual(nav[1].items.map((i) => i.label), ['Moderation', 'In-Game Ops']) + // order 0 puts it above both core rows, which is the whole point of the field. + assert.deepEqual(nav[2].items.map((i) => i.label), ['Shard', 'Users', 'Settings']) +}) + +test('an unknown group appends a new group instead of dropping the row', () => { + // A typo must cost a position, never a link. + registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Moderaton' }] }) + const nav = withModuleNav(ADMIN, 'admin') + assert.equal(nav.length, ADMIN.length + 1) + assert.deepEqual(nav.at(-1), { title: 'Moderaton', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Moderaton', moduleId: 'uo' }] }) +}) + +test('an admin row with no group gets a trailing untitled group of its own', () => { + // NOT folded into one of core's untitled groups: those are Dashboard at the + // top and Account at the bottom, and a module page belongs beside neither. + registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas' }] }) + const nav = withModuleNav(ADMIN, 'admin') + assert.equal(nav.length, ADMIN.length + 1) + assert.equal(nav.at(-1).title, undefined) + assert.deepEqual(nav.at(-1).items.map((i) => i.label), ['Atlas']) + assert.deepEqual(nav[0].items.map((i) => i.label), ['Dashboard']) + assert.deepEqual(nav[3].items.map((i) => i.label), ['Account']) +}) + +test('a row whose `to` collides with a core row is dropped, not rendered twice', () => { + // `to` is the key the override layer stores under and React renders by. Two + // rows sharing one would give an admin a single editor row that moves both. + const warnings = [] + const warn = console.warn + console.warn = (msg) => warnings.push(msg) + try { + registerNav('uo', { + area: 'public', + items: [{ label: 'Not News', to: '/site/news' }, { label: 'Atlas', to: '/uo/atlas' }], + }) + const nav = withModuleNav(PUBLIC, 'public') + assert.deepEqual(nav.map((i) => i.label), ['Home', 'News', 'About', 'Atlas']) + assert.equal(warnings.length, 1) + assert.match(warnings[0], /\/site\/news.*collides/) + } finally { + console.warn = warn + } +}) + +test('two modules cannot claim the same path either', () => { + const warn = console.warn + console.warn = () => {} + try { + registerNav('aa', { area: 'public', items: [{ label: 'First', to: '/shared' }] }) + registerNav('zz', { area: 'public', items: [{ label: 'Second', to: '/shared' }] }) + const labels = withModuleNav(PUBLIC, 'public').map((i) => i.label) + assert.deepEqual(labels, ['Home', 'News', 'About', 'First']) + } finally { + console.warn = warn + } +}) + +test('a module row carries its moduleId through, which is how the gate finds it', () => { + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas', feature: 'atlas' }] }) + const row = withModuleNav(PUBLIC, 'public').at(-1) + assert.equal(row.moduleId, 'uo') + assert.equal(row.feature, 'atlas') +}) + +test('areas do not leak into one another', () => { + registerNav('uo', { area: 'admin', items: [{ label: 'Shard', to: '/admin/uo/link', group: 'System' }] }) + assert.equal(withModuleNav(PUBLIC, 'public'), PUBLIC) +}) + +// ── The relationship that is the actual requirement ─────────────────────── + +test('an admin override applies to a module row exactly as to a core row', () => { + // The reason the interleave happens BEFORE the merge and not after: the merge + // drops any key its base array does not declare, so appending module rows + // afterwards would make every one of them unorderable, unrelabellable and + // unhideable — a visible regression the day the UO rows leave core. + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }] }) + const base = withModuleNav(PUBLIC, 'public') + const merged = applyNavOverrides(base, { + '/uo/atlas': { label: 'Bestiary', order: 0 }, + '/site/news': { order: 3 }, + }) + assert.deepEqual(merged.map((i) => i.label), ['Bestiary', 'Home', 'About', 'News']) +}) + +test('an override can hide a module row, and the public tree can section it', () => { + registerNav('uo', { area: 'public', items: [{ label: 'Atlas', to: '/uo/atlas' }, { label: 'Market', to: '/uo/market' }] }) + const base = withModuleNav(PUBLIC, 'public') + + const hidden = buildPublicNav(base, { '/uo/atlas': { hidden: true } }) + assert.equal(hidden.some((n) => n.to === '/uo/atlas'), false) + + const sectioned = buildPublicNav(base, { + items: { '/uo/market': { section: 'sec_shard' } }, + sections: [{ id: 'sec_shard', label: 'Shard', order: 0 }], + }) + assert.equal(sectioned[0].kind, 'section') + assert.deepEqual(sectioned[0].items.map((i) => i.to), ['/uo/market']) +}) + +test('a module row can be moved between admin groups by an override', () => { + registerNav('uo', { area: 'admin', items: [{ label: 'Shard', to: '/admin/uo/link', group: 'System' }] }) + const base = withModuleNav(ADMIN, 'admin') + const merged = applyNavOverrides(base, { '/admin/uo/link': { group: 'Moderation' } }) + assert.deepEqual(merged[1].items.map((i) => i.to), ['/admin/moderation', '/admin/uo/link']) + assert.deepEqual(merged[2].items.map((i) => i.to), ['/admin/users', '/admin/settings']) +}) + +test('a group a module created is itself a legal override destination', () => { + // Falls out of building the destination set from the base nav it is handed — + // recorded because it is the kind of thing that would otherwise be discovered + // by an admin finding a section they cannot move anything into. + registerNav('uo', { area: 'admin', items: [{ label: 'Atlas', to: '/admin/uo/atlas', group: 'Shard' }] }) + const base = withModuleNav(ADMIN, 'admin') + const merged = applyNavOverrides(base, { '/admin/users': { group: 'Shard' } }) + assert.deepEqual(merged.at(-1).items.map((i) => i.to), ['/admin/uo/atlas', '/admin/users']) +}) diff --git a/client/test/moduleRegistry.test.js b/client/test/moduleRegistry.test.js index 2c5740a..49c6153 100644 --- a/client/test/moduleRegistry.test.js +++ b/client/test/moduleRegistry.test.js @@ -12,6 +12,7 @@ import { routesFor, navFor, featureProviderFor, + featureProviders, registeredIds, _reset, } from '../src/modules/registry.js' @@ -126,6 +127,28 @@ test('a feature provider is stored under its namespace, with its owner', () => { assert.equal(featureProviderFor('nothing'), undefined) }) +test('providers can be enumerated in registration order, with their owner', () => { + // Core's feature context has to CALL each of these, as a hook, in a fixed + // order — so it needs the list, and it needs the owner id to resolve a nav + // row whose `moduleId` says who it belongs to (modules/features.jsx). + const uo = () => null + const rust = () => null + registerFeatureProvider('uo', 'shard', uo) + registerFeatureProvider('rust', 'server', rust) + assert.deepEqual(featureProviders(), [ + { id: 'uo', namespace: 'shard', hook: uo }, + { id: 'rust', namespace: 'server', hook: rust }, + ]) +}) + +test('enumerating providers is NOT part of the module-facing surface', () => { + // A module asks for a namespace it knows the name of; enumerating what + // everyone else registered is core's business, so `featureProviders` is a + // module export and not a member of window.__rg.registry. + assert.equal(registry.featureProviders, undefined) + assert.equal(typeof featureProviders, 'function') +}) + test('every registration marks the module registered', () => { registerRoutes('a', { public: [{ path: 'x' }] }) registerNav('b', { area: 'public', items: [] })