From e4f9cb7a1488683e325c5f9b05d02ce1179d5523 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 17:05:34 +0000 Subject: [PATCH 1/2] fix(app-shell): permission matrix sets and shows readScope / writeScope (#26187) Every object row in Setup's PermissionTable and Studio Access's StudioObjectRow gains a read-scope and a write-scope picker over ObjectAccessScopeSchema's values. The three hierarchy values carry the enterprise hint, View All greys the read picker, and Modify All greys both. Granting View All or Modify All drops the scope the BASE schema refuses beside it, in the per-checkbox cascade and in the All bulk arm. Claude-Session: https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup Co-authored-by: Claude --- .../26187-permission-matrix-record-scope.md | 16 + ...missionMatrixEditor.bulkMergeKeys.test.tsx | 47 +-- ...ionMatrixEditor.recordScope-26187.test.tsx | 314 ++++++++++++++++++ .../metadata-admin/PermissionMatrixEditor.tsx | 211 ++++++++++-- .../src/views/metadata-admin/i18n.ts | 24 ++ .../views/metadata-admin/permission-slice.ts | 48 +++ 6 files changed, 620 insertions(+), 40 deletions(-) create mode 100644 .changeset/26187-permission-matrix-record-scope.md create mode 100644 ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.recordScope-26187.test.tsx diff --git a/.changeset/26187-permission-matrix-record-scope.md b/.changeset/26187-permission-matrix-record-scope.md new file mode 100644 index 00000000000..e9167bd5988 --- /dev/null +++ b/.changeset/26187-permission-matrix-record-scope.md @@ -0,0 +1,16 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): the permission matrix sets and shows each object's read and write record scope + +Clause-②: no +Metadata-protocol: none + +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`. + +- **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. +- **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. +- **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. + +What to do: nothing. No spec key, stored shape or API changes. diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.bulkMergeKeys.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.bulkMergeKeys.test.tsx index edd45b993a2..906d6fe22bb 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.bulkMergeKeys.test.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.bulkMergeKeys.test.tsx @@ -4,14 +4,23 @@ * objectui#6605 — the bulk buttons (R / CRUD / All / None) and the keys the * matrix does not author. * - * `bulkSetObject` used to REPLACE the object's permission row. Three - * spec-declared keys are modelled by neither the local `ObjectPerm` interface - * nor the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057 - * access-depth axis `readScope` / `writeScope` — so one bulk click dropped - * whatever those held. The sharpest shape is the button labelled **All**: an - * admin clicking what reads as "grant everything" could WIDEN effective read - * access by deleting a `readScope: 'own'` narrowing, with no diff shown and no - * error. + * `bulkSetObject` used to REPLACE the object's permission row. Keys outside + * the `OBJECT_ACTIONS` column list — `allowExport`, and the ADR-0057 + * access-depth axis `readScope` / `writeScope` (authored by the row's own + * pickers since objectstack#26187, never by a bulk button) — were dropped by + * one bulk click. The sharpest shape was "CRUD" or "R": an admin narrowing the + * grant could WIDEN effective read access by deleting a `readScope: 'own'` + * narrowing, with no diff shown and no error. + * + * ## "All" drops the scopes — objectstack#26187, seat ruling under 6416233738 Q1 + * + * "All" grants View All and Modify All, and the BASE schema refuses + * `readScope` beside either and `writeScope` beside Modify All (#16870): the + * resolvers answer `org` on those bits before they read a scope, so the + * scope changes nothing and the stored pair is refused. "All" therefore drops + * both scopes, as the per-checkbox cascade does — effective access is the same + * `org` either way. It still merges: `allowExport` and every other key the + * matrix does not author rides through. * * Every pin here asserts the SAVED payload, never editor state. That is the * card's own argument for why the defect persisted: both save doors carry the @@ -145,7 +154,7 @@ async function autosaved(server: FakeServer) { } describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#6605)', () => { - it('"All" — the widening shape — grants every column AND the saved row keeps readScope / writeScope / allowExport', async () => { + it('"All" grants every column, keeps allowExport, and drops readScope / writeScope — the pair the BASE schema refuses (objectstack#26187)', async () => { const server = await renderMatrix({ a_account: { allowRead: true, allowExport: true, readScope: 'own', writeScope: 'own' }, }); @@ -155,14 +164,13 @@ describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#66 const row = payload.objects.a_account; for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true); - // The narrowings survive the click that used to delete them. `readScope` - // is the load-bearing one: dropping `own` silently widened read access. - expect(row.readScope).toBe('own'); - expect(row.writeScope).toBe('own'); + // View All + Modify All already answer `org`; a scope beside them is the + // #16870 pair the server refuses, so "All" drops both. `allowExport` is a + // key the matrix does not author and still rides through. + expect('readScope' in row).toBe(false); + expect('writeScope' in row).toBe(false); expect(row.allowExport).toBe(true); - expect(Object.keys(row).sort()).toEqual( - [...MATRIX_KEYS, 'allowExport', 'readScope', 'writeScope'].sort(), - ); + expect(Object.keys(row).sort()).toEqual([...MATRIX_KEYS, 'allowExport'].sort()); }); 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 expect(payload.objects.a_contact).toEqual({ allowRead: true, readScope: 'own' }); }); - it('package door: the merged slice keeps the unmodelled keys after "All", and other packages\' rows survive byte-for-byte', async () => { + it('package door: the merged slice keeps allowExport and drops the scopes after "All", and other packages\' rows survive byte-for-byte', async () => { // In-scope: a_account (this package's row, carrying the narrowings). // Out-of-scope: b_order — another package's contribution, not listed by // this package, which `mergePermissionSlice` must copy verbatim from base. @@ -251,8 +259,9 @@ describe('PermissionMatrixEditor · bulk buttons vs unmodelled keys (objectui#66 const row = payload.objects.a_account; for (const key of MATRIX_KEYS) expect(row[key], key).toBe(true); - expect(row.readScope).toBe('own'); - expect(row.writeScope).toBe('own'); + // objectstack#26187 — the scopes go with "All" at this door too. + expect('readScope' in row).toBe(false); + expect('writeScope' in row).toBe(false); expect(row.allowExport).toBe(true); // The other package's row is untouched — and the save went through the // package door (a draft write), not a live record write. diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.recordScope-26187.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.recordScope-26187.test.tsx new file mode 100644 index 00000000000..98838fc423d --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.recordScope-26187.test.tsx @@ -0,0 +1,314 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#26187 — the permission matrix authors and shows the record scope + * (`readScope` / `writeScope`, ADR-0057 D1) that ADR-0084 declares for it. + * + * What is pinned, in both renderers (Setup's `PermissionTable` and Studio + * Access's `StudioObjectRow`): + * + * - every object row carries a Read scope and a Write scope picker, listing + * "Not set" and the five `ObjectAccessScopeSchema` values in the spec's + * order, and showing what the stored row holds; + * - the three hierarchy values carry the "requires enterprise; otherwise + * owner-only" hint and stay selectable; `own` and `org` carry none; + * - the #16870 pairs the BASE schema refuses grey the pickers: View All + * disables the read picker, Modify All disables both; + * - ticking View All drops `readScope` and ticking Modify All drops both, so + * the per-checkbox cascade never saves a refused pair — read through the + * spec's real `ObjectPermissionSchema`, not a restatement of its rule; + * - a picked value saves and reloads unchanged at the environment door AND at + * the package door (`mergePermissionSlice`), and "Not set" deletes the key. + * + * The bulk "All" arm's half of the clearing rule is pinned beside the other + * bulk arms, in `PermissionMatrixEditor.bulkMergeKeys.test.tsx`. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import { ObjectAccessScopeSchema, ObjectPermissionSchema } from '@objectstack/spec/security'; +import * as slice from './permission-slice'; + +interface FakeServer { + set: Record; + objectNames: string[]; + saved: Record | null; + savedOpts: Record | undefined; +} + +function makeClient(server: FakeServer) { + return { + layered: async () => ({ effective: server.set, code: null, overlay: null, overlayScope: null }), + getDraft: async () => null, + list: async (type: string) => + type === 'object' ? server.objectNames.map((name) => ({ item: { name } })) : [], + get: async (type: string) => (type === 'object' ? { fields: [] } : null), + save: async (_t: string, _n: string, payload: Record, opts?: Record) => { + server.saved = payload; + server.savedOpts = opts; + server.set = payload; // the reopen reads what was saved + return payload; + }, + } as any; +} + +let clientImpl: any; + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => clientImpl, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'permission', label: 'Permission', allowEnvironmentOverlay: true }], + }), +})); +vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null })); +vi.mock('@object-ui/fields', async (importOriginal) => ({ + ...(await importOriginal()), + CapabilityMultiSelectField: () =>
, + parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []), +})); + +import { PermissionMatrixEditPage } from './PermissionMatrixEditor'; + +afterEach(cleanup); + +type Site = 'page' | 'studio'; + +function newServer(objects: Record, objectNames?: string[]): FakeServer { + return { + set: { name: 'sales_perms', label: 'Sales', objects, fields: {} }, + objectNames: objectNames ?? Object.keys(objects), + saved: null, + savedOpts: undefined, + }; +} + +async function mount(server: FakeServer, site: Site, packageId?: string) { + clientImpl = makeClient(server); + const utils = render( + + {site === 'studio' ? ( + {} }} + /> + ) : ( + + )} + , + ); + await screen.findByText(server.objectNames[0]); + return utils; +} + +const readPicker = (object: string) => screen.getByTestId(`perm-scope-read-${object}`); +const writePicker = (object: string) => screen.getByTestId(`perm-scope-write-${object}`); + +const HINT = 'requires enterprise; otherwise owner-only'; +/** The picker's options as listed, in order, each read as `label` (hint stripped) and whether it carries the hint. */ +async function openPicker(trigger: HTMLElement) { + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + const listbox = await screen.findByRole('listbox'); + return within(listbox).getAllByRole('option'); +} + +async function pick(trigger: HTMLElement, label: string) { + const options = await openPicker(trigger); + const option = options.find((o) => (o.textContent ?? '').startsWith(label)); + if (!option) throw new Error(`no "${label}": ${options.map((o) => o.textContent).join(' | ')}`); + fireEvent.click(option); +} + +/** The checkbox for one matrix column inside one object's row (`aria-label` is ` `). */ +function columnBox(object: string, tip: RegExp) { + return screen + .getAllByRole('checkbox') + .find((el) => el.getAttribute('aria-label')?.startsWith(`${object} `) && tip.test(el.getAttribute('aria-label') ?? ''))!; +} + +async function saveEnv(server: FakeServer) { + fireEvent.click(screen.getByRole('button', { name: /^Save$/ })); + await waitFor(() => expect(server.saved).not.toBeNull()); + return server.saved!; +} + +async function autosaved(server: FakeServer) { + await waitFor(() => expect(server.saved).not.toBeNull(), { timeout: 4000 }); + return server.saved!; +} + +/** The spec's own verdict on a saved row — the server-side #16870 refusal, not a restatement of it. */ +function baseSchemaAccepts(row: unknown) { + const r = ObjectPermissionSchema.safeParse(row); + return r.success ? 'accepted' : r.error.issues.map((i) => `${i.path.join('.')}: ${i.code}`).join('; '); +} + +describe('the scope vocabulary is the spec’s (objectstack#26187)', () => { + it('OBJECT_ACCESS_SCOPES lists ObjectAccessScopeSchema’s values in the spec’s order', () => { + expect((slice as Record).OBJECT_ACCESS_SCOPES).toEqual(ObjectAccessScopeSchema.options); + }); + + it('the hierarchy subset is exactly the three values that need the enterprise resolver', () => { + expect([...((slice as Record).HIERARCHY_SCOPES as string[])].sort()).toEqual( + ['own_and_reports', 'unit', 'unit_and_below'], + ); + }); +}); + +describe.each(['page', 'studio'])('record scope pickers (%s renderer)', (site) => { + it('every object row has a Read scope and a Write scope picker showing the stored row', async () => { + await mount(newServer({ a_account: { allowRead: true, readScope: 'unit', writeScope: 'own' }, a_contact: { allowRead: true } }), site); + + expect(readPicker('a_account').textContent).toBe('My unit'); + expect(writePicker('a_account').textContent).toBe('Own'); + expect(readPicker('a_contact').textContent).toBe('Not set'); + expect(writePicker('a_contact').textContent).toBe('Not set'); + }); + + it('lists "Not set" and the five spec values in order; only the three hierarchy values carry the hint', async () => { + await mount(newServer({ a_account: { allowRead: true } }), site); + + const options = await openPicker(readPicker('a_account')); + const listed = options.map((o) => ({ text: o.textContent ?? '', hint: (o.textContent ?? '').includes(HINT) })); + expect(listed.map((o) => o.text.replace(HINT, '').replace(/[\s·]+$/, ''))).toEqual([ + 'Not set', + 'Own', + 'Own and reports', + 'My unit', + 'My unit and below', + 'Whole org', + ]); + expect(listed.map((o) => o.hint)).toEqual([false, false, true, true, true, false]); + // Selectable: none of them is disabled, licensed or not. + for (const o of options) expect(o).not.toHaveAttribute('data-disabled'); + }); + + it('View All greys the read picker only; Modify All greys both — with the reason as a title', async () => { + await mount( + newServer({ + a_account: { allowRead: true, viewAllRecords: true, writeScope: 'own' }, + a_contact: { allowRead: true, allowEdit: true, modifyAllRecords: true }, + a_lead: { allowRead: true }, + }), + site, + ); + + expect(readPicker('a_account')).toBeDisabled(); + expect(readPicker('a_account').getAttribute('title')).toMatch(/View All/); + expect(writePicker('a_account')).toBeEnabled(); + expect(readPicker('a_contact')).toBeDisabled(); + expect(writePicker('a_contact')).toBeDisabled(); + expect(writePicker('a_contact').getAttribute('title')).toMatch(/Modify All/); + // Control: a row with neither bit leaves both pickers open. + expect(readPicker('a_lead')).toBeEnabled(); + expect(writePicker('a_lead')).toBeEnabled(); + }); + + it('a hierarchy value is selectable and saves (package door), the other package’s row untouched', async () => { + const server = newServer( + { a_account: { allowRead: true }, b_order: { allowRead: true, readScope: 'unit' } }, + ['a_account'], + ); + await mount(server, site, 'app.a'); + + await pick(readPicker('a_account'), 'Own and reports'); + const payload = await autosaved(server); + expect(payload.objects.a_account).toEqual({ allowRead: true, readScope: 'own_and_reports' }); + expect(payload.objects.b_order).toEqual({ allowRead: true, readScope: 'unit' }); + expect(server.savedOpts).toMatchObject({ mode: 'draft', packageId: 'app.a' }); + }); +}); + +describe('the clearing rule on the per-checkbox cascade (#16870 pairs)', () => { + it('ticking View All drops readScope and keeps writeScope; the saved row is one the BASE schema accepts', async () => { + const server = newServer({ a_account: { allowRead: true, readScope: 'unit', writeScope: 'own' } }); + await mount(server, 'page'); + + fireEvent.click(columnBox('a_account', /View All/)); + const row = (await saveEnv(server)).objects.a_account; + expect(row).toEqual({ allowRead: true, viewAllRecords: true, writeScope: 'own' }); + expect(baseSchemaAccepts(row)).toBe('accepted'); + }); + + it('ticking Modify All drops both scopes; the saved row is one the BASE schema accepts', async () => { + const server = newServer({ a_account: { allowRead: true, readScope: 'unit', writeScope: 'own' } }); + await mount(server, 'page'); + + fireEvent.click(columnBox('a_account', /Modify All/)); + const row = (await saveEnv(server)).objects.a_account; + expect(row).toEqual({ allowRead: true, allowEdit: true, modifyAllRecords: true }); + expect(baseSchemaAccepts(row)).toBe('accepted'); + }); + + it('control: the same row with its scopes kept beside Modify All is what the BASE schema refuses', () => { + expect( + baseSchemaAccepts({ allowRead: true, allowEdit: true, modifyAllRecords: true, readScope: 'unit', writeScope: 'own' }), + ).not.toBe('accepted'); + }); + + it('a tick on any other column leaves both scopes alone', async () => { + const server = newServer({ a_account: { allowRead: true, readScope: 'unit', writeScope: 'own' } }); + await mount(server, 'page'); + + fireEvent.click(columnBox('a_account', /Create/)); + const row = (await saveEnv(server)).objects.a_account; + expect(row).toEqual({ allowRead: true, allowCreate: true, readScope: 'unit', writeScope: 'own' }); + }); +}); + +describe('save round-trip', () => { + it('environment door: picked values save, and a reopen shows them unchanged', async () => { + const server = newServer({ a_account: { allowRead: true, allowEdit: true, allowExport: true } }); + const { unmount } = await mount(server, 'page'); + + await pick(readPicker('a_account'), 'My unit and below'); + await pick(writePicker('a_account'), 'Own'); + const row = (await saveEnv(server)).objects.a_account; + expect(row).toEqual({ allowRead: true, allowEdit: true, allowExport: true, readScope: 'unit_and_below', writeScope: 'own' }); + expect(server.savedOpts?.mode).not.toBe('draft'); + + unmount(); + server.saved = null; + await mount(server, 'page'); + expect(readPicker('a_account').textContent).toBe('My unit and below'); + expect(writePicker('a_account').textContent).toBe('Own'); + }); + + it('environment door: "Not set" deletes the key rather than saving a blank', async () => { + const server = newServer({ a_account: { allowRead: true, readScope: 'org', writeScope: 'own' } }); + await mount(server, 'page'); + + await pick(readPicker('a_account'), 'Not set'); + const row = (await saveEnv(server)).objects.a_account; + expect(row).toEqual({ allowRead: true, writeScope: 'own' }); + expect('readScope' in row).toBe(false); + }); + + it('package door: a picked value survives mergePermissionSlice and reloads unchanged', async () => { + const server = newServer( + { + a_account: { allowRead: true, allowEdit: true }, + b_order: { allowRead: true, viewAllRecords: true, writeScope: 'unit' }, + }, + ['a_account'], + ); + const { unmount } = await mount(server, 'page', 'app.a'); + + await pick(writePicker('a_account'), 'Whole org'); + const payload = await autosaved(server); + expect(payload.objects.a_account).toEqual({ allowRead: true, allowEdit: true, writeScope: 'org' }); + expect(payload.objects.b_order).toEqual({ allowRead: true, viewAllRecords: true, writeScope: 'unit' }); + + unmount(); + server.saved = null; + await mount(server, 'page', 'app.a'); + expect(writePicker('a_account').textContent).toBe('Whole org'); + expect(readPicker('a_account').textContent).toBe('Not set'); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx index 8a000e12583..b275ac54950 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx @@ -73,6 +73,13 @@ import { DialogHeader, DialogTitle, } from '@object-ui/components'; +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@object-ui/components'; import { Sheet, SheetContent, @@ -113,10 +120,14 @@ import type { PermissionSetCloned } from './permission-set-clone.js'; import { mergePermissionSlice, scopePermissionSet, + OBJECT_ACCESS_SCOPES, + isHierarchyScope, type ObjectPerm, + type ObjectPermFlag, type FieldPerm, type PermissionSetDraft, } from './permission-slice.js'; +import type { ObjectAccessScope } from '@objectstack/spec/security'; /** * objectui#13113 — a refused save, in the author's words where the refusal @@ -278,9 +289,108 @@ function withFieldBits(cur: FieldPerm | undefined, readable: boolean, editable: return next; } +/** + * objectstack#26187 — the #16870 pairs, in the direction the matrix needs: a + * super-user bit already answers `org` before the resolver reads a scope, so + * the BASE schema (`checkScopeAgainstSuperUserBits`) refuses `readScope` + * beside View All or Modify All, and `writeScope` beside Modify All. Applied + * where the matrix GRANTS a bit (the per-checkbox cascade and the "All" arm), + * so no save from the matrix carries a refused pair. Mutates `row`. + */ +function dropScopesBypassedBySuperUserBits(row: ObjectPerm): void { + if (row.viewAllRecords === true || row.modifyAllRecords === true) delete row.readScope; + if (row.modifyAllRecords === true) delete row.writeScope; +} + +/** + * The same pairs, read for the pickers: the catalogue row naming the bit that + * greys this axis's picker, or `null` when the picker is open. The UI mirrors + * the server's refusal; it does not decide it. + */ +function scopeLockKey(perm: ObjectPerm, axis: ScopeAxis): string | null { + if (perm.modifyAllRecords === true) return 'perm.scope.lockedByModifyAll'; + if (axis === 'readScope' && perm.viewAllRecords === true) return 'perm.scope.lockedByViewAll'; + return null; +} + +type ScopeAxis = 'readScope' | 'writeScope'; + +/** The two scope columns, in column order: the catalogue row each header reads (shared with the preview). */ +const SCOPE_AXES: ReadonlyArray<{ axis: ScopeAxis; testId: string; labelKey: string; tipKey: string }> = [ + { axis: 'readScope', testId: 'read', labelKey: 'engine.permissionPreview.readScope', tipKey: 'perm.scope.readTip' }, + { axis: 'writeScope', testId: 'write', labelKey: 'engine.permissionPreview.writeScope', tipKey: 'perm.scope.writeTip' }, +]; + +/** Item tokens that are not scope values: `SelectItem` refuses `''`, so "Not set" needs a token of its own. */ +const SCOPE_UNSET = 'unset'; +const SCOPE_OUTSIDE = 'outside'; + +/** + * objectstack#26187 — one row's read or write scope, drawn with the shared + * `Select` the facet pickers use (objectui#11865). Lists "Not set" and the + * spec's five values; the three hierarchy values say they need the enterprise + * resolver and stay selectable (the open runtime resolves them to the owner + * alone). A stored value outside the spec's list is shown as stored, and + * picking it writes nothing. Disabled where the row is read-only or a + * super-user bit already decides this axis ({@link scopeLockKey}), with the + * reason as the trigger's title. + */ +function ScopePicker({ + objectName, + perm, + axis, + writable, + t, + onChange, +}: { + objectName: string; + perm: ObjectPerm; + axis: (typeof SCOPE_AXES)[number]; + writable: boolean; + t: (key: string) => string; + onChange: (objectName: string, axis: ScopeAxis, value: ObjectAccessScope | undefined) => void; +}) { + const value: string | undefined = perm[axis.axis]; + const known = value === undefined || (OBJECT_ACCESS_SCOPES as readonly string[]).includes(value); + const label = (v: string | undefined) => (v === undefined ? t('perm.scope.unset') : t(`perm.scope.value.${v}`)); + const hint = t('perm.scope.hierarchyHint'); + const lock = scopeLockKey(perm, axis.axis); + const title = lock ? t(lock) : isHierarchyScope(value) ? `${t(axis.tipKey)} (${hint})` : t(axis.tipKey); + return ( + + ); +} + function getObjectActions( locale: string, -): Array<{ key: keyof ObjectPerm; short: string; tip: string }> { +): Array<{ key: ObjectPermFlag; short: string; tip: string }> { return [ { key: 'allowCreate', short: 'C', tip: translate('perm.action.create', locale) }, { key: 'allowRead', short: 'R', tip: translate('perm.action.read', locale) }, @@ -961,7 +1071,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, } /* ── Mutators ───────────────────────────────────────────── */ - function updateObjectPerm(objectName: string, action: keyof ObjectPerm, value: boolean) { + function updateObjectPerm(objectName: string, action: ObjectPermFlag, value: boolean) { setDraft((prev) => { const cur = prev.objects[objectName] ?? {}; const nextObj: ObjectPerm = { ...cur, [action]: value }; @@ -971,6 +1081,12 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, nextObj.allowEdit = true; nextObj.allowRead = true; } + // objectstack#26187 — granting a super-user bit drops the scope it + // already overrides (the #16870 pairs), so the row never holds a pair + // the BASE schema refuses. + if (value && (action === 'viewAllRecords' || action === 'modifyAllRecords')) { + dropScopesBypassedBySuperUserBits(nextObj); + } return { ...prev, objects: { ...prev.objects, [objectName]: nextObj }, @@ -978,11 +1094,29 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, }); } + /** + * objectstack#26187 — a row's read or write scope from its picker. `undefined` + * ("Not set") deletes the key, so the row reads exactly as one never given a + * scope. Every other key of the row rides through, as in `updateObjectPerm`. + */ + function updateObjectScope( + objectName: string, + axis: ScopeAxis, + value: ObjectAccessScope | undefined, + ) { + setDraft((prev) => { + const next: ObjectPerm = { ...(prev.objects[objectName] ?? {}) }; + if (value === undefined) delete next[axis]; + else next[axis] = value; + return { ...prev, objects: { ...prev.objects, [objectName]: next } }; + }); + } + function bulkSetObject(objectName: string, action: 'all' | 'none' | 'crud' | 'read') { setDraft((prev) => { // `none` REPLACES the row with `{}` — deliberately, unlike the three // granting arms below (#6605). The defect those arms had was a GRANT - // that silently dropped a narrowing: "All" deleting a `readScope: 'own'` + // that silently dropped a narrowing: "CRUD" deleting a `readScope: 'own'` // widens effective read access with no diff and no error. `none` grants // nothing, so nothing survives for a scope to narrow; merging here would // instead leave `allowExport: true` (and the scopes) alive after a click @@ -994,25 +1128,30 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, return { ...prev, objects: { ...prev.objects, [objectName]: {} } }; } // The granting arms MERGE (#6605): start from the current row, reset the - // keys this matrix authors (`OBJECT_ACTIONS`), then set the granted - // ones. Keys the matrix does not model — `allowExport`, `readScope`, - // `writeScope`, anything an older or newer editor wrote — ride through - // exactly as they do on the per-checkbox path (`updateObjectPerm`'s - // spread). Replacing the row wholesale is what silently deleted them, - // and both save doors persist the row as-is: the environment door writes - // the whole record, and at package scope `mergePermissionSlice` takes - // in-scope rows entirely from `edited` (ADR-0086 P0), so `base` cannot - // restore what a bulk click dropped. + // boolean columns this matrix authors (`OBJECT_ACTIONS`), then set the + // granted ones. Keys the matrix does not model — `allowExport`, anything + // an older or newer editor wrote — ride through exactly as they do on + // the per-checkbox path (`updateObjectPerm`'s spread), and so do the + // scopes on "CRUD" and "R". Replacing the row wholesale is what silently + // deleted them, and both save doors persist the row as-is: the + // environment door writes the whole record, and at package scope + // `mergePermissionSlice` takes in-scope rows entirely from `edited` + // (ADR-0086 P0), so `base` cannot restore what a bulk click dropped. const cur = prev.objects[objectName] ?? {}; const next: ObjectPerm = { ...cur }; for (const a of OBJECT_ACTIONS) delete next[a.key]; - const grants: Array = + const grants: ObjectPermFlag[] = action === 'all' ? OBJECT_ACTIONS.map((a) => a.key) : action === 'crud' ? ['allowCreate', 'allowRead', 'allowEdit', 'allowDelete'] : ['allowRead']; for (const key of grants) next[key] = true; + // objectstack#26187 — "All" grants View All and Modify All, which the + // BASE schema refuses beside either scope (#16870): the same clearing + // rule as the per-checkbox cascade. Effective access is unchanged — the + // resolvers answer `org` on the super-user bits before reading a scope. + dropScopesBypassedBySuperUserBits(next); return { ...prev, objects: { ...prev.objects, [objectName]: next }, @@ -1892,6 +2031,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, t={t} onToggleExpand={toggleExpand} onObjectPerm={updateObjectPerm} + onObjectScope={updateObjectScope} onFieldPerm={updateFieldPerm} onBulkSet={bulkSetObject} onFieldBulk={bulkSetFields} @@ -2070,6 +2210,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, t={t} onToggleExpand={toggleExpand} onObjectPerm={updateObjectPerm} + onObjectScope={updateObjectScope} onFieldPerm={updateFieldPerm} onBulkSet={bulkSetObject} onFieldBulk={bulkSetFields} @@ -2119,7 +2260,9 @@ interface PermissionTableProps { objectActions: ReturnType; t: (key: string) => string; onToggleExpand: (objectName: string) => void; - onObjectPerm: (objectName: string, action: keyof ObjectPerm, value: boolean) => void; + onObjectPerm: (objectName: string, action: ObjectPermFlag, value: boolean) => void; + /** objectstack#26187 — a row's read / write scope; `undefined` clears it. */ + onObjectScope: (objectName: string, axis: ScopeAxis, value: ObjectAccessScope | undefined) => void; onFieldPerm: (objectName: string, fieldName: string, action: keyof FieldPerm, value: boolean) => void; onBulkSet: (objectName: string, action: 'all' | 'none' | 'crud' | 'read') => void; /** objectui#2600 B4 — field-level bulk over the filtered field set. */ @@ -2142,6 +2285,7 @@ function PermissionTable({ t, onToggleExpand, onObjectPerm, + onObjectScope, onFieldPerm, onBulkSet, onFieldBulk, @@ -2155,7 +2299,8 @@ function PermissionTable({ // squishing the CRUD grid and clipping the Bulk column off the right edge. // The min-width is deliberately unchanged by the two columns objectui#6595 // retired: it is a floor, so the grid simply has more room to breathe. - + // objectstack#26187 adds the two scope pickers (~2 × 9rem) to the floor. +
@@ -2168,13 +2313,18 @@ function PermissionTable({ {a.short} ))} + {SCOPE_AXES.map((s) => ( + + ))} {objects.length === 0 && ( - @@ -2247,6 +2397,11 @@ function PermissionTable({ /> ))} + {SCOPE_AXES.map((s) => ( + + ))} -
{t('perm.col.object')} + {t(s.labelKey)} + {t('perm.col.bulk')}
+ {t('perm.filter.empty')}
+ +
+ a.key === STUDIO_SHARING_GROUP_START); const divider = (i: number) => (i === 0 || i === sharingStart ? 'border-l' : ''); return ( - +
@@ -2425,6 +2581,11 @@ function StudioObjectMatrix({ {a.short} ))} + {SCOPE_AXES.map((s, i) => ( + + ))} @@ -2433,7 +2594,7 @@ function StudioObjectMatrix({ {objects.length === 0 && ( - @@ -2452,6 +2613,7 @@ function StudioObjectMatrix({ t={t} onToggleExpand={onToggleExpand} onObjectPerm={onObjectPerm} + onObjectScope={onObjectScope} onFieldPerm={onFieldPerm} onBulkSet={onBulkSet} onFieldBulk={onFieldBulk} @@ -2477,6 +2639,7 @@ function StudioObjectRow({ t, onToggleExpand, onObjectPerm, + onObjectScope, onFieldPerm, onBulkSet, onFieldBulk, @@ -2494,7 +2657,8 @@ function StudioObjectRow({ divider: (i: number) => string; t: (key: string) => string; onToggleExpand: (objectName: string) => void; - onObjectPerm: (objectName: string, action: keyof ObjectPerm, value: boolean) => void; + onObjectPerm: (objectName: string, action: ObjectPermFlag, value: boolean) => void; + onObjectScope: (objectName: string, axis: ScopeAxis, value: ObjectAccessScope | undefined) => void; onFieldPerm: (objectName: string, fieldName: string, action: keyof FieldPerm, value: boolean) => void; onBulkSet: (objectName: string, action: 'all' | 'none' | 'crud' | 'read') => void; onFieldBulk: (objectName: string, action: 'readable' | 'writable' | 'clear', fieldNames: string[]) => void; @@ -2591,6 +2755,11 @@ function StudioObjectRow({ /> ))} + {SCOPE_AXES.map((s, i) => ( + + ))} {open && ( -
{t('perm.col.object')} + {t(s.labelKey)} + {t('perm.col.bulk')}
+ {t('perm.filter.empty')}
+ + {active && ( @@ -2611,7 +2780,7 @@ function StudioObjectRow({
+ = { 'perm.bulk.crud': 'CRUD', 'perm.bulk.all': 'All', 'perm.bulk.none': 'None', + // objectstack#26187 — the record-scope pickers (ADR-0057 D1 access depth). + 'perm.scope.unset': 'Not set', + 'perm.scope.value.own': 'Own', + 'perm.scope.value.own_and_reports': 'Own and reports', + 'perm.scope.value.unit': 'My unit', + 'perm.scope.value.unit_and_below': 'My unit and below', + 'perm.scope.value.org': 'Whole org', + 'perm.scope.hierarchyHint': 'requires enterprise; otherwise owner-only', + 'perm.scope.readTip': 'Read scope: which records this set’s read reaches, by where the user sits in the org.', + 'perm.scope.writeTip': 'Write scope: which records this set’s edit and delete reach, by where the user sits in the org.', + 'perm.scope.lockedByViewAll': 'View All Records already grants org-wide read, so no read scope applies. Untick View All to set one.', + 'perm.scope.lockedByModifyAll': 'Modify All Records already grants org-wide read and write, so no scope applies. Untick Modify All to set one.', 'perm.filter.placeholder': 'Filter objects…', 'perm.filter.onlyGranted': 'Only granted', 'perm.filter.empty': 'No objects match the filter.', @@ -6200,6 +6212,18 @@ const ENGINE_STRINGS_ZH: Record = { 'perm.bulk.crud': '增改删', 'perm.bulk.all': '全选', 'perm.bulk.none': '清空', + // objectstack#26187 — 记录范围选择器（ADR-0057 D1 访问深度）。 + 'perm.scope.unset': '未设置', + 'perm.scope.value.own': '本人', + 'perm.scope.value.own_and_reports': '本人及下属', + 'perm.scope.value.unit': '本部门', + 'perm.scope.value.unit_and_below': '本部门及下级', + 'perm.scope.value.org': '全组织', + 'perm.scope.hierarchyHint': '需要企业版；否则仅限本人', + 'perm.scope.readTip': '读取范围：本权限集的读取按用户在组织中的位置覆盖哪些记录。', + 'perm.scope.writeTip': '写入范围：本权限集的编辑与删除按用户在组织中的位置覆盖哪些记录。', + 'perm.scope.lockedByViewAll': '“查看所有记录”已授予全组织读取，读取范围不再适用。取消勾选“查看所有记录”后可设置。', + 'perm.scope.lockedByModifyAll': '“修改所有记录”已授予全组织读写，范围不再适用。取消勾选“修改所有记录”后可设置。', 'perm.filter.placeholder': '筛选对象…', 'perm.filter.onlyGranted': '只看已授权', 'perm.filter.empty': '没有匹配的对象。', diff --git a/ui/packages/app-shell/src/views/metadata-admin/permission-slice.ts b/ui/packages/app-shell/src/views/metadata-admin/permission-slice.ts index b6415d57105..6fe03296565 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/permission-slice.ts +++ b/ui/packages/app-shell/src/views/metadata-admin/permission-slice.ts @@ -18,6 +18,9 @@ * ({@link EDITOR_AUTHORED_KEYS}). */ +import type { HierarchyScope } from '@objectstack/spec/contracts'; +import type { ObjectAccessScope } from '@objectstack/spec/security'; + export interface ObjectPerm { allowCreate?: boolean; allowRead?: boolean; @@ -48,6 +51,51 @@ export interface ObjectPerm { // the `@objectstack/spec` bump that lands the retirement. viewAllRecords?: boolean; modifyAllRecords?: boolean; + /** + * objectstack#26187 — the record-scope (access DEPTH, ADR-0057 D1) columns + * ADR-0084 declares for the matrix. Authored by the row's two pickers. The + * BASE schema refuses `readScope` beside either super-user bit and + * `writeScope` beside `modifyAllRecords` (#16870), so the matrix greys the + * pickers on those bits and drops the scope when it grants one. + */ + readScope?: ObjectAccessScope; + writeScope?: ObjectAccessScope; +} + +/** The boolean columns of a row — every {@link ObjectPerm} key but the two scopes. */ +export type ObjectPermFlag = Exclude; + +/** + * objectstack#26187 — `ObjectAccessScopeSchema`'s values, narrowest first, in + * the spec's own order. Restated rather than imported for the reason + * `useTenancyPosture` gives: the `@objectstack/spec/security` value entry adds + * its schema modules to the console bundle for a five-value list. The type is + * the spec's, so a value the spec does not declare does not compile, and + * `PermissionMatrixEditor.recordScope-26187.test.tsx` compares this list with + * the schema's `options`, so one the spec adds fails CI here. + */ +export const OBJECT_ACCESS_SCOPES: readonly ObjectAccessScope[] = [ + 'own', + 'own_and_reports', + 'unit', + 'unit_and_below', + 'org', +]; + +/** + * The three scopes only a hierarchy resolver enforces (`HierarchyScope`): + * `hierarchy-security`, an enterprise capability. The open runtime resolves + * them to the owner alone (fails closed), so the pickers keep them selectable + * and say so. + */ +export const HIERARCHY_SCOPES: ReadonlySet = new Set([ + 'own_and_reports', + 'unit', + 'unit_and_below', +]); + +export function isHierarchyScope(value: string | undefined): boolean { + return value !== undefined && (HIERARCHY_SCOPES as ReadonlySet).has(value); } export interface FieldPerm { From 93c9211e2ecea210adfaf52d3f21a09c77192ed1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 17:07:02 +0000 Subject: [PATCH 2/2] test(app-shell): column-header pin counts the two record-scope columns (#26187) Claude-Session: https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup Co-authored-by: Claude --- .../PermissionMatrixEditor.retiredLifecycleKeys.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx index ca0bd958805..599a42fcc1a 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx @@ -109,8 +109,10 @@ describe('PermissionMatrixEditor · retired lifecycle keys (objectui#6595)', () const headers = screen .getAllByRole('columnheader') .map((th) => th.textContent?.trim() ?? ''); - // Object + capabilities + Bulk; the capability strip is what this pins. - expect(headers.slice(1, -1)).toEqual(LIVE_COLUMN_SHORTS); + // Object + capabilities + the two record-scope pickers (objectstack#26187) + // + Bulk; the capability strip is what this pins. + expect(headers.slice(1, -3)).toEqual(LIVE_COLUMN_SHORTS); + expect(headers.slice(-3, -1)).toEqual(['Read scope', 'Write scope']); // Stated twice on purpose: the set assertion above is the guard, these two // name the keys this card retired so a future reader sees them by name. expect(headers).not.toContain('Re');