Skip to content

Commit e14d9ce

Browse files
fix(app-shell): the Delegated Admin Scope block lists a delegate's companion grants and grants the missing ones (#26191) (#26340)
Fixes #26191 Clause-②: no Metadata-protocol: none Part of the #26059 ruling (6416233738, Q3 child 5, item 2). UI and docs only: no server change, no spec key. ## What changes **The Delegated Admin Scope block lists the companion grants a delegate needs.** A scope says what its holder may administer; it does not let the holder in. Once the scope names a business unit (the block's existing gate for its dependent controls), the block lists: - `setup.access`: enter Setup - `sys_user`, `sys_business_unit`, `sys_business_unit_member`: read, so the lookups can pick people and units - `sys_user_position`: create, read, edit and delete, for the assignment rows Each row is marked "On this set" or "Not on this set", read from the draft being edited (`systemPermissions` and `objects`). A row counts as granted only when every flag it needs is `true`; nothing is inferred. **"Grant the missing ones" adds only what is missing, as one draft edit.** It is one `setDraft` call, and the set's normal Save persists it. `setup.access` is appended to `systemPermissions`. Each companion object row gains only the flags it lacks. An existing row keeps every key it already has (an extra Edit, a View All), and a row that already covers its grant is left as the same object. Nothing is removed or narrowed, and no new key is written: both keys already exist on `PermissionSetSchema`. The button shows only when the set is writable and something is missing. **Under a package binding, the hint is shown without the action.** The package door's draft holds, and its Save writes, only the package's own objects (`scopePermissionSet` / `mergePermissionSlice`, ADR-0086 P0). There, the `sys_*` rows would not survive Save, and their absence from the draft says nothing about the stored set. So under a package the block lists the five grants without a granted/missing mark and without the button, plus a note: grant these in Setup, on an environment permission set the delegate holds. **How the facet knows it is package-bound.** `PermissionAdvancedFacets` gains an optional `packageBound` prop. When the prop is omitted, it is read from `only`. The Studio Access pillar is the one host that sets `only`, and it always edits under a `packageId` (`AccessPillar`'s `packageId` is required). Setup's permission page sets neither. `PermissionMatrixEditor.tsx` is outside this card's claimed file surface, so this PR does not wire `packageBound={!!packageId}` there. See Acceptance notes. **Docs.** `content/docs/permissions/delegated-administration.mdx`: the authoring example now carries `systemPermissions: ['setup.access']`. A new paragraph names the five companion grants and why each is needed, and says the Setup editor lists them and grants the missing ones. **Strings.** Nine new `perm.admin.companions.*` keys in `metadata-admin/i18n.ts`, in both locales the block's existing strings use (en, zh). ## Tests `PermissionAdvancedFacets.companionGrants-26191.test.tsx` drives the facet with the draft held as the host holds it. It has 10 cases: - the five grants are listed and marked missing on an empty set - grants the set already carries are marked granted, and read alone is not CRUD - the block is hidden until the scope names a business unit - one click is one draft write, adds only the missing grants, and leaves existing rows and an unrelated object untouched - the written set parses under `PermissionSetSchema` and carries no new key - there is no action when nothing is missing - read-only: the hint shows and the action does not - `packageBound`: the hint shows unmarked, there is no action, and the note shows - the Studio mount (`only`) is package-bound by default - an explicit `packageBound={false}` keeps the action Red first, at `c3066c2b` (the test committed, the component not yet changed): 9 failed, 1 passed. The one that passed is the "hidden until a business unit is named" case, which holds both before and after the change. The failures were `Unable to find [data-testid="admin-companions"]` and the like, not errors. Green, all at head `8ecd7344` (this branch with `main` merged after #26284 landed), from cwd `ui/` under `scripts/pm/os-verify-lock.sh`: - `pnpm exec vitest run FILES --maxWorkers=2` over 254 files: every app-shell test file that imports `PermissionAdvancedFacets` or `views/metadata-admin/i18n`, the new file included. `Test Files 254 passed (254)`, `Tests 2578 passed (2578)`. The lock's verdict was `command-exit 0`. The full app-shell suite was not run locally; CI runs it. - `eslint` on the three changed source files: 0 errors and 5 warnings. Two warnings are new, both `no-explicit-any` in the test harness, where the updater's `prev` is typed `any`, as the prop itself types it. The other three are on existing lines. - `pnpm --filter @object-ui/app-shell type-check`: **NOT MEASURED** locally. It ran without the `@object-ui/*` dependency closure built, so it answered only unresolved-module cascades (TS2307 / TS7006). None of them is on a line this PR adds. CI's `Type Check UI` is the reading. - `node scripts/pm/dispatch-gates.mjs --commands`: 68 derived commands, reconciled with `--ran` as 66 run (all exit 0) and 2 NOT MEASURED. Both of those are exit-3 prerequisite refusals: `check:skill-examples` (`packages/client-react` was not built; this page's example block carries no type-check marker) and `check:type-check-debt` (it needs the shared lock for its closure build and timed out in the queue). CI's `Lint & Repo Gates` runs both. - `node scripts/check-sdui-manifest.mjs`: exit 0. app-shell has no sdui record, so there was nothing to re-record. ## Acceptance notes - **Package binding wiring.** The package-bound default is read from `only`, not from `packageId`, because `PermissionMatrixEditor.tsx` is outside this card's file surface (#26284 held it). Once that file is open to a card, passing `packageBound={!!packageId}` at both facet mounts makes the binding explicit, and the `only` fallback can go. Until then the two coincide by construction: `packageId` is passed only by `AccessPillar`, always together with `studio` chrome, and that chrome is what sets `only`. - **Marks are exact, not inferred.** A set that reaches Setup through some broader grant is still shown as "Not on this set" for `setup.access`. Clicking then adds the exact capability, which is an addition and narrows nothing. - `PermissionMatrixEditor.tsx`, `permission-slice.ts`, the server, `packages/spec` and `ui/packages/i18n` are untouched. app-shell has no sdui record (`node scripts/check-sdui-manifest.mjs` exits 0). - The docs path (`content/docs/**`) is not a governed surface: `node scripts/pm/check-governed-merges.mjs --test` over the five files exits 0. --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0489214 commit e14d9ce

