docs(website): settle the nav interleave and the feature-provider seam
Records Phase 2 PR 8 of the module system: MODULE_SYSTEM.md gains the PR entry in 2.7 and the built-it notes on 1.4 and 1.5; MODULE_API.md 3.3 is amended where building it settled something the draft left open or got wrong. The amendments to 3.3: - The pipeline arrow had role/feature filtering BEFORE admin overrides. The code has always been the other way round, deliberately - the filter runs last so it stays a boundary an override cannot cross (THEMING_AND_NAV 7). - `feature` was documented as public-area-only. It applies in all three areas: core's admin and player navs still carry no flags, but a module row that declares a gate and has it silently ignored is a trap. - How a provider is found: by the module that registered the row, not by a prefix parsed out of the flag name. Core's own rows resolve against the owner id `core`, which core registers useShardFlags under. - What a provider hook returns, and that every unknown fails OPEN. - Why calling one hook per provider in a loop is legal, and why the enumerator is a module export rather than a member of registry. - Six details the interleave settled: unordered rows append rather than defaulting to 0; an ungrouped admin row gets its own trailing group rather than joining core's; a module-created group is a legal override destination; a colliding `to` is dropped with a warning; and why the interleave must precede the override merge. MOD_PATHS' replacement is recorded in both files, including the defect the derivation fixed: the moderator redirect was a third hardcoded list that disagreed with MOD_PATHS about /admin/houses. THEMING_AND_NAV 7's "moderator confinement" note is amended to match. Code: website PR 8 (client-only; 160 client tests, manifest and OpenAPI unchanged), verified with the 7.7 browser smoke. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -539,20 +539,82 @@ registry.registerNav('uo', {
|
||||
|
||||
`group` names an existing core group; an unknown group name appends a new group at the end rather
|
||||
than dropping the item. `order` sorts within the group, core items keeping their current positions.
|
||||
`feature` (public area only) names a flag resolved by §3.3's provider.
|
||||
`feature` names a flag resolved by the provider below.
|
||||
|
||||
`MOD_PATHS` in `AdminLayout.jsx:109` — today a hardcoded allowlist of two UO paths — becomes a
|
||||
computation over each item's `roles`, so moderator visibility follows from the registration instead
|
||||
of from a second list that has to be kept in sync.
|
||||
Six details settled when this was built (Phase 2 PR 8, `client/src/modules/nav.js`):
|
||||
|
||||
- **`order` on a flat nav is a position among core's rows**, which are keyed by their index; an
|
||||
explicit `order` beats a core row that merely sits at that index. A row with **no** `order`
|
||||
appends after the coded rows rather than defaulting to 0 — otherwise "I didn't ask for a
|
||||
position" would mean "put me first", which is the one place a module could take over the header
|
||||
without asking for anything.
|
||||
- **A row with no `group` on the admin sidebar gets a trailing untitled group of its own**, not a
|
||||
place in one of core's untitled groups. Those are Dashboard at the top and Account at the bottom;
|
||||
a module page belongs beside neither, and core does not invent a display title out of a module id.
|
||||
- **A group a module created is itself a legal override destination.** It falls out of building the
|
||||
destination set from the merged base nav, and is recorded so it is not mistaken for an accident.
|
||||
- **A row whose `to` collides with an existing row is dropped, with a console warning.** `to` is the
|
||||
key the override layer stores under and React renders by, so two rows sharing one would give an
|
||||
admin a single editor row that silently moves both. Core's row wins, since that is the one any
|
||||
stored override was written against.
|
||||
- **`feature` is not public-area-only.** An earlier draft of this section said it was, on the
|
||||
grounds that core's admin and player navs carry no flags. They still do not — but a module row
|
||||
that declares a gate and has it silently ignored is a trap, so the gate is applied in all three
|
||||
areas and the field means one thing everywhere.
|
||||
- **The interleave happens before the override merge, and that ordering is load-bearing.** The merge
|
||||
is keyed by `to` and drops any key its base array does not declare, so module rows appended
|
||||
afterwards would be unorderable, unrelabellable and unhideable — a visible regression for every
|
||||
operator who has ever edited a nav, the day the UO rows leave core.
|
||||
|
||||
Moderator confinement, `MOD_PATHS` in `AdminLayout.jsx:109` — a hardcoded allowlist of five paths —
|
||||
becomes a computation over each item's `roles`, so moderator visibility follows from the
|
||||
registration instead of from a second list that has to be kept in sync. It moved to
|
||||
`client/src/lib/adminNav.js`, plain JS so the test runner can reach it, along with the redirect that
|
||||
confines a moderator who deep-links. **The redirect derives from the base nav, never the
|
||||
override-merged one**: an override is presentation and must not move an authorization boundary in
|
||||
either direction — hiding a row must not also bar someone from the page, and un-hiding one must not
|
||||
admit them to it.
|
||||
|
||||
Deriving it changed what a moderator sees, in both cases toward what the server already permitted:
|
||||
**Dashboard**, whose `roles` had always named moderator while `MOD_PATHS` omitted it, and **My
|
||||
Characters**, which is ungated self-service. It also fixed a defect the two lists had between them —
|
||||
`/admin/houses` was on the sidebar and not in the redirect's own third list, so a moderator clicking
|
||||
Houses in their own nav was bounced back to Moderation.
|
||||
|
||||
The pipeline is unchanged from `THEMING_AND_NAV.md`, with one new first step:
|
||||
|
||||
> registered defaults (core **+ modules**) → role/feature filtering → admin overrides → rendered nav
|
||||
> registered defaults (core **+ modules**) → admin overrides → role/feature filtering → rendered nav
|
||||
|
||||
An earlier draft of this line had the last two the other way round. The filter runs **last** and
|
||||
that is deliberate — it is what keeps it a boundary an override cannot cross (`THEMING_AND_NAV.md`
|
||||
§7), and both layouts have always been written that way.
|
||||
|
||||
**`registerFeatureProvider`** — core keeps a generic flag context; the module supplies the hook that
|
||||
fills its namespace (`useShardFeatures` for `uo`). With no module installed the filter is a correct
|
||||
no-op, because no core nav item carries a `feature` today.
|
||||
|
||||
**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 `useShardFlags` under (`main.jsx`), the client twin of the server's `registries.registerCore()`.
|
||||
So the ten shard-gated rows in the public header already run through the module seam rather than
|
||||
beside it, and Phase 3 deletes core's registration instead of rewriting the header.
|
||||
|
||||
A provider hook returns **a Set-like of the flags this viewer may see, or `null`** while the answer
|
||||
is in flight. Every unknown — no provider, a null answer, a provider that returned something without
|
||||
a `has`, a malformed row — **shows the link**. This is presentation and the server is the gate, so 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.
|
||||
|
||||
Core calls every registered provider's hook unconditionally, in a fixed order, at the top of the
|
||||
context component. That is legal because the rules of hooks require the same hooks in the same order
|
||||
on every render of a component, not a statically known list: registration completes before the first
|
||||
render (§3.1), nothing unregisters, and the provider list is snapshotted per component instance
|
||||
anyway. `registry.featureProviders()` is a module export and deliberately **not** a member of the
|
||||
`registry` object handed to modules — a module asks for a namespace it knows the name of, and has no
|
||||
business enumerating what everyone else registered.
|
||||
|
||||
### 3.4 `ui` — the shared component kit
|
||||
|
||||
**This is the largest addition Phase 1 makes to the plan, and it is not optional** (§6.2; approved
|
||||
|
||||
@@ -97,6 +97,14 @@ moderator-visible.
|
||||
**Resolved:** nav registration takes a target group and order (`{ group: 'Moderation', order: 30 }`),
|
||||
and `MOD_PATHS` becomes a `roles`-derived computation rather than a path allowlist.
|
||||
|
||||
Built in Phase 2 PR 8. Two things it turned up that this section did not predict. The interleave has
|
||||
to happen **before** the admin-override merge and not after it, because that merge drops any `to`
|
||||
its base array does not declare — appending module rows afterwards would leave them uneditable in
|
||||
Admin → Navigation, which today's UO rows are not. And there was a **third** hardcoded list: the
|
||||
redirect that confines a moderator checked three path prefixes, while `MOD_PATHS` listed five paths,
|
||||
and they disagreed about `/admin/houses` — a moderator who clicked Houses in their own sidebar was
|
||||
bounced straight back to Moderation. One derivation cannot disagree with itself.
|
||||
|
||||
### 1.5 The public nav's feature-gating mechanism is itself a shard system
|
||||
|
||||
Ten of the sixteen entries in `SiteHeader.jsx`'s NAV carry a `feature:` key (`status`, `champs`,
|
||||
@@ -108,6 +116,12 @@ Extracting the module removes the provider that core's own nav filter depends on
|
||||
registers its `useShardFeatures` for its own namespace. No core nav item carries a `feature` today,
|
||||
so with no module installed the filter is a correct no-op.
|
||||
|
||||
Built in Phase 2 PR 8, and core registers into it **now** rather than at extraction: `useShardFlags`
|
||||
goes in under the owner id `core`, so the ten rows above are already resolved through the seam and
|
||||
`SiteHeader` runs one mechanism instead of two. Which provider answers a row is decided by the
|
||||
module that registered it, not by a prefix parsed out of the flag name, so those ten keep the exact
|
||||
strings they carry today and Phase 3 moves them without a rename.
|
||||
|
||||
### 1.6 There is no migration system to model a module migration runner on
|
||||
|
||||
`server/db/schema.sql` is a single idempotent file — 1,380 lines, 67 tables — replayed in full on
|
||||
@@ -627,6 +641,43 @@ fixed enum because the leg set is whatever has been registered.
|
||||
filesystem-conditional static mount, not API surface, for the same reason `/uploads` and `/brand`
|
||||
are not in the manifest.
|
||||
|
||||
- **PR 8** — the nav half PR 7 deferred, and the two seams §1.4 and §1.5 asked for: `withModuleNav`
|
||||
(`client/src/modules/nav.js`) interleaving module rows into core's three navs, `MOD_PATHS` and the
|
||||
moderator redirect replaced by a `roles`-derived computation in `client/src/lib/adminNav.js`, and
|
||||
the generic feature-provider seam (`modules/features.jsx` + `modules/featureGate.js`) that core
|
||||
registers its own `useShardFlags` into. Four decisions, all recorded in
|
||||
[`MODULE_API.md`](MODULE_API.md) §3.3.
|
||||
|
||||
**The interleave happens before the admin-override merge**, which is the decision the rest follow
|
||||
from: the merge is keyed by `to` and drops any key its base array does not declare, so module rows
|
||||
appended after it would be unorderable, unrelabellable and unhideable — and 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 edited their nav. Doing it first means a module row is an ordinary row to
|
||||
everything downstream: nothing in `navOverrides.js`, `NavEditor.jsx` or the layouts knows a module
|
||||
exists. **Moderator visibility derives purely from `roles`**, which moves two rows the old
|
||||
allowlist withheld — Dashboard, whose `roles` had always named moderator, and My Characters, which
|
||||
is ungated self-service — both toward what the server already permitted. **A row's `feature` is
|
||||
resolved by the provider its own module registered**, so the namespace comes from the registration
|
||||
rather than from a parsed string prefix. And **core registers through the same seam**, under the
|
||||
owner id `core`, so `SiteHeader` holds one mechanism instead of two and Phase 3 is a deletion.
|
||||
|
||||
The PR also fixed a defect that predates the module system: the moderator redirect was a **third**
|
||||
hardcoded list, and it disagreed with `MOD_PATHS` about `/admin/houses`, so a moderator who
|
||||
clicked Houses in their own sidebar was bounced back to Moderation. The derived allow-list is
|
||||
computed from the **base** nav rather than the merged one, so an override — which is presentation
|
||||
— cannot move that boundary in either direction.
|
||||
|
||||
**933 server tests** (unchanged — this PR is client-only) and **160 client tests** (+37) pass;
|
||||
`routes.manifest.json` is unchanged at 230 routes and the OpenAPI spec regenerates byte-identical.
|
||||
The [§7.7](MODULE_API.md#77-the-client-half-has-to-be-verified-in-a-browser--the-timing-bug-no-test-could-see)
|
||||
browser smoke was re-run, since this is the seam that rule exists for: a throwaway module
|
||||
registering nav in all three areas and a provider granting one flag and withholding another. It
|
||||
confirmed, in Chrome with the console open, that the row lands inside core's Moderation group
|
||||
rather than in an appended block, that the withheld row does not render while the granted one
|
||||
does, that a moderator reaches both `/admin/houses` and the module's own admin page, and that an
|
||||
admin can relabel a module row in Admin → Navigation and have it persist and apply — the whole
|
||||
point of merging before the override layer. Zero CSP reports, zero console errors.
|
||||
|
||||
**Phase 3 — Extract `module-uo`.** Moves out of `website/`: the 8 model directories and their 25
|
||||
tables; the nine UO `utils/` files plus `newsGump.js`; the 13 router/controller files;
|
||||
`scripts/importSpawnAtlas.js` and `db/spawnAtlas.art.json`; `usersShard.controller.js` **minus
|
||||
|
||||
@@ -536,10 +536,20 @@ Three existing behaviors the merge must not disturb:
|
||||
no menu. `pruneNav` applies the gate inside a section and then drops one it
|
||||
leaves empty.
|
||||
|
||||
- **Moderator confinement.** `AdminLayout` restricts moderators to `MOD_PATHS` and
|
||||
redirects them out of anything else. Overrides apply before that filter, so a
|
||||
moderator can still end up with a legitimately short sidebar — but the redirect
|
||||
effect must keep working untouched.
|
||||
- **Moderator confinement.** `AdminLayout` restricts moderators to the rows their
|
||||
role carries and redirects them out of anything else. Overrides apply before
|
||||
that filter, so a moderator can still end up with a legitimately short sidebar
|
||||
— but the redirect effect must keep working untouched.
|
||||
|
||||
Amended by the module system's Phase 2 PR 8: this used to be a hardcoded
|
||||
`MOD_PATHS` allowlist plus a second, differently-worded prefix check in the
|
||||
redirect, and the two had drifted — `/admin/houses` was on the sidebar and not
|
||||
in the redirect, so a moderator who clicked Houses was bounced to Moderation.
|
||||
Both are now derived from each row's own `roles`
|
||||
([`adminNav.js`](../../website/client/src/lib/adminNav.js)), and the redirect
|
||||
derives from the **base** nav rather than the merged one, which is what keeps
|
||||
an override from moving the boundary in either direction. See
|
||||
[`MODULE_SYSTEM.md`](MODULE_SYSTEM.md) §1.4.
|
||||
- **Empty groups.** `AdminLayout` drops groups whose items all filtered out. An
|
||||
override that hides every item in a group must produce no orphaned header.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user