Skip to content

Commit 5cb1c1a

Browse files
fix(app-shell): permission matrix sets and shows readScope / writeScope (#26187) (#26284)
Part of #26187 Clause-②: no Metadata-protocol: none PR 1 of 2 for #26187: the matrix editor. PR 2 wires `PermissionPreview` read-only into the permission-set and position pages, so #26187 remains open after this merges. ## What changes - **Pickers in both renderers.** Every object row in Setup's `PermissionTable` and in Studio Access's `StudioObjectRow` gains a **Read scope** and a **Write scope** picker. Each picker is the shared `Select` that the facet pickers use (objectui#11865). It lists "Not set" plus the five `ObjectAccessScopeSchema` values in the spec's order. A pick writes the key through a new mutator, `updateObjectScope`. "Not set" deletes the key. Every other key of the row rides through. - **Hierarchy hint.** `own_and_reports`, `unit` and `unit_and_below` show the hint "requires enterprise; otherwise owner-only" on their items, and stay selectable. Per the seat's ruling, the hint is unconditional: no client signal says whether `hierarchy-security` is licensed. - **Greying.** These are the #16870 pairs, mirrored from the server and not decided by the client. View All disables the read picker. Modify All disables both pickers. Each disabled trigger carries the reason as its `title`. - **Clearing rule.** Granting View All drops `readScope`. Granting Modify All drops both scopes. One helper, `dropScopesBypassedBySuperUserBits`, applies the rule in the per-checkbox cascade and in the "All" bulk arm. On `origin/main`, both paths saved a row that the BASE schema refuses. Effective access does not change: the resolvers answer `org` on the super-user bit before they read a scope. - **Types and vocabulary.** `ObjectPerm` gains `readScope?` / `writeScope?`, typed with the spec's `ObjectAccessScope`. The boolean columns get their own type, `ObjectPermFlag`, so a checkbox can no longer address a scope key. `OBJECT_ACCESS_SCOPES` restates the five values with a type-only import. The reason is the one `useTenancyPosture` documents: the `@objectstack/spec/security` value entry costs about 237 KB of bundle. A test compares the list with `ObjectAccessScopeSchema.options`. `HIERARCHY_SCOPES` is typed with the spec's `HierarchyScope`. - **Strings.** The new strings live only in `metadata-admin/i18n.ts` (`perm.scope.*`, en + zh). The column headers reuse `engine.permissionPreview.readScope` / `writeScope`. No `ui/packages/i18n` locale pack is touched. ## How the greying covers the two bulk paths (the card's question) - **"All"** grants View All and Modify All, so it now drops both scopes, the same as the per-checkbox cascade. After the click the row holds Modify All, so both pickers are greyed. `allowExport` and any other unauthored key still ride through, as #6605 requires. `PermissionMatrixEditor.bulkMergeKeys.test.tsx` flips its two "All" pins (environment door and package door) from "keeps `readScope: 'own'`" to "drops both scopes". - **"None"** is unchanged: it replaces the row with `{}`, scopes included (the #6605 fence). No super-user bit remains, so both pickers are open and show "Not set". - **"CRUD" / "R"** reset View All and Modify All, and the scopes ride through. Those pins are kept as they were. ## Evidence All figures are from `ui/` vitest on this branch, run under `scripts/pm/os-verify-lock.sh`. - **Red first.** The three implementation files (`PermissionMatrixEditor.tsx`, `permission-slice.ts`, `i18n.ts`) were set back to `origin/main` `df8aac41` (hash-equal to the BASE blobs, 0 `perm-scope-` markers). The new and flipped suites then gave `Tests 17 failed | 5 passed (22)`. All 15 behaviour pins of `recordScope-26187` failed, and so did both "All" pins of `bulkMergeKeys`. The 5 that stayed green are expected: the BASE-schema refusal control, the "other column leaves the scopes alone" preservation pin, and the CRUD / R / None pins. The restore was proven by blob hash equal to HEAD and an empty `git diff HEAD`. - **Ablation.** The "All"-arm `dropScopesBypassedBySuperUserBits(next);` was deleted once with `scripts/ablation-replace.mjs` (anchor x1 → x0, blob `b275ac54` → `d01962e8`). `bulkMergeKeys` then gave `Tests 2 failed | 3 passed (5)`: both "All" pins failed and CRUD / R / None stayed green. Restored: blob == HEAD and `git diff HEAD` is empty. - **Round trip at both doors.** A picked value is saved, then reopened from the saved record, at the environment door (`client.save` with no draft mode). At the package door, the autosaved draft goes through `mergePermissionSlice`, and an out-of-scope row from another package comes back byte for byte. The saved rows after the View All and Modify All ticks are checked with the spec's real `ObjectPermissionSchema.safeParse`, not with a restatement of its rule. ## Tests (local, each under `scripts/pm/os-verify-lock.sh`) - `recordScope-26187`, `bulkMergeKeys` and `retiredLifecycleKeys`: `Test Files 3 passed (3)`, `Tests 26 passed (26)` at `93c9211e`. The later merge of `origin/main` (`c696351d`) touched none of the seven files in this diff. - `pnpm --filter @object-ui/app-shell type-check` (both tsc programs) at `c696351d`, after `turbo run build --filter='@object-ui/app-shell^...'` (64/64 tasks): `VERDICT command-exit 0`. - The full app-shell suite (1,740 files at `--maxWorkers=2`) is **NOT MEASURED**. I stopped my own run after about 50 minutes without a verdict, because it was holding the shared lock. Declared narrowing: the 195 app-shell test files that import the editor, `permission-slice` or the metadata-admin `i18n`, plus the Studio Access suites. That set was still queued for the lock when this PR was opened, so its result goes in the `os-dev-report` comment on #26187. CI's `Test UI` runs its affected selection over this diff. ## Gates at `c696351d` (`dispatch-gates.mjs --commands`, every command run and reconciled with `--ran`) - 35 families derived and recorded with exit codes: 34 exited 0, including `check-sdui-manifest` (0, the manifest is untouched), `check:nul-bytes`, `check:cross-package-test-inputs`, `check:test-source-alias` and `check:type-check-coverage`. - `check:type-check-debt` is **NOT MEASURED** (exit 3, `PREREQUISITE NOT MET`): its closure build timed out in the shared lock's queue. CI runs it in `Lint & Repo Gates`. - Changed lines: 666 (+624 / -42) over 7 files. ## Acceptance notes - **File surface.** `PermissionMatrixEditor.retiredLifecycleKeys.test.tsx` pinned the header strip as `slice(1, -1)`. The two new columns made that pin false, so it now reads `slice(1, -3)` and also pins the two scope headers. The claim's file surface did not list this file. - **Stored pre-#16870 pair (not changed here).** A row stored before #16870 can hold View All plus `readScope`. Such a row loads with the read picker greyed and showing the stored value. Saving it unchanged sends the pair, and the server refuses it, as it does on `origin/main`. The matrix does not strip the key on load, because that would delete stored data without the admin acting. To clear it, untick View All, set "Not set", then tick View All again. This is in the report's open questions. - **"Only granted" and the granted count (observation, not filed).** Both count a row with any truthy key as granted (`Object.values(perm).some(Boolean)`). A row that carries only a scope is therefore listed. This was already true on `origin/main` for `allowExport` and for scopes set through the API. It is noted, not filed. Carrier: none. - `scripts/sdui-manifest.*` is untouched (app-shell is outside the sdui registry closure). `check-sdui-manifest` exits 0. --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d3bdf43 commit 5cb1c1a

7 files changed

Lines changed: 624 additions & 42 deletions
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): the permission matrix sets and shows each object's read and write record scope
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
ADR-0084 declares a record-scope column for the permission matrix, but Setup → Permission Sets and Studio → Access had no control for an object permission's `readScope` / `writeScope`. The only way to set them was `PUT /api/v1/meta/permission/:name`.
11+
12+
- **Pickers.** Every object row, in both the Setup table and the Studio Access matrix, now has a **Read scope** and a **Write scope** picker. Each lists "Not set" and the five `ObjectAccessScopeSchema` values: own, own and reports, my unit, my unit and below, whole org. A picked value saves and reloads unchanged. This holds at the environment door and at the package door, where the slice merge keeps it. "Not set" removes the key.
13+
- **Hierarchy values.** "Own and reports", "My unit" and "My unit and below" carry the hint "requires enterprise; otherwise owner-only" and stay selectable. The open runtime resolves them to the owner alone. No hierarchy resolver is installed there.
14+
- **View All / Modify All.** The spec refuses `readScope` beside View All or Modify All, and `writeScope` beside Modify All, because those bits already grant org-wide access. Ticking View All now disables the read-scope picker and removes `readScope`. Ticking Modify All disables both pickers and removes both scopes. The **All** bulk button does the same. Before this change, both paths saved a row the server refuses. Effective access does not change, because the resolvers answer `org` on those bits before they read a scope. "CRUD", "R" and "None" are unchanged.
15+
16+
What to do: nothing. No spec key, stored shape or API changes.