5 files changed

Lines changed: 469 additions & 1 deletion

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): the Delegated Admin Scope block lists the companion grants a delegate needs, and grants the missing ones in one click
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A permission set's admin scope says what its holder may administer; it does not let the holder in. A delegate also needs the `setup.access` capability to enter Setup, read on `sys_user`, `sys_business_unit` and `sys_business_unit_member` so the lookups can pick people and units, and create, read, edit and delete on `sys_user_position` for the assignment rows. The scope block in the permission set editor now lists these five once the scope names a business unit, and marks each one as granted or not by the set being edited.
11+
12+
"Grant the missing ones" adds only what is missing to the same set, in one edit that the set's normal Save persists: `setup.access` joins `systemPermissions`, and each object row gains only the flags it lacks. No grant the set already carries is removed or narrowed, and no new key is written. Under a package (the Studio Access pillar) the list is shown without the action, with a note that these grants belong on an environment permission set in Setup: the package editor saves only the package's own objects.
13+
14+
What to do: nothing. A set's stored shape is unchanged.

‎content/docs/permissions/delegated-administration.mdx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export const FieldOpsDelegate = definePermissionSet({
2828
manageAssignments: true, // user ↔ position rows (sys_user_position)
2929
assignablePermissionSets: ['showcase_contributor', 'showcase_manager'],
3030
},
31+
// …the delegate has to be able to open Setup, where the assignment screens live…
32+
systemPermissions: ['setup.access'],
3133
// …and plain CRUD on the RBAC link tables lets the requests through at all.
3234
// Both are required: table CRUD with NO scope is refused outright.
3335
objects: {
@@ -43,6 +45,17 @@ export const FieldOpsDelegate = definePermissionSet({
4345
misconfigured name resolves to an empty subtree and **approves nothing**
4446
(fail-closed).
4547

48+
The scope says *what* a delegate may administer; it does not let them in. Five
49+
companion grants do that, and the example carries all of them: the
50+
`setup.access` capability, without which the delegate is turned away at the
51+
Setup app ("You don't have access to this app"); read on `sys_user`,
52+
`sys_business_unit` and `sys_business_unit_member`, so the lookups can pick
53+
people and units; and CRUD on `sys_user_position`, the assignment rows
54+
themselves. They may sit on the scope's own set or on any other set the
55+
delegate holds. In Setup's permission set editor, the Delegated Admin Scope
56+
block lists the five, marks which ones the set already grants, and adds the
57+
missing ones to the set in one click.
58+
4659
## What the runtime gate enforces
4760

4861
Writes to the governed tables (`sys_user_position`, `sys_user_permission_set`,
Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Pins the companion-grant hint of the Delegated Admin Scope facet.
5+
*
6+
* A scope says WHAT its holder may administer; it does not let the holder in.
7+
* A delegate also needs the `setup.access` capability to enter Setup, read on
8+
* `sys_user` / `sys_business_unit` / `sys_business_unit_member` so the pickers
9+
* can list people and units, and CRUD on `sys_user_position` for the
10+
* assignment rows themselves. Without them the delegate meets "You don't have
11+
* access to this app" — the server is right to refuse, and the editor said
12+
* nothing about it.
13+
*
14+
* The facet now lists those five grants, marks each as granted or not by THIS
15+
* set, and offers one action that adds only the missing ones to the same
16+
* draft — never removing or narrowing a grant the set already carries. Under
17+
* a package binding the list stays and the action does not: the package
18+
* editor saves only the package's own objects, so the grants belong on an
19+
* environment set.
20+
*
21+
* Driven through the editor with the draft held as the host holds it, so the
22+
* assertions read what the facet actually wrote.
23+
*/
24+
25+
import * as React from "react";
26+
import { describe, it, expect, afterEach } from "vitest";
27+
import { render, screen, within, cleanup } from "@testing-library/react";
28+
import userEvent from "@testing-library/user-event";
29+
import { PermissionSetSchema } from "@objectstack/spec/security";
30+
import { PermissionAdvancedFacets } from "./PermissionAdvancedFacets";
31+
32+
afterEach(cleanup);
33+
34+
const t = (k: string) => k;
35+
36+
type Draft = Record<string, unknown>;
37+
38+
let latest: Draft = {};
39+
let draftWrites = 0;
40+
41+
function Harness({
42+
initial,
43+
writable = true,
44+
only,
45+
packageBound,
46+
}: {
47+
initial: Draft;
48+
writable?: boolean;
49+
only?: "admin";
50+
packageBound?: boolean;
51+
}) {
52+
const [draft, setDraft] = React.useState<Draft>(initial);
53+
React.useEffect(() => {
54+
latest = draft;
55+
}, [draft]);
56+
const countedSetDraft = React.useCallback((updater: (prev: any) => any) => {
57+
draftWrites += 1;
58+
setDraft(updater);
59+
}, []);
60+
return (
61+
<PermissionAdvancedFacets
62+
draft={draft}
63+
setDraft={countedSetDraft}
64+
writable={writable}
65+
allSetNames={[]}
66+
t={t}
67+
only={only}
68+
packageBound={packageBound}
69+
/>
70+
);
71+
}
72+
73+
async function mount(
74+
initial: Draft,
75+
opts: { writable?: boolean; only?: "admin"; packageBound?: boolean } = {},
76+
) {
77+
const user = userEvent.setup();
78+
latest = initial;
79+
draftWrites = 0;
80+
render(<Harness initial={initial} {...opts} />);
81+
if (opts.only === undefined) {
82+
// The stacked (Setup) presentation: the section starts collapsed.
83+
await user.click(screen.getByText("perm.admin.title"));
84+
}
85+
return user;
86+
}
87+
88+
const ANCHORED = { businessUnit: "bu_field_ops", manageAssignments: true };
89+
90+
const COMPANIONS = [
91+
"setup.access",
92+
"sys_user",
93+
"sys_business_unit",
94+
"sys_business_unit_member",
95+
"sys_user_position",
96+
] as const;
97+
98+
const row = (id: string) => screen.getByTestId(`admin-companion-${id}`);
99+
const grantButton = () => screen.queryByTestId("admin-companions-grant");
100+
101+
describe("PermissionAdvancedFacets · delegated admin companion grants", () => {
102+
it("lists the five companion grants, each marked missing on an empty set", async () => {
103+
await mount({ name: "field_ops_delegate", objects: {}, adminScope: ANCHORED });
104+
105+
expect(screen.getByTestId("admin-companions")).toBeTruthy();
106+
for (const id of COMPANIONS) {
107+
expect(row(id).getAttribute("data-granted"), id).toBe("false");
108+
}
109+
expect(grantButton()).toBeTruthy();
110+
});
111+
112+
it("marks each grant this set already carries as granted", async () => {
113+
await mount({
114+
name: "field_ops_delegate",
115+
systemPermissions: ["setup.access"],
116+
objects: {
117+
sys_user: { allowRead: true },
118+
// CRUD is all four; read alone does not cover the assignment rows.
119+
sys_user_position: { allowRead: true },
120+
},
121+
adminScope: ANCHORED,
122+
});
123+
124+
expect(row("setup.access").getAttribute("data-granted")).toBe("true");
125+
expect(row("sys_user").getAttribute("data-granted")).toBe("true");
126+
expect(row("sys_business_unit").getAttribute("data-granted")).toBe("false");
127+
expect(row("sys_business_unit_member").getAttribute("data-granted")).toBe("false");
128+
expect(row("sys_user_position").getAttribute("data-granted")).toBe("false");
129+
});
130+
131+
it("is not shown until the scope names a business unit, matching the block's gate", async () => {
132+
await mount({ name: "field_ops_delegate", objects: {} });
133+
expect(screen.queryByTestId("admin-companions")).toBeNull();
134+
expect(grantButton()).toBeNull();
135+
});
136+
137+
it("one click adds only the missing grants, in one draft write, and narrows nothing", async () => {
138+
const user = await mount({
139+
name: "field_ops_delegate",
140+
systemPermissions: ["manage_org_users"],
141+
objects: {
142+
account: { allowRead: true, allowEdit: true },
143+
sys_user: { allowRead: true, allowEdit: true },
144+
sys_user_position: { allowRead: true, viewAllRecords: true },
145+
},
146+
adminScope: ANCHORED,
147+
});
148+
const before = draftWrites;
149+
150+
await user.click(grantButton() as HTMLElement);
151+
152+
expect(draftWrites - before).toBe(1);
153+
expect(latest.systemPermissions).toEqual(["manage_org_users", "setup.access"]);
154+
expect(latest.objects).toEqual({
155+
// Untouched: not a companion.
156+
account: { allowRead: true, allowEdit: true },
157+
// Untouched: already granted, and its extra Edit is kept.
158+
sys_user: { allowRead: true, allowEdit: true },
159+
// Widened to CRUD; its existing View All is kept.
160+
sys_user_position: {
161+
allowRead: true,
162+
viewAllRecords: true,
163+
allowCreate: true,
164+
allowEdit: true,
165+
allowDelete: true,
166+
},
167+
sys_business_unit: { allowRead: true },
168+
sys_business_unit_member: { allowRead: true },
169+
});
170+
// The scope itself is not touched by the grant.
171+
expect(latest.adminScope).toEqual(ANCHORED);
172+
// Every companion now reads as granted, so there is nothing left to add.
173+
for (const id of COMPANIONS) {
174+
expect(row(id).getAttribute("data-granted"), id).toBe("true");
175+
}
176+
expect(grantButton()).toBeNull();
177+
});
178+
179+
it("writes a set the spec accepts, with no key the set did not already have a slot for", async () => {
180+
const user = await mount({ name: "field_ops_delegate", objects: {}, adminScope: ANCHORED });
181+
await user.click(grantButton() as HTMLElement);
182+
183+
expect(Object.keys(latest).sort()).toEqual(
184+
["adminScope", "name", "objects", "systemPermissions"].sort(),
185+
);
186+
const verdict = PermissionSetSchema.safeParse(latest);
187+
expect(verdict.success, JSON.stringify(verdict.error?.issues)).toBe(true);
188+
});
189+
190+
it("offers no action when every grant is already on the set", async () => {
191+
await mount({
192+
name: "field_ops_delegate",
193+
systemPermissions: ["setup.access"],
194+
objects: {
195+
sys_user: { allowRead: true },
196+
sys_business_unit: { allowRead: true },
197+
sys_business_unit_member: { allowRead: true },
198+
sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
199+
},
200+
adminScope: ANCHORED,
201+
});
202+
expect(screen.getByTestId("admin-companions")).toBeTruthy();
203+
expect(grantButton()).toBeNull();
204+
});
205+
206+
it("read-only: the hint stays, the action does not", async () => {
207+
await mount({ name: "field_ops_delegate", objects: {}, adminScope: ANCHORED }, { writable: false });
208+
expect(screen.getByTestId("admin-companions")).toBeTruthy();
209+
expect(grantButton()).toBeNull();
210+
});
211+
212+
it("under a package binding: the hint stays, the action does not, and the note says where to grant", async () => {
213+
await mount(
214+
{
215+
name: "field_ops_delegate",
216+
systemPermissions: ["setup.access"],
217+
objects: {},
218+
adminScope: ANCHORED,
219+
},
220+
{ packageBound: true },
221+
);
222+
const hint = screen.getByTestId("admin-companions");
223+
// Listed, but not marked: the package door's draft holds only the
224+
// package's own objects, so a `sys_*` row absent here says nothing about
225+
// the stored set.
226+
for (const id of COMPANIONS) expect(row(id).hasAttribute("data-granted"), id).toBe(false);
227+
expect(grantButton()).toBeNull();
228+
expect(within(hint).getByText("perm.admin.companions.packageNote")).toBeTruthy();
229+
});
230+
231+
it("the Studio Access pillar mount (the package door's only host) is package-bound by default", async () => {
232+
await mount({ name: "field_ops_delegate", objects: {}, adminScope: ANCHORED }, { only: "admin" });
233+
expect(screen.getByTestId("admin-companions")).toBeTruthy();
234+
expect(grantButton()).toBeNull();
235+
});
236+
237+
it("an explicit packageBound={false} on that mount keeps the action", async () => {
238+
await mount(
239+
{ name: "field_ops_delegate", objects: {}, adminScope: ANCHORED },
240+
{ only: "admin", packageBound: false },
241+
);
242+
expect(grantButton()).toBeTruthy();
243+
});
244+
});

0 commit comments

Comments
 (0)