Skip to content

Commit e998240

Browse files
committed
fix(rest): drop a nav group DECLARED empty, not just one the gate emptied (#7380)
`filterNav`'s docblock has promised "Drop empty groups so the sidebar doesn't render a label with nothing under it" since #4651, but the guard in front of that branch was `Array.isArray(e.children) && e.children.length > 0`. A group authored `children: []` therefore never entered the branch that OWNS the rule — it fell through the else and shipped in `GET /meta/app` as a bare label. The one shape the sentence most obviously covers was the one shape it could not reach. The judgement is now on what SURVIVES rather than on how the entry got there: a `type: 'group'` with no surviving children is dropped whether it BECAME empty (children gated away) or STARTED empty (`children: []`). Nesting composes, and a group carrying no `children` key at all — unreachable through the spec, but reachable at runtime because this filter reads untyped documents off the metadata store — is the same dead label and drops too. Contribution slots are the shape this actually shipped. `setup.app.ts` is authored entirely out of it: nine group anchors with `children: []`, filled on read by `Registry.applyNavContributions` (ADR-0029 D7). That merge runs in the protocol layer BEFORE this filter, so a filled slot arrives with children and survives while an unfilled one arrives `[]` and is now dropped — exactly the "a disabled capability contributes nothing and its slot stays empty" case `setup.app.ts` documents. The rule is `type: 'group'` and nothing else. An `object` entry navigates on its own `objectName`, so emptiness says nothing about whether to serve it; non-group entries keep today's behaviour exactly. A group cannot be a target — `GroupNavItemSchema` is a `strictObject` declaring no target key and rejecting any, and reads "Does not perform navigation itself". Measured before the change: of the 16 childless `type: 'group'` entries across the shipped apps and the examples, zero carry a target key, so the drop is unconditional. Because `areas[].navigation` runs through the same `filterNav`, an area holding only childless groups now empties and is dropped by the existing area rule; an area authored `navigation: []` still passes through, and that divergence is documented at `filterAreas`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WNJY6He34CWyS8VDBtxVM9
1 parent ba63f0e commit e998240

3 files changed

Lines changed: 411 additions & 9 deletions

File tree

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): drop a navigation group that was DECLARED empty, not just one the gate emptied (#7380)
6+
7+
`filterAppForUser`'s docblock has promised "Empty groups collapse so the sidebar
8+
doesn't render a label with no children" since #4651, and its `filterNav` branch
9+
carried the matching comment. The guard in front of that branch was
10+
`Array.isArray(e.children) && e.children.length > 0`, so a group authored
11+
`children: []` never reached the rule that owns the promise — it fell through the
12+
`else` and shipped in `GET /meta/app` as a bare label. The one shape the sentence
13+
most obviously covers was the one shape it could not reach.
14+
15+
The judgement is now on what SURVIVES rather than on how the entry got there. A
16+
`type: 'group'` with no surviving children is dropped whether it **became** empty
17+
(children filtered away by `requiredPermissions` / the ADR-0057 D10
18+
`requiresService` gate) or **started** empty (`children: []`). Nesting composes:
19+
an outer group left holding only a dropped inner group collapses in the same
20+
pass. A group carrying no `children` key at all — unreachable through the spec,
21+
where `children` is required on both the input and output `group` branches, but
22+
reachable at runtime because this filter reads untyped documents off the metadata
23+
store — is the same dead label and drops too.
24+
25+
**Contribution slots are the shape this actually shipped.** `setup.app.ts` is
26+
authored entirely out of it: nine `type: 'group'` anchors with `children: []`,
27+
filled on read by `Registry.applyNavContributions` (ADR-0029 D7) from whichever
28+
capability packages are installed. That merge runs in the protocol layer *before*
29+
this filter, so a slot a plugin filled arrives here with children and survives,
30+
while a slot left empty because its capability is disabled arrives `[]` and is
31+
now dropped — exactly the "a disabled capability contributes nothing and its slot
32+
stays empty" case `setup.app.ts` documents. Deployments that ran without the
33+
optional plugins were serving those anchors as empty, unopenable sidebar
34+
headings; they now disappear, and the ones with contributions are untouched.
35+
36+
**The rule is `type: 'group'` and nothing else.** The navigation union nests on
37+
two branches (`object` and `group`). An `object` entry navigates on its own
38+
`objectName`, so `{ type: 'object', objectName: 'lead', children: [] }` is a live
39+
link that nests nothing, and emptiness says nothing about whether to serve it —
40+
non-group entries keep their existing behaviour exactly, including when the gate
41+
empties their children. A group cannot be a target: `GroupNavItemSchema` is a
42+
`strictObject` declaring no `objectName` / `pageName` / `componentRef` / `url`
43+
(it rejects them), and its docblock reads "Does not perform navigation itself."
44+
Measured before the change: 41 `type: 'group'` entries across the shipped apps
45+
(`account`, `setup`, `studio`), the examples (`app-crm`, `app-showcase`,
46+
`app-todo`) and the spec's nav type-assertion fixtures. 16 are childless — the 9
47+
`setup` slots plus 7 spec fixtures, none in the example apps — and zero of the 41
48+
carry `objectName` / `pageName` / `componentRef` / `url` or any other target. The
49+
drop is therefore unconditional; no standalone-group shape needed sparing.
50+
51+
One consequence worth naming: because `areas[].navigation` is filtered through
52+
this same `filterNav`, an area whose entries are all childless groups now empties
53+
and is dropped by the existing area-collapse rule. An area authored
54+
`navigation: []` is still passed through untouched, as before — a group is a
55+
sidebar label and nothing else, while an area is a workspace the shell can select
56+
on its own, and that divergence is documented at `filterAreas`.

packages/rest/src/rest-server.ts

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,7 +2772,11 @@ export class RestServer {
27722772
* a subset of the user's system permissions.
27732773
* - Recursively strips child navigation entries (groups, items) whose
27742774
* `requiredPermissions` are not satisfied. Empty groups collapse so
2775-
* the sidebar doesn't render a label with no children.
2775+
* the sidebar doesn't render a label with no children — [#7380] a
2776+
* `type: 'group'` with no SURVIVING children is dropped whether it was
2777+
* emptied by the gate or authored `children: []`. Only `group` collapses;
2778+
* an `object` entry is its own target and is served however many children
2779+
* it has. See the rule at the `filterNav` branch for the measurement.
27762780
* - [#4722] Applies the SAME item gate to every `areas[].navigation` tree.
27772781
* Both trees are the same shape and the keys mean the same thing in both,
27782782
* so `filterNav` is reused — there is deliberately no second
@@ -2839,13 +2843,57 @@ export class RestServer {
28392843
const req = Array.isArray(e.requiredPermissions) ? e.requiredPermissions : [];
28402844
if (req.length > 0 && !req.every((p: string) => sysPerms.has(p))) continue;
28412845
if (typeof e.requiresService === 'string' && serviceGate && serviceGate(e.requiresService) === false) continue;
2842-
if (Array.isArray(e.children) && e.children.length > 0) {
2846+
// [#7380] A `group` is judged on what SURVIVES, never on how it
2847+
// got there. Both childless shapes render the same dead sidebar
2848+
// label, so both are dropped:
2849+
// - BECAME empty — authored with children, all gated away;
2850+
// - STARTED empty — authored `children: []`.
2851+
// The old guard (`children.length > 0`) sent the second shape
2852+
// down the else branch, which never reaches the drop rule, so a
2853+
// declared-empty group shipped as a bare label the docblock
2854+
// above already promised it would not. That shape is not a
2855+
// corner case: `setup.app.ts` is authored entirely out of it —
2856+
// nine `children: []` contribution slots (ADR-0029 D7) that
2857+
// `Registry.applyNavContributions` fills on read, BEFORE this
2858+
// filter runs. So a slot a capability plugin filled arrives here
2859+
// with children and survives; a slot left empty because its
2860+
// capability is disabled arrives `[]` and is now dropped, which
2861+
// is exactly the "a disabled capability contributes nothing and
2862+
// its slot stays empty" case `setup.app.ts` documents.
2863+
//
2864+
// The rule is `type === 'group'` ONLY, and stays that way. The
2865+
// union nests on two branches (`NAV_VARIANTS_ACCEPTING_CHILDREN`
2866+
// = `object` | `group`), and an `object` entry is its own
2867+
// navigation target — `{ type: 'object', objectName: 'lead',
2868+
// children: [] }` is a live link to the lead list, not a label,
2869+
// so emptiness says nothing about whether to serve it. A group
2870+
// cannot be a target: `GroupNavItemSchema` is a `strictObject`
2871+
// over the base keys plus `expanded`/`children` and declares no
2872+
// `objectName` / `pageName` / `componentRef` / `url` — it
2873+
// REJECTS them — and its docblock reads "Does not perform
2874+
// navigation itself." Measured against that before the change
2875+
// (#7380): 41 `type: 'group'` entries across the shipped apps
2876+
// (`account`, `setup`, `studio`), the examples (`app-crm`,
2877+
// `app-showcase`, `app-todo`) and the spec's nav type-assertion
2878+
// fixtures. 16 are childless — the 9 `setup` slots and 7 spec
2879+
// fixtures; the three example apps have none — and ZERO of the
2880+
// 41 carry `objectName` / `pageName` / `componentRef` / `url` or
2881+
// any other target. So the drop is unconditional: there is no
2882+
// standalone childless-group shape in the tree to spare.
2883+
//
2884+
// A group with NO `children` key is covered by the same rule for
2885+
// the same reason — same dead label. It is unreachable through
2886+
// the spec (`children` is required on both the input and output
2887+
// group branches; `app.nav-type-assertions.ts` pins that with a
2888+
// `@ts-expect-error`), but this filter reads untyped documents
2889+
// off the metadata store, so leaving it out would just reopen
2890+
// the bypass one keyword over.
2891+
if (Array.isArray(e.children)) {
28432892
const kids = filterNav(e.children);
2844-
// Drop empty groups so the sidebar doesn't render a label
2845-
// with nothing under it (matches AppSidebar UX).
28462893
if (e.type === 'group' && kids.length === 0) continue;
28472894
out.push({ ...e, children: kids });
28482895
} else {
2896+
if (e.type === 'group') continue;
28492897
out.push(e);
28502898
}
28512899
}
@@ -2858,11 +2906,25 @@ export class RestServer {
28582906
// INSIDE an area, through the very same `filterNav` the top-level tree
28592907
// uses, so the two trees can never disagree about what a key means.
28602908
//
2861-
// Collapse rule, taken from what `filterNav` already does to a `group`:
2862-
// an area whose authored tree is emptied BY the gate is dropped (a bare
2863-
// area label with nothing reachable under it is not a useful response),
2864-
// while an area authored empty is passed through untouched — filtering
2865-
// reports what the caller may not see, it does not tidy the metadata.
2909+
// Collapse rule: an area whose authored tree is emptied BY the gate is
2910+
// dropped (a bare area label with nothing reachable under it is not a
2911+
// useful response), while an area authored `navigation: []` is passed
2912+
// through untouched — filtering reports what the caller may not see, it
2913+
// does not tidy the metadata.
2914+
//
2915+
// [#7380] That second half is where an area and a `group` now DIVERGE,
2916+
// deliberately: `filterNav` drops a childless group however it got that
2917+
// way, an area authored empty still ships. The reason is what the two
2918+
// shapes are. A group is a sidebar label and nothing else, so childless
2919+
// it renders dead — and the shipped `setup` app authors nine of them as
2920+
// contribution SLOTS, which makes "declared empty" the normal steady
2921+
// state of an unfilled one rather than an authoring slip. An area is a
2922+
// top-level workspace the shell can select and route to on its own; an
2923+
// author who ships `navigation: []` has declared an area that is not
2924+
// populated yet, and this filter is not the layer that judges that.
2925+
// What is NOT divergent is the walk: an area whose entries are all
2926+
// childless groups empties through the very same `filterNav` and is
2927+
// dropped by the rule above — one implementation, as everywhere else.
28662928
const filterAreas = (list: any[]): any[] => {
28672929
const out: any[] = [];
28682930
for (const a of list) {

0 commit comments

Comments
 (0)