Skip to content

Commit 5ad6f22

Browse files
fix(app-shell): the delegated-admin scope facet shows the spec default for an unset key (#26990)
Fixes #26706 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio › Access › 权限集 › 委派管理(Delegated admin)里的开关,对范围里没有设置的键显示协议(`AdminScopeSchema`)的默认值;管理员切换开关时才写入显式的布尔值。 - 为什么改:委派管理是大客户安全下放管理权的方式,屏幕上显示的委派范围必须和服务端实际执行的一致,管理员才能正确判断自己授出了什么。 - 风险与回滚:只改前端显示,不改存储数据、保存载荷和服务端校验;回滚即 revert 本 PR。 - 先看:`PermissionAdvancedFacets.tsx` 的 `ADMIN_SCOPE_SWITCH_DEFAULTS` 与 `scopeSwitchOn`(风险 hunk:两个 Switch 的 `checked`);新测试 `PermissionAdvancedFacets.adminScopeDefault-26706.test.tsx`;`PermissionAdvancedFacets.adminScopeAnchor.test.tsx` 里两处期望值的更新。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/PermissionAdvancedFacets.adminScopeDefault-26706.test.tsx`(先 build `@objectstack/spec`),预期 `Tests 6 passed (6)`。 - 需要你做的:无,席位落地。 ## What changes The delegated-admin scope facet shows the spec default for an unset key. - Each switch in the Delegated admin facet now draws `scopeSwitchOn(scope, key)`: the scope's own boolean when it sets one, and otherwise the spec default for that key from `ADMIN_SCOPE_SWITCH_DEFAULTS`. That is the value `AdminScopeSchema` parses the key to and the value the server enforces. - Drawing a default writes nothing. The draft keeps an untouched key unset, so a save keeps it unset. A flip writes an explicit boolean, as before. The write paths (`setScope` / `setScopeDetail`) and the business-unit gate are unchanged. - The two defaults are restated, not imported, for the reason `useTenancyPosture` gives (the `@objectstack/spec/security` value entry adds its schema modules to the console bundle). They are typed against the spec's `AdminScope`, and the new test reads every expected value from the installed spec, so a default the spec moves fails CI here. - Landing site: `ui/packages/app-shell/src/views/metadata-admin/PermissionAdvancedFacets.tsx`, the site the card expected. A grep of `ui/` for the two keys finds no other surface that draws them. ## Tests - New `PermissionAdvancedFacets.adminScopeDefault-26706.test.tsx`, 6 cases: a CONTROL that the installed spec parses each unset switch key to a boolean default; an unset key draws the spec default (with a business unit, and while the switches wait for one); naming the business unit writes that key alone; a flip writes an explicit boolean and draws it, and flipping back keeps the key written; an explicit boolean draws as itself. - `PermissionAdvancedFacets.adminScopeAnchor.test.tsx`: two expectations updated. Each asserted the boolean a single flip writes from the drawn state, and that value follows what the switch draws. - Ablation, one time, through `scripts/ablation-replace.mjs`: one switch's `checked` reading was put back to its pre-change expression (anchor 1 to 0, replacement 0 to 1, blob `cabc1416e0fd` to `c5282ce2ba31`). Result: `Tests 6 failed | 12 passed (18)` across the two facet files, in the direction declared in advance (red). Restored: the blob equals HEAD and `git diff HEAD` is empty. The test imports the component by relative path, so the run read `src/` and no `dist/` leg applies. Run on `3592965f`; the merge of `main` since then does not touch the facet or its tests. ## Acceptance notes - None. No out-of-scope finding. ## Verification All readings below were taken on head `394213f2` (a merge of `origin/main` at `f1cd529d`). <details><summary>Facet suites: 8 files, 113 tests, exit 0</summary> ``` pnpm --dir ui exec vitest run --maxWorkers=2 (the 7 PermissionAdvancedFacets.*.test.tsx files + src/__tests__/spec-symbol-parity.test.ts) Test Files 8 passed (8) Tests 113 passed (113) ``` </details> <details><summary>Type check: @object-ui/app-shell type-check, exit 0</summary> `tsc --noEmit && tsc -p tsconfig.test.json`. `tsc -p tsconfig.test.json --listFilesOnly` lists both changed test files, so the test-layer program compiles them. </details> <details><summary>Derived gate families: 34 of 34 run; 33 exit 0, one test run timed out under load and passed on re-run</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (no paths; change set from the merge base) derived 34 commands. Each ran with its exit code captured before any pipe. - 33 commands: exit 0. - `pnpm --dir ui exec vitest run scripts/`: exit 1, `Tests 9 failed | 5056 passed | 2 skipped (5067)`. All 9 failures are `Test timed out in 15000ms` in whole-tree scans, on a shared box with the other gates running. The 6 files that timed out, re-run alone under the verify lock: `Test Files 6 passed (6)`, `Tests 522 passed (522)`, exit 0. The same command on `3592965f`, before the merge of `main`, exited 0. This diff does not touch `ui/scripts/`. - `--ran` reconciliation: `34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED (a DERIVED zero — all 34 recorded an exit code and none of them is 3)`. Declared narrowing: `origin/main` moved again after the merge. The derivation names 6 gate-input files that changed upstream since `394213f2`. CI runs those families on the merge ref of this PR. </details> Not run locally, and owned by CI: `pnpm lint`, the `test:repo` project, and the workspace type-check lanes. --- _Generated by [Claude Code](https://claude.ai/code/session_01J8VVAkZns5rNnK2rtszzbQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e46e25c commit 5ad6f22

4 files changed

Lines changed: 198 additions & 6 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): the delegated-admin scope facet shows the spec default for an unset key
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In Studio › Access › a permission set › Delegated admin, each switch now draws the value the server enforces: the scope's own boolean when it sets one, and the `AdminScopeSchema` default when it leaves the key unset. Drawing a default writes nothing, so a save keeps an untouched key unset; flipping a switch writes an explicit boolean. Nothing the server accepts or enforces changes.
11+
12+
What to do: nothing. Review a delegated-admin scope that you set up by reading this screen, and confirm that its switches show the scope you intended.