‎ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.bulkMergeKeys.test.tsx‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,23 @@
44
* objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the
55
* matrix does not author.
66
*
7-
* `bulkSetObject` used to REPLACE the object's permission row. Three
8-
* spec-declared keys are modelled by neither the local `ObjectPerm` interface
9-
* nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
10-
* access-depth axis `readScope` / `writeScope` — so one bulk click dropped
11-
* whatever those held. The sharpest shape is the button labelled **All**: an
12-
* admin clicking what reads as "grant everything" could WIDEN effective read
13-
* access by deleting a `readScope: 'own'` narrowing, with no diff shown and no
14-
* error.
7+
* `bulkSetObject` used to REPLACE the object's permission row. Keys outside
8+
* the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057
9+
* access-depth axis `readScope` / `writeScope` (authored by the row's own
10+
* pickers since objectstack#26187, never by a bulk button) — were dropped by
11+
* one bulk click. The sharpest shape was "CRUD" or "R": an admin narrowing the
12+
* grant could WIDEN effective read access by deleting a `readScope: 'own'`
13+
* narrowing, with no diff shown and no error.
14+
*
15+
* ## "All" drops the scopes — objectstack#26187, seat ruling under 6416233738 Q1
16+
*
17+
* "All" grants View All and Modify All, and the BASE schema refuses
18+
* `readScope` beside either and `writeScope` beside Modify All (#16870): the
19+
* resolvers answer `org` on those bits before they read a scope, so the
20+
* scope changes nothing and the stored pair is refused. "All" therefore drops
21+
* both scopes, as the per-checkbox cascade does — effective access is the same
22+
* `org` either way. It still merges: `allowExport` and every other key the
23+
* matrix does not author rides through.
1524
*
1625
* Every pin here asserts the SAVED payload, never editor state. That is the
1726
* card's own argument for why the defect persisted: both save doors carry the
@@ -145,7 +154,7 @@ async function autosaved(server: FakeServer) {
145154
}
146155

147156
describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => {
148-
it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => {
157+
it('"All" grants every column, keeps allowExport, and drops readScope / writeScope — the pair the BASE schema refuses (objectstack#26187)', async () => {
149158
const server = await renderMatrix({
150159
a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' },
151160
});
@@ -155,14 +164,13 @@ describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#66
155164

156165
const row = payload.objects.a_account;
157166
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
158-
// The narrowings survive the click that used to delete them. `readScope`
159-
// is the load-bearing one: dropping `own` silently widened read access.
160-
expect(row.readScope).toBe('own');
161-
expect(row.writeScope).toBe('own');
167+
// View All + Modify All already answer `org`; a scope beside them is the
168+
// #16870 pair the server refuses, so "All" drops both. `allowExport` is a
169+
// key the matrix does not author and still rides through.
170+
expect('readScope' in row).toBe(false);
171+
expect('writeScope' in row).toBe(false);
162172
expect(row.allowExport).toBe(true);
163-
expect(Object.keys(row).sort()).toEqual(
164-
[...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(),
165-
);
173+
expect(Object.keys(row).sort()).toEqual([...MATRIX_KEYS, 'allowExport'].sort());
166174
});
167175

168176
it('"CRUD" merges the unmodelled keys through but still RESETS the matrix columns outside its grant', async () => {
@@ -234,7 +242,7 @@ describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#66
234242
expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' });
235243
});
236244

237-
it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => {
245+
it('package door: the merged slice keeps allowExport and drops the scopes after "All", and other packages\' rows survive byte-for-byte', async () => {
238246
// In-scope: a_account (this package's row, carrying the narrowings).
239247
// Out-of-scope: b_order — another package's contribution, not listed by
240248
// this package, which `mergePermissionSlice` must copy verbatim from base.
@@ -251,8 +259,9 @@ describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#66
251259

252260
const row = payload.objects.a_account;
253261
for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true);
254-
expect(row.readScope).toBe('own');
255-
expect(row.writeScope).toBe('own');
262+
// objectstack#26187 — the scopes go with "All" at this door too.
263+
expect('readScope' in row).toBe(false);
264+
expect('writeScope' in row).toBe(false);
256265
expect(row.allowExport).toBe(true);
257266
// The other package's row is untouched — and the save went through the
258267
// package door (a draft write), not a live record write.

0 commit comments

Comments
 (0)