From 4754b382c0da522e32c5ab29a4359ff03c6f6842 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 12:35:24 +0000 Subject: [PATCH 1/2] fix(app-shell): Setup's permission-set and position holders offer Add and Remove only under the grant the server asks (objectui#12082) The census's last four unmapped write sites, graded on measured reach. Setup's own gate is not the boundary: its Positions and Permission Sets entries ask only the app's setup.access, which objectstack's shipped organization administrator set holds while it is read-only on both assignment objects, and the server asks the object grant of every caller at its CRUD step. So each section reads the related list's rows, asked of its assignment object: Add reads relatedNew (a junction row created), Remove reads relatedRowDelete (one deleted). A closed row withholds the Add button, its picker and each row's Remove; the holders still list. The census grades the four sites mapped and its frozen backlog is empty. The record feed pin gains one case with the registered sys_comment and sys_comment_reaction schemas in the page objects (contract review note e). Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude --- .changeset/12082-setup-holders-grant.md | 27 +++ packages/app-shell/README.md | 13 +- .../RecordDetailView.feedGrant-12082.test.tsx | 60 +++++- .../metadata-admin/AssignedUsersSection.tsx | 120 ++++++++---- .../metadata-admin/PositionHoldersSection.tsx | 93 ++++++--- .../setupHoldersGrant-12082.test.tsx | 185 ++++++++++++++++++ packages/core/README.md | 14 +- .../src/affordanceGrantMap-12082.test.tsx | 57 +++--- 8 files changed, 476 insertions(+), 93 deletions(-) create mode 100644 .changeset/12082-setup-holders-grant.md create mode 100644 packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx diff --git a/.changeset/12082-setup-holders-grant.md b/.changeset/12082-setup-holders-grant.md new file mode 100644 index 0000000000..66654d0e85 --- /dev/null +++ b/.changeset/12082-setup-holders-grant.md @@ -0,0 +1,27 @@ +--- +'@object-ui/app-shell': patch +--- + +Setup's permission-set and position holders offer Add and Remove only to a +caller holding the grant the server asks for those writes (objectui#12082). + +A permission set's holders (`AssignedUsersSection`) and a position's holders +(`PositionHoldersSection`) offered Add and a per-row Remove to whoever reached +the page. Reaching Setup is not that grant: the Setup app asks `setup.access`, +and a caller can hold it while the server refuses their write of an assignment +row at its object CRUD step. objectstack's shipped organization administrator +set is such a caller: it holds `setup.access` and is read-only on both +assignment objects. That administrator was offered writes the server refused. + +- **Add** creates a junction row under the item, so it reads the `relatedNew` + row of the affordance-to-grant map (the create grant), the row a related + list's Add existing reads. **Remove** deletes one, so it reads + `relatedRowDelete` (the delete grant). Each is asked of the assignment + object, `sys_user_permission_set` or `sys_user_position`, with its schema as + the registry serves it. +- A closed row withholds its affordance: the Add button and the picker it + opens, and each row's Remove. The holders still list, since listing them + asks only to read. +- With no permission provider mounted every grant reads open, as before. + +No row is added to `AFFORDANCE_GRANTS`, and no export, prop or type changes. diff --git a/packages/app-shell/README.md b/packages/app-shell/README.md index d85623a112..99073c7086 100644 --- a/packages/app-shell/README.md +++ b/packages/app-shell/README.md @@ -561,11 +561,20 @@ family for them: it uses the same metadata-admin list and editors at (`features.tenancyPosture`) and the session's `isPlatformAdmin`, which is the server's own `PLATFORM_ADMIN` rung. It applies in every scope, because the server refuses a package-door draft of a catalog item the same way. Assigning - holders stays writable. + holders is not under this gate; the next item says which gate it is under. - **The Setup half of an item.** It renders under the definition: a permission set's holders (`AssignedUsersSection`), and a position's holders (`PositionHoldersSection`). Both read and write assignment rows by the - item's name. + item's name. The list asks only to read. Add and Remove each read their row + of the affordance-to-grant map in `@object-ui/core`, asked of the assignment + object (`sys_user_permission_set`, `sys_user_position`): Add reads + `relatedNew` (the create grant) and Remove `relatedRowDelete` (the delete + grant). Reaching Setup is not that grant. The Setup app asks `setup.access`, + and objectstack's shipped organization administrator set holds it while it is + read-only on both assignment objects (objectstack's own pin for that is the + test `organization_admin is read-only on global RBAC tables to prevent + privilege escalation`), so that administrator sees the holders with no Add and + no Remove. The list has an active switch and an active/inactive filter, through `catalog-activation.ts`. Both show the activation ledger's state diff --git a/packages/app-shell/src/views/RecordDetailView.feedGrant-12082.test.tsx b/packages/app-shell/src/views/RecordDetailView.feedGrant-12082.test.tsx index 55aa3d090a..f5b48628d5 100644 --- a/packages/app-shell/src/views/RecordDetailView.feedGrant-12082.test.tsx +++ b/packages/app-shell/src/views/RecordDetailView.feedGrant-12082.test.tsx @@ -34,6 +34,7 @@ import { MemoryRouter } from 'react-router-dom'; import { MetadataCtx } from '@object-ui/react'; import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; import { DETAIL_DEFAULT_TRANSLATIONS } from '@object-ui/plugin-detail'; +import { effectiveOperationsArray, resolveEffectiveApiMethods } from '@objectstack/spec/data'; vi.mock('@object-ui/auth', async (importOriginal) => ({ ...(await importOriginal>()), @@ -170,14 +171,20 @@ function metadataFor(objects: unknown[]) { /** * Mounts the record page and waits for the comment to show. `perms` null is * the no-provider host (a standalone embed), where every grant reads open. + * `objects` is the page's `objects` prop, the schemas the feed's gates read. */ -async function mount(perms: MePermissionsResponse | null, dataSource = makeDataSource(), registry: unknown[] = REGISTRY.records) { +async function mount( + perms: MePermissionsResponse | null, + dataSource = makeDataSource(), + registry: unknown[] = REGISTRY.records, + objects: unknown[] = OBJECTS, +) { const page = ( {}} objectNameOverride={OBJECT_NAME} recordIdOverride={RECORD_ID} @@ -284,6 +291,55 @@ describe("the record feed's reaction control reads feedReactionAdd / feedReactio }); }); +/** + * The two feed objects as objectstack registers them (`plugin-audit`): their + * policy keys (`managedBy`, `userActions`) and the `enable` block the server + * derives the effective operation set from. The other cases pass an `objects` + * prop holding neither, so the policy leg there reads the default bucket. + */ +const REGISTERED_COMMENT = { + name: COMMENT, + label: 'Comment', + managedBy: 'platform', + enable: { trackHistory: true, searchable: true, apiEnabled: true, clone: false }, +}; +const REGISTERED_REACTION = { + name: REACTION, + label: 'Comment Reaction', + managedBy: 'platform', + userActions: { edit: false, import: false }, + enable: { + trackHistory: false, + searchable: false, + apiEnabled: true, + apiMethods: ['get', 'list', 'create', 'delete', 'bulk'], + feeds: false, + activities: false, + clone: false, + }, +}; + +describe('against the registered feed schemas in the page objects (objectui#12082)', () => { + it("under a full grant the registered policy closes nothing the feed offers: the composer, the reaction add and the take-back are each offered", async () => { + // The reaction object's effective operation set, derived by the spec from + // its `enable` block (no export grant is held, as in `SHAPE.full`). + const reactionOperations = effectiveOperationsArray( + resolveEffectiveApiMethods(REGISTERED_REACTION.enable, { userExportAllowed: false }), + ); + const { view, dataSource } = await mount( + answer({ [COMMENT]: SHAPE.full, [REACTION]: { ...SHAPE.full, apiOperations: reactionOperations } }), + makeDataSource(), + [...OBJECTS, REGISTERED_COMMENT, REGISTERED_REACTION], + [...OBJECTS, REGISTERED_COMMENT, REGISTERED_REACTION], + ); + expect(composer(view)).not.toBeNull(); + expect(addReaction(view)).not.toBeNull(); + fireEvent.click(thumbsChip(view)!); + await act(async () => {}); + expect(dataSource.delete.mock.calls).toEqual([[REACTION, 'r1']]); + }); +}); + describe('on the column store the reaction control reads feedReactionColumn: update on sys_comment (objectui#12082)', () => { it.each([ ['create-only', SHAPE.createOnly, false], diff --git a/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx b/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx index 00f7e7c921..7f50639058 100644 --- a/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx +++ b/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx @@ -40,15 +40,28 @@ * (`ai_seat`) is just one of them. The generic add-by-picker engine (spec * RecordRelatedListProps.add) powers the capability; this is the polished * surface for the high-value case. + * + * ## Who may change the holders (objectui#12082) + * + * ADD and REMOVE are offered only where the caller holds the grant the server + * asks of them, on the grant object itself — see {@link GRANT_OBJECT} and the + * gate in the component. Listing the holders asks only to read, so a caller + * who may read but not write the grants sees the list with no Add and no + * Remove. */ import * as React from 'react'; import { Button } from '@object-ui/components'; +import { resolveAffordance, type SchemaLike } from '@object-ui/core'; import { RecordPickerDialog } from '@object-ui/fields'; +import { usePermissions } from '@object-ui/permissions'; import { useAdapter, useMetadata } from '@object-ui/react'; import { Plus, X, Users, Loader2, AlertCircle } from 'lucide-react'; import { useMetadataLocale } from './i18n.js'; +/** The object a permission set's direct grants are rows of: ADD creates one, REMOVE deletes one. */ +const GRANT_OBJECT = 'sys_user_permission_set'; + export interface AssignedUsersSectionProps { /** The permission set's machine name (e.g. `ai_seat`, `admin_full_access`). */ permissionSetName: string; @@ -163,12 +176,39 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection const [busy, setBusy] = React.useState(false); const [error, setError] = React.useState(null); + /** + * May the caller add or remove a holder? Each is a write of a + * {@link GRANT_OBJECT} row, so each reads its row of the affordance-to-grant + * map (`resolveAffordance` in `@object-ui/core`), asked of that object with + * its schema as the registry serves it: ADD creates a junction row under the + * set, the `relatedNew` row a related list's Add existing reads; REMOVE + * deletes one, the `relatedRowDelete` row a related list row's Delete reads. + * + * Setup's own gate is not this one. The Setup app asks `setup.access`, and + * a caller can hold it while the server refuses their write of a grant row + * at its object CRUD step — objectstack's shipped organization administrator + * set is such a caller (that repo's pin `organization_admin is read-only on + * global RBAC tables to prevent privilege escalation`). So a closed row + * withholds its affordance: the Add button and the picker it opens, each + * row's Remove. With no permission provider every grant reads open, as + * before. + */ + const perms = usePermissions(); + const grantSource = { + objectSchema: ((metadataStore.objects ?? []).find((o) => (o as { name?: unknown } | null)?.name === GRANT_OBJECT) ?? + null) as SchemaLike | null, + objectName: GRANT_OBJECT, + perms, + }; + const canAdd = resolveAffordance('relatedNew', grantSource).allowed; + const canRemove = resolveAffordance('relatedRowDelete', grantSource).allowed; + const load = React.useCallback(async () => { setLoading(true); try { // Direct grants, by the set's NAME. const grants = asArray( - await adapter.find('sys_user_permission_set', { $filter: { permission_set: permissionSetName }, $top: 500 }), + await adapter.find(GRANT_OBJECT, { $filter: { permission_set: permissionSetName }, $top: 500 }), ); // Effective holders = direct grants ∪ holders of every position that @@ -251,7 +291,7 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection if (!uid || assignedIds.has(uid)) continue; // By name alone: no catalog row is read and no legacy // `permission_set_id` is written (module doc). - await adapter.create('sys_user_permission_set', { + await adapter.create(GRANT_OBJECT, { user_id: uid, permission_set: permissionSetName, }); @@ -278,7 +318,7 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection async (grantId: string) => { setError(null); try { - await adapter.delete('sys_user_permission_set', grantId); + await adapter.delete(GRANT_OBJECT, grantId); await load(); } catch { /* keep the row; a failed delete is non-destructive */ @@ -297,19 +337,21 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection {c.countOf(rows.length)} )} - + {canAdd && ( + + )} {error && ( @@ -360,16 +402,20 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection )} {r.grantId ? ( - + // A direct grant: removable only where the delete grant on + // the grant object is held (`canRemove`, above). + canRemove && ( + + ) ) : ( — @@ -380,16 +426,18 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection )} - setPickerOpen(o)} - multiple - dataSource={adapter} - objectName="sys_user" - title={c.pickTitle} - onSelect={() => {}} - onSelectRecords={(records: any[]) => void addUsers(records)} - /> + {canAdd && ( + setPickerOpen(o)} + multiple + dataSource={adapter} + objectName="sys_user" + title={c.pickTitle} + onSelect={() => {}} + onSelectRecords={(records: any[]) => void addUsers(records)} + /> + )} ); } diff --git a/packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx b/packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx index 50176c39fe..a0b47e4bf1 100644 --- a/packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx +++ b/packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx @@ -14,8 +14,12 @@ * which names the position BY NAME (`position` stores the machine name). So * this section is an ordinary data page over that object — the same reads and * writes the generic assignment page makes — and the server decides who may - * add or remove a holder, under every posture (under a wall a tenant - * administrator assigns what the operator defined). + * add or remove a holder, under every posture: its object CRUD step asks the + * caller's grant on `sys_user_position`, and its delegated-administration gate + * may refuse a holder of that grant as well. The section reads the first half + * (objectui#12082): ADD and REMOVE are offered only where that grant is held, + * so a caller who may read the holders but not write them sees the list + * alone. Reaching Setup is not that grant — see the gate in the component. * * The shape follows `AssignedUsersSection` (a permission set's holders): * people first, add through `RecordPickerDialog`, remove per row. The server @@ -28,8 +32,10 @@ import * as React from 'react'; import { Button } from '@object-ui/components'; +import { resolveAffordance, type SchemaLike } from '@object-ui/core'; import { RecordPickerDialog } from '@object-ui/fields'; -import { useAdapter } from '@object-ui/react'; +import { usePermissions } from '@object-ui/permissions'; +import { useAdapter, useMetadata } from '@object-ui/react'; import { AlertCircle, Loader2, Plus, UserCheck, X } from 'lucide-react'; import { useMetadataLocale } from './i18n.js'; @@ -102,6 +108,35 @@ export function PositionHoldersSection({ name }: PositionHoldersSectionProps): R const [busy, setBusy] = React.useState(false); const [error, setError] = React.useState(null); + /** + * May the caller add or remove a holder? Each is a write of a + * {@link USER_POSITION_OBJECT} row, so each reads its row of the + * affordance-to-grant map (`resolveAffordance` in `@object-ui/core`), asked + * of that object with its schema as the registry serves it: ADD creates a + * junction row under the position, the `relatedNew` row a related list's Add + * existing reads; REMOVE deletes one, the `relatedRowDelete` row a related + * list row's Delete reads. + * + * Setup's own gate is not this one. The Setup app asks `setup.access`, and + * a caller can hold it while the server refuses their write of an assignment + * row at its object CRUD step — objectstack's shipped organization + * administrator set is such a caller (that repo's pin `organization_admin is + * read-only on global RBAC tables to prevent privilege escalation`). So a + * closed row withholds its affordance: the Add button and the picker it + * feeds, each row's Remove. With no permission provider every grant reads + * open, as before. + */ + const { objects } = useMetadata(); + const perms = usePermissions(); + const holdingSource = { + objectSchema: ((objects ?? []).find((o) => (o as { name?: unknown } | null)?.name === USER_POSITION_OBJECT) ?? + null) as SchemaLike | null, + objectName: USER_POSITION_OBJECT, + perms, + }; + const canAdd = resolveAffordance('relatedNew', holdingSource).allowed; + const canRemove = resolveAffordance('relatedRowDelete', holdingSource).allowed; + const load = React.useCallback(async () => { if (!adapter) return; setLoading(true); @@ -187,19 +222,21 @@ export function PositionHoldersSection({ name }: PositionHoldersSectionProps): R {c.title} {!loading && {rows.length}} - + {canAdd && ( + + )}
{error && ( @@ -228,22 +265,24 @@ export function PositionHoldersSection({ name }: PositionHoldersSectionProps): R
{r.email}
)}
- + {canRemove && ( + + )} ))} )} - {adapter && ( + {adapter && canAdd && ( setPickerOpen(o)} diff --git a/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx b/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx new file mode 100644 index 0000000000..c97e5e3a8f --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx @@ -0,0 +1,185 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Setup's holders sections read the affordance-to-grant map (objectui#12082, + * the census's last four `unmapped` write sites). + * + * A permission set's holders (`AssignedUsersSection`) and a position's holders + * (`PositionHoldersSection`) each offered Add and a per-row Remove to whoever + * reached the page, and read no grant. Reaching the page is not the grant: the + * Setup app asks `setup.access`, and objectstack's shipped organization + * administrator set holds it while it is read-only on both assignment objects, + * so that caller was offered writes the server refuses at its object CRUD + * step. Now each write reads its row, asked of the object it writes: + * + * - Add (a junction row created): `relatedNew`, the create grant; + * - Remove (a junction row deleted): `relatedRowDelete`, the delete grant. + * + * Real subjects: both sections under the REAL `MePermissionsProvider` holding + * the caller's `/me/permissions` answer. The adapter and the metadata store + * are fixtures; the picker stands in as a marker so its mount can be read. + */ + +import * as React from 'react'; +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import { MePermissionsProvider, type MePermissionsResponse } from '@object-ui/permissions'; + +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => adapter, + useMetadata: () => metadataStore, +})); +// The picker stands in as a marker, rendered whenever it is mounted (open or not). +vi.mock('@object-ui/fields', async (importOriginal) => { + const { createElement } = await import('react'); + return { + ...(await importOriginal()), + RecordPickerDialog: () => createElement('div', { 'data-testid': 'record-picker' }), + }; +}); + +import { AssignedUsersSection } from './AssignedUsersSection'; +import { PositionHoldersSection } from './PositionHoldersSection'; + +const GRANT_OBJECT = 'sys_user_permission_set'; +const HOLDING_OBJECT = 'sys_user_position'; + +const data: Record>> = { + [GRANT_OBJECT]: [{ id: 'grant_1', permission_set: 'showcase_contributor', user_id: 'u_direct' }], + [HOLDING_OBJECT]: [{ id: 'up_1', user_id: 'u_held', position: 'auditor' }], + sys_user: [ + { id: 'u_direct', name: 'Direct Dana', email: 'dana@example.com' }, + { id: 'u_held', name: 'Held Henry', email: 'henry@example.com' }, + ], +}; + +const adapter = { + find: vi.fn(async (object: string, query: { $filter?: Record } | undefined) => { + const filter = query?.$filter ?? {}; + return { + data: (data[object] ?? []).filter((row) => + Object.entries(filter).every(([k, v]) => + v && typeof v === 'object' && Array.isArray((v as { $in?: unknown[] }).$in) + ? (v as { $in: unknown[] }).$in.includes(row[k]) + : row[k] === v, + ), + ), + }; + }), + create: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), +}; + +/** The registry: both assignment objects as objectstack registers them (`system-data`, full CRUD by default). */ +const metadataStore = { + objects: [ + { name: GRANT_OBJECT, label: 'User Permission Set', managedBy: 'system-data' }, + { name: HOLDING_OBJECT, label: 'User Position', managedBy: 'system-data' }, + ], + ensureType: vi.fn(async () => []), +}; + +/** The grant shapes on one assignment object, as `/me/permissions` carries them. */ +const SHAPE = { + readOnly: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false }, + createOnly: { allowRead: true, allowCreate: true, allowEdit: false, allowDelete: false }, + deleteOnly: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: true }, + full: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, +} as const; + +function answer(objects: MePermissionsResponse['objects']): MePermissionsResponse { + return { + authenticated: true, + userId: 'u-admin', + tenantId: null, + roles: ['member'], + permissionSets: ['member'], + objects, + fields: {}, + } as MePermissionsResponse; +} + +/** + * An organization administrator's answer: a super-user wildcard beside the + * explicit read-only entries its set names for both assignment objects. The + * explicit entry is the set's whole answer for the object, on the server's + * evaluator and in the map it hands the client alike. + */ +const ORG_ADMIN = answer({ + '*': { + allowRead: true, + allowCreate: true, + allowEdit: true, + allowDelete: true, + viewAllRecords: true, + modifyAllRecords: true, + }, + [GRANT_OBJECT]: SHAPE.readOnly, + [HOLDING_OBJECT]: SHAPE.readOnly, +} as MePermissionsResponse['objects']); + +const SECTIONS = [ + { + name: 'AssignedUsersSection', + object: GRANT_OBJECT, + holder: 'Direct Dana', + add: 'Add user', + element: , + }, + { + name: 'PositionHoldersSection', + object: HOLDING_OBJECT, + holder: 'Held Henry', + add: 'Add holder', + element: , + }, +] as const; + +type Section = (typeof SECTIONS)[number]; + +/** Mounts a section and waits for its holder to list. `perms` null is the no-provider host. */ +async function mount(section: Section, perms: MePermissionsResponse | null) { + render(perms ? {section.element} : section.element); + await waitFor(() => expect(screen.getByText(section.holder)).toBeInTheDocument()); +} + +/** What the section offers: its Add button, the picker that Add feeds, and its rows' Remove. */ +const offered = (section: Section) => ({ + add: !!screen.queryByRole('button', { name: section.add }), + picker: !!screen.queryByTestId('record-picker'), + remove: screen.queryAllByRole('button', { name: 'Remove' }).length > 0, +}); + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe.each(SECTIONS)("Setup's $name reads relatedNew / relatedRowDelete on its assignment object (objectui#12082)", (section) => { + it('an organization administrator — read-only on the assignment object beside a super-user wildcard — sees the holders and is offered no write', async () => { + await mount(section, ORG_ADMIN); + expect(offered(section)).toEqual({ add: false, picker: false, remove: false }); + }); + + it.each([ + ['read-only', SHAPE.readOnly, { add: false, picker: false, remove: false }], + ['create-only', SHAPE.createOnly, { add: true, picker: true, remove: false }], + ['delete-only', SHAPE.deleteOnly, { add: false, picker: false, remove: true }], + ['full', SHAPE.full, { add: true, picker: true, remove: true }], + ] as const)('under a %s grant on the assignment object, Add follows create and Remove follows delete', async (_shape, bits, expected) => { + await mount(section, answer({ [section.object]: bits })); + expect(offered(section)).toEqual(expected); + }); + + it('an effective operation set without create or delete closes both, whatever the grant', async () => { + await mount(section, answer({ [section.object]: { ...SHAPE.full, apiOperations: ['get', 'list'] } })); + expect(offered(section)).toEqual({ add: false, picker: false, remove: false }); + }); + + it('CONTROL — with no permission provider both are offered, as before', async () => { + await mount(section, null); + expect(offered(section)).toEqual({ add: true, picker: true, remove: true }); + }); +}); diff --git a/packages/core/README.md b/packages/core/README.md index 4cb5055b70..2ae2aae700 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -228,8 +228,13 @@ reads NO grant at all — the calendar's quick-create and drag-to-reschedule affordance that writes with the same verb on the same object reuses that row: the record overlay's inline edit reads `recordEdit`, a gantt's bar edits and a grid's bulk field update read `rowEdit`, a gantt row's Delete reads `rowDelete`, -the AI build panel's spreadsheet import reads `listImport`, and a related list's -Add existing reads `relatedNew` (a link row) or `relatedRowEdit` (a re-parent). +the AI build panel's spreadsheet import reads `listImport`, a related list's +Add existing reads `relatedNew` (a link row) or `relatedRowEdit` (a re-parent), +and Setup's permission-set and position holders read `relatedNew` (a holder +added) and `relatedRowDelete` (a holder removed), each asked of its assignment +object (`sys_user_permission_set`, `sys_user_position`). Reaching Setup is not +that grant: the Setup app asks `setup.access`, which a caller can hold while the +server refuses their write of an assignment row. The record feed's writes have rows of their own, each asked of the object it writes: `feedComment` (a comment or reply, `sys_comment`), `feedReactionAdd` and `feedReactionRemove` (the caller's own `sys_comment_reaction` row), and @@ -239,8 +244,9 @@ enumeration pin in `@object-ui/plugin-form` (`affordanceGrantMap-12082.test.tsx` also counts every write call site in the console tree and holds each to the row it sits behind, or to the reason it has none — and, for a component whose write its host gates (the import wizard), holds every mount to reading the row. The -sites it found with no row yet form a frozen backlog: a new one cannot be added, -so a write affordance gets its row when it lands. +sites it found with no row yet formed a frozen backlog, which the pin now holds +empty: a new one cannot be added, so a write affordance gets its row when it +lands. ### Undo snapshot for an update (`captureUpdateUndoData`) diff --git a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx index 81a24ce646..b7baf92701 100644 --- a/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx +++ b/packages/plugin-form/src/affordanceGrantMap-12082.test.tsx @@ -52,11 +52,13 @@ * write affordance with no row yet (`unmapped`, the backlog this census * found, named one by one). A write site added later with no entry turns it * red, naming the site; an entry whose site is gone turns it red too. - * 8. **The backlog is frozen.** `UNMAPPED_BACKLOG` names every key the census - * still grades `unmapped`, and the two must be equal: a NEW `unmapped` entry - * turns it red until the site is graded (`mapped`, or the reason it is no - * affordance), and a key graded out of the backlog must leave the list. So - * the backlog can only shrink. + * 8. **The backlog is frozen, and now empty.** `UNMAPPED_BACKLOG` names every + * key the census still grades `unmapped`, and the two must be equal: a NEW + * `unmapped` entry turns it red until the site is graded (`mapped`, or the + * reason it is no affordance), and a key graded out of the backlog must + * leave the list. So the backlog can only shrink, and it has shrunk to + * nothing: the card's done-when, a census with no `unmapped` entry, holds + * for as long as the list stays empty. */ import { describe, it, expect, afterEach } from 'vitest'; import { render, cleanup } from '@testing-library/react'; @@ -610,7 +612,9 @@ type WriteSite = | { kind: 'outOfFamily'; why: string } /** * A write affordance with no map row yet — the backlog this census found, - * named one by one. FROZEN: only a key `UNMAPPED_BACKLOG` holds may carry it. + * named one by one. FROZEN: only a key `UNMAPPED_BACKLOG` holds may carry it, + * and that list is empty, so no entry may. Its helper retired with its last + * entry; the kind stays so the freeze below still reads a grade like it. */ | { kind: 'unmapped'; why: string }; @@ -618,7 +622,6 @@ const mapped = (rows: ConsoleAffordance[], ...readBy: string[]): WriteSite => ({ const helper = (name: string): WriteSite => ({ kind: 'helper', helper: name }); const door = (why: string): WriteSite => ({ kind: 'door', why }); const outOfFamily = (why: string): WriteSite => ({ kind: 'outOfFamily', why }); -const unmapped = (why: string): WriteSite => ({ kind: 'unmapped', why }); const FIELD_WRITE_GATE = 'packages/plugin-form/src/fieldWriteGate.ts'; const DETAIL_VIEW = 'packages/plugin-detail/src/DetailView.tsx'; @@ -638,7 +641,11 @@ const MASTER_DETAIL = 'packages/plugin-form/src/MasterDetailForm.tsx'; const EXCEL_IMPORT_BAR = 'packages/app-shell/src/console/ai/ExcelImportBar.tsx'; const RELATED_LIST = 'packages/plugin-detail/src/RelatedList.tsx'; const RELATED_LIST_BLOCK = 'packages/plugin-detail/src/renderers/record-related-list.tsx'; -const ADAPTER = 'a DataSource implementation: it runs the writes its callers hand it'; +// Setup's holders sections: each reads its own rows, so every mount (the +// permission-set editor, the position editor, the record renderer) is gated. +const ASSIGNED_USERS = 'packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx'; +const POSITION_HOLDERS = 'packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx'; +const ADAPTER ='a DataSource implementation: it runs the writes its callers hand it'; /** * Every write call site in the console tree, by `FILE :: SYMBOLS :: VERB`. @@ -680,14 +687,25 @@ const WRITE_SITES: Record = { mapped(['recordDelete'], RECORD_PAGE), 'packages/app-shell/src/views/RelatedRecordActionsBridge.tsx :: RelatedRecordActionsBridge > value > resolve :: delete': mapped(['relatedRowDelete'], 'packages/app-shell/src/views/RelatedRecordActionsBridge.tsx'), + // Setup's permission-set and position holders (objectui#12082, leg B's last + // four). Reach, measured: Setup's own gate is not the boundary. Its Positions + // and Permission Sets entries ask nothing beyond the app's `setup.access`, + // which objectstack's shipped organization administrator set holds while it + // is read-only on both assignment objects (that repo's pin `organization_admin + // is read-only on global RBAC tables to prevent privilege escalation`), and + // the server asks the object grant of every caller at its CRUD step. Each + // section is a people-first list of a junction under its item, so it reads + // the related list's rows: a holder added is a junction row created + // (`relatedNew`, as Add existing with a link field), a holder removed one + // deleted (`relatedRowDelete`), each asked of the assignment object. 'packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx :: AssignedUsersSection > addUsers :: create': - unmapped("Setup's permission-set assignment creates sys_user_permission_set rows and reads no grant on that object"), + mapped(['relatedNew'], ASSIGNED_USERS), 'packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx :: AssignedUsersSection > removeUser :: delete': - unmapped("Setup's permission-set assignment deletes sys_user_permission_set rows and reads no grant on that object"), + mapped(['relatedRowDelete'], ASSIGNED_USERS), 'packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx :: PositionHoldersSection > addHolders :: create': - unmapped("Setup's position assignment creates sys_user_position rows and reads no grant on that object"), + mapped(['relatedNew'], POSITION_HOLDERS), 'packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx :: PositionHoldersSection > removeHolder :: delete': - unmapped("Setup's position assignment deletes sys_user_position rows and reads no grant on that object"), + mapped(['relatedRowDelete'], POSITION_HOLDERS), // ── core ── 'packages/core/src/actions/TransactionManager.ts :: TransactionManager > executeBatch :: batch': door('the action engine\'s transaction executor runs the operations its caller hands it'), @@ -857,17 +875,12 @@ const WRITE_SITES: Record = { * - a key graded out of `unmapped` (mapped, out of family, or gone from the * tree) is red until it is deleted from this list. * - * ⛔ Never add a key here. The list exists to shrink to nothing: the card's - * done-when is a census with no `unmapped` entry, and then this list is empty. + * ⛔ Never add a key here. The list existed to shrink to nothing: the card's + * done-when is a census with no `unmapped` entry, and this list is now empty. + * The last four keys, Setup's permission-set and position holders, were graded + * `mapped` once their reach was measured (their entries above say how). */ -const UNMAPPED_BACKLOG: ReadonlySet = new Set([ - // Setup's permission-set and position assignment: the readers of the - // objectui#7611 epic's C9, graded with it. - 'packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx :: AssignedUsersSection > addUsers :: create', - 'packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx :: AssignedUsersSection > removeUser :: delete', - 'packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx :: PositionHoldersSection > addHolders :: create', - 'packages/app-shell/src/views/metadata-admin/PositionHoldersSection.tsx :: PositionHoldersSection > removeHolder :: delete', -]); +const UNMAPPED_BACKLOG: ReadonlySet = new Set(); const isFn = (e: ts.Node | undefined): boolean => !!e && (ts.isArrowFunction(e) || ts.isFunctionExpression(e)); From cf08306b07a85697fddff022eec9ae7c1de05e1c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 14:18:20 +0000 Subject: [PATCH 2/2] fix(app-shell): say "click Add" in the permission set's empty state only where Add is offered AssignedUsersSection's empty state read "No users assigned yet. Click "Add user" to assign." (and its zh twin) to a caller whose create grant on sys_user_permission_set is closed, so it named a button the section no longer renders for them. The copy table splits the empty state into the statement and the click-Add instruction, in en and zh, and the instruction is rendered only under canAdd. The statement shows either way. setupHoldersGrant-12082 pins it in both locales: a set nobody holds, under a delete-only grant (create closed) shows the statement and no instruction, and under a create-only grant shows both, beside the Add button it names. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude --- .../metadata-admin/AssignedUsersSection.tsx | 26 ++++++++-- .../setupHoldersGrant-12082.test.tsx | 49 +++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx b/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx index 7f50639058..91688f271f 100644 --- a/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx +++ b/packages/app-shell/src/views/metadata-admin/AssignedUsersSection.tsx @@ -47,7 +47,7 @@ * asks of them, on the grant object itself — see {@link GRANT_OBJECT} and the * gate in the component. Listing the holders asks only to read, so a caller * who may read but not write the grants sees the list with no Add and no - * Remove. + * Remove, and an empty list that does not tell them to click Add. */ import * as React from 'react'; @@ -77,7 +77,14 @@ interface AssignedRow { via: Array<{ kind: 'direct' } | { kind: 'position'; position: string }>; } -/** Minimal locale-aware copy (zh vs everything-else) — keeps the surface in the user's language. */ +/** + * Minimal locale-aware copy (zh vs everything-else) — keeps the surface in the user's language. + * + * The empty state is two sentences: `empty` says nobody holds the set, and + * `emptyAddHint` tells the reader to click Add, so it is said only where the + * Add button is offered (`canAdd`, in the component). `sentenceGap` joins + * them: a space in English, none after a CJK full stop. + */ function useCopy() { const zh = useMetadataLocale() === 'zh-CN'; return React.useMemo( @@ -87,7 +94,9 @@ function useCopy() { title: '已分配用户', add: '添加用户', remove: '移除', - empty: '还没有分配任何用户。点击「添加用户」来分配。', + empty: '还没有分配任何用户。', + emptyAddHint: '点击「添加用户」来分配。', + sentenceGap: '', loading: '加载中…', pickTitle: '选择要分配的用户', seatFull: (n: number) => @@ -103,7 +112,9 @@ function useCopy() { title: 'Assigned Users', add: 'Add user', remove: 'Remove', - empty: 'No users assigned yet. Click "Add user" to assign.', + empty: 'No users assigned yet.', + emptyAddHint: 'Click "Add user" to assign.', + sentenceGap: ' ', loading: 'Loading…', pickTitle: 'Select users to assign', seatFull: (n: number) => @@ -377,7 +388,12 @@ export function AssignedUsersSection({ permissionSetName }: AssignedUsersSection {c.loading} ) : rows.length === 0 ? ( - !everyoneBound &&
{c.empty}
+ !everyoneBound && ( +
+ {/* The click-Add sentence names a button only `canAdd` renders. */} + {canAdd ? c.empty + c.sentenceGap + c.emptyAddHint : c.empty} +
+ ) ) : (
    {rows.map((r) => ( diff --git a/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx b/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx index c97e5e3a8f..c62371263d 100644 --- a/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/setupHoldersGrant-12082.test.tsx @@ -15,6 +15,9 @@ * - Add (a junction row created): `relatedNew`, the create grant; * - Remove (a junction row deleted): `relatedRowDelete`, the delete grant. * + * The permission set's empty state follows Add: its sentence telling the + * reader to click Add is said only where Add is offered. + * * Real subjects: both sections under the REAL `MePermissionsProvider` holding * the caller's `/me/permissions` answer. The adapter and the metadata store * are fixtures; the picker stands in as a marker so its mount can be read. @@ -39,6 +42,12 @@ vi.mock('@object-ui/fields', async (importOriginal) => { RecordPickerDialog: () => createElement('div', { 'data-testid': 'record-picker' }), }; }); +// The sections' locale, so the empty state is read in both of its languages. +let localeFixture: 'en-US' | 'zh-CN' = 'en-US'; +vi.mock('./i18n', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadataLocale: () => localeFixture, +})); import { AssignedUsersSection } from './AssignedUsersSection'; import { PositionHoldersSection } from './PositionHoldersSection'; @@ -155,6 +164,7 @@ const offered = (section: Section) => ({ afterEach(() => { cleanup(); vi.clearAllMocks(); + localeFixture = 'en-US'; }); describe.each(SECTIONS)("Setup's $name reads relatedNew / relatedRowDelete on its assignment object (objectui#12082)", (section) => { @@ -183,3 +193,42 @@ describe.each(SECTIONS)("Setup's $name reads relatedNew / relatedRowDelete on it expect(offered(section)).toEqual({ add: true, picker: true, remove: true }); }); }); + +/** + * A permission set nobody holds. The empty state's first sentence says so under + * every grant; its second tells the reader to click Add, so it is said only + * where the Add button is offered. Said to a caller offered no Add, it names a + * button that is not there. + * + * The two grants flip create and delete in opposite directions, so a gate on + * the wrong row reads red in both cases. + */ +const EMPTY_STATE = [ + { locale: 'en-US', statement: /No users assigned yet/, add: 'Add user' }, + { locale: 'zh-CN', statement: /还没有分配任何用户/, add: '添加用户' }, +] as const; + +describe.each(EMPTY_STATE)("AssignedUsersSection's empty state names Add only where Add is offered, in $locale (objectui#12082)", ({ locale, statement, add }) => { + /** Mounts a set with no holders and returns its empty state, found by its first sentence. */ + async function mountUnheld(perms: MePermissionsResponse) { + localeFixture = locale; + render( + + + , + ); + return screen.findByText(statement); + } + + it('with the create grant closed, it says nobody is assigned and does not say to click Add', async () => { + const empty = await mountUnheld(answer({ [GRANT_OBJECT]: SHAPE.deleteOnly })); + expect(screen.queryByRole('button', { name: add })).toBeNull(); + expect(empty.textContent).not.toContain(add); + }); + + it('with the create grant open, it says both, and the button it names is offered', async () => { + const empty = await mountUnheld(answer({ [GRANT_OBJECT]: SHAPE.createOnly })); + expect(screen.getByRole('button', { name: add })).toBeInTheDocument(); + expect(empty.textContent).toContain(add); + }); +});