‎ui/packages/app-shell/src/views/metadata-admin/PermissionAdvancedFacets.adminScopeAnchor.test.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,12 @@ describe("PermissionAdvancedFacets · delegated admin scope (objectui#9464)", ()
139139
const user = await mountFacet({ adminScope: { businessUnit: "bu_sales" } });
140140
await user.click(switchFor("perm.admin.includeSubtree"));
141141

142+
// The switch draws the spec default for the unset key, so one flip writes
143+
// its opposite as an explicit boolean
144+
// (`PermissionAdvancedFacets.adminScopeDefault-26706.test.tsx`).
142145
expect(latest.adminScope).toEqual({
143146
businessUnit: "bu_sales",
144-
includeSubtree: true,
147+
includeSubtree: false,
145148
});
146149
expect(AdminScopeSchema.safeParse(latest.adminScope).success).toBe(true);
147150
});
@@ -214,9 +217,11 @@ describe("PermissionAdvancedFacets · the retired delegated-admin keys (objectui
214217
AdminScopeSchema.safeParse(written).success,
215218
`the editor wrote ${JSON.stringify(written)}, which the spec refuses`,
216219
).toBe(true);
220+
// Each flip writes the opposite of the spec default its switch draws for
221+
// the unset key.
217222
expect(written).toEqual({
218223
businessUnit: "bu_sales",
219-
includeSubtree: true,
224+
includeSubtree: false,
220225
manageAssignments: true,
221226
assignablePermissionSets: ["ps_support"],
222227
});
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Pins what the Delegated Admin Scope facet's switches draw against the spec's
5+
* own defaults (objectstack#26706).
6+
*
7+
* A delegated-admin scope may leave a defaulted key unset; the server parses it
8+
* with `AdminScopeSchema`, so the unset key means that schema's default. Each
9+
* switch must therefore draw the spec default for an unset key, draw an
10+
* explicit boolean as itself, write nothing merely by being drawn, and write an
11+
* explicit boolean when the author flips it.
12+
*
13+
* Every expected value is read from the installed spec (`AdminScopeSchema`),
14+
* not restated here, so the pin follows the protocol: a default the spec moves
15+
* fails this file against the editor's restated
16+
* `ADMIN_SCOPE_SWITCH_DEFAULTS`. The CONTROL case keeps a reading that never
17+
* reached the schema from passing as a reading about the editor.
18+
*/
19+
20+
import * as React from "react";
21+
import { describe, it, expect, afterEach } from "vitest";
22+
import { render, screen, within, cleanup } from "@testing-library/react";
23+
import userEvent from "@testing-library/user-event";
24+
import { AdminScopeSchema } from "@objectstack/spec/security";
25+
import { PermissionAdvancedFacets } from "./PermissionAdvancedFacets";
26+
27+
afterEach(cleanup);
28+
29+
const t = (k: string) => k;
30+
31+
type Draft = Record<string, unknown>;
32+
33+
/** The draft as the host would hold it after the last update. */
34+
let latest: Draft = {};
35+
36+
function Harness({ initial }: { initial: Draft }) {
37+
const [draft, setDraft] = React.useState<Draft>(initial);
38+
React.useEffect(() => {
39+
latest = draft;
40+
}, [draft]);
41+
return (
42+
<PermissionAdvancedFacets
43+
draft={draft}
44+
setDraft={setDraft}
45+
writable
46+
allSetNames={[]}
47+
t={t}
48+
only="admin"
49+
/>
50+
);
51+
}
52+
53+
function mount(initial: Draft) {
54+
const user = userEvent.setup();
55+
latest = initial;
56+
render(<Harness initial={initial} />);
57+
return user;
58+
}
59+
60+
const BU = "bu_east";
61+
62+
/** Each boolean key the facet draws a switch for, with the switch's caption. */
63+
const SWITCHES = [
64+
["includeSubtree", "perm.admin.includeSubtree"],
65+
["manageAssignments", "perm.admin.manageAssignments"],
66+
] as const;
67+
68+
/** The spec's own parse of a scope that names its business unit and nothing else. */
69+
const specParsed = () => AdminScopeSchema.parse({ businessUnit: BU });
70+
71+
/** A switch lives inside the `<label>` that carries its caption. */
72+
const switchFor = (captionKey: string) =>
73+
within(screen.getByText(captionKey).closest("label") as HTMLElement).getByRole("switch");
74+
75+
const drawn = (captionKey: string) => switchFor(captionKey).getAttribute("aria-checked");
76+
77+
describe("PermissionAdvancedFacets · delegated-admin switches draw the spec default (objectstack#26706)", () => {
78+
it("CONTROL — the installed spec parses each unset switch key to a boolean default", () => {
79+
const parsed = specParsed();
80+
for (const [key] of SWITCHES) {
81+
expect(typeof parsed[key], key).toBe("boolean");
82+
}
83+
expect(parsed.includeSubtree).toBe(true);
84+
});
85+
86+
it("an unset key draws the spec default", () => {
87+
mount({ adminScope: { businessUnit: BU } });
88+
const parsed = specParsed();
89+
for (const [key, caption] of SWITCHES) {
90+
expect(drawn(caption), key).toBe(String(parsed[key]));
91+
}
92+
expect(switchFor("perm.admin.includeSubtree")).toBeChecked();
93+
});
94+
95+
it("the switches draw the same defaults while they wait for a business unit", () => {
96+
mount({});
97+
const parsed = specParsed();
98+
for (const [key, caption] of SWITCHES) {
99+
expect(switchFor(caption)).toBeDisabled();
100+
expect(drawn(caption), key).toBe(String(parsed[key]));
101+
}
102+
});
103+
104+
it("naming the business unit writes that key alone: drawing a default never writes it", async () => {
105+
const user = mount({});
106+
await user.type(screen.getByRole("textbox"), BU);
107+
108+
expect(latest.adminScope).toEqual({ businessUnit: BU });
109+
const parsed = specParsed();
110+
for (const [key, caption] of SWITCHES) {
111+
expect(drawn(caption), key).toBe(String(parsed[key]));
112+
}
113+
});
114+
115+
it("a flip writes an explicit boolean, the opposite of what was drawn, and draws it", async () => {
116+
const parsed = specParsed();
117+
for (const [key, caption] of SWITCHES) {
118+
const user = mount({ adminScope: { businessUnit: BU } });
119+
120+
await user.click(switchFor(caption));
121+
expect(latest.adminScope, key).toEqual({ businessUnit: BU, [key]: !parsed[key] });
122+
expect(drawn(caption), key).toBe(String(!parsed[key]));
123+
124+
// Flipped back, the key stays written: explicit, and equal to the default.
125+
await user.click(switchFor(caption));
126+
expect(latest.adminScope, key).toEqual({ businessUnit: BU, [key]: parsed[key] });
127+
expect(drawn(caption), key).toBe(String(parsed[key]));
128+
expect(AdminScopeSchema.safeParse(latest.adminScope).success, key).toBe(true);
129+
130+
cleanup();
131+
}
132+
});
133+
134+
it("an explicit boolean draws as itself, whichever the default", () => {
135+
for (const value of [true, false]) {
136+
mount({ adminScope: { businessUnit: BU, includeSubtree: value, manageAssignments: value } });
137+
for (const [key, caption] of SWITCHES) {
138+
expect(drawn(caption), `${key}: ${value}`).toBe(String(value));
139+
}
140+
cleanup();
141+
}
142+
});
143+
});

‎ui/packages/app-shell/src/views/metadata-admin/PermissionAdvancedFacets.tsx‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,9 @@ function stripRetiredRlsKeys(policy: RlsPolicy): RlsPolicy {
119119
* `Partial<>` is the single deliberate divergence and it is the draft-buffer
120120
* one: the spec REQUIRES `businessUnit`, while this editor materializes the
121121
* facet from `asObject(draft.adminScope)` — `{}` before the author has typed
122-
* anything — and every field below reads through a fallback (`?? ''`, `!!`)
123-
* because a half-filled facet is the normal mid-edit state. Pinned in
122+
* anything — and every field below reads through a fallback (`?? ''`, and the
123+
* spec default for a switch, {@link scopeSwitchOn}) because a half-filled facet
124+
* is the normal mid-edit state. Pinned in
124125
* `spec-symbol-parity.test.ts`, both halves: the spec still requires
125126
* `businessUnit` (so the widening is real and load-bearing) and no key here is
126127
* invented (so the widening stays confined to requiredness).
@@ -141,6 +142,37 @@ function stripRetiredRlsKeys(policy: RlsPolicy): RlsPolicy {
141142
*/
142143
type AdminScope = Partial<SpecAdminScope>;
143144

145+
/**
146+
* The spec's default for each boolean key the delegated-admin facet draws a
147+
* switch for: what `AdminScopeSchema` parses the key to when the scope leaves it
148+
* unset, and so the value the server enforces for it.
149+
*
150+
* Restated rather than imported for the reason `useTenancyPosture` gives: the
151+
* `@objectstack/spec/security` value entry adds its schema modules to the
152+
* console bundle, here for two booleans. The keys are the spec type's, and
153+
* `PermissionAdvancedFacets.adminScopeDefault-26706.test.tsx` compares what each
154+
* switch draws for an unset key with the schema's own parse of it, so a default
155+
* the spec moves fails CI here.
156+
*/
157+
const ADMIN_SCOPE_SWITCH_DEFAULTS = {
158+
includeSubtree: true,
159+
manageAssignments: false,
160+
} as const satisfies Required<Pick<SpecAdminScope, 'includeSubtree' | 'manageAssignments'>>;
161+
162+
type AdminScopeSwitch = keyof typeof ADMIN_SCOPE_SWITCH_DEFAULTS;
163+
164+
/**
165+
* What a delegated-admin switch draws: the draft's own boolean, or, for a key
166+
* the draft does not set to a boolean, the spec default from
167+
* {@link ADMIN_SCOPE_SWITCH_DEFAULTS}. Drawing the value never writes it: the
168+
* draft keeps the key unset until the author flips the switch, and a flip
169+
* writes an explicit boolean.
170+
*/
171+
function scopeSwitchOn(scope: AdminScope, key: AdminScopeSwitch): boolean {
172+
const value = scope[key];
173+
return typeof value === 'boolean' ? value : ADMIN_SCOPE_SWITCH_DEFAULTS[key];
174+
}
175+
144176
type TabVisibility = 'visible' | 'hidden' | 'default_on' | 'default_off';
145177
type TabPerms = Record<string, TabVisibility>;
146178

@@ -821,7 +853,7 @@ export function PermissionAdvancedFacets({
821853
</div>
822854
<label className="flex items-center gap-1.5 text-xs pb-1.5">
823855
<Switch
824-
checked={!!scope.includeSubtree}
856+
checked={scopeSwitchOn(scope, 'includeSubtree')}
825857
disabled={!writable || !scopeAnchored}
826858
onCheckedChange={(v) => setScopeDetail({ includeSubtree: !!v })}
827859
/>
@@ -841,7 +873,7 @@ export function PermissionAdvancedFacets({
841873
<div className="flex flex-wrap gap-x-6 gap-y-2">
842874
<label className="flex items-center gap-1.5 text-xs">
843875
<Switch
844-
checked={!!scope.manageAssignments}
876+
checked={scopeSwitchOn(scope, 'manageAssignments')}
845877
disabled={!writable || !scopeAnchored}
846878
onCheckedChange={(v) => setScopeDetail({ manageAssignments: !!v })}
847879
/>

0 commit comments

Comments
 (0)