diff --git a/.changeset/27404-permission-set-system-field-rows.md b/.changeset/27404-permission-set-system-field-rows.md new file mode 100644 index 00000000000..7a05bf26a30 --- /dev/null +++ b/.changeset/27404-permission-set-system-field-rows.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/console': patch +--- + +fix(setup): a permission set's field rows no longer offer the platform's bookkeeping columns + +Clause-②: no +Metadata-protocol: none + +- **Before.** Expanding an object in a permission set listed every field of the merged definition, so the platform's search companion `__search` sorted first and `organization_id` / `owning_business_unit_id` sat among the object's own fields. +- **Now.** A field the platform marks both `system` and `hidden` is not offered, the same rule the Studio Data rail, form designer and pickers already apply. The bulk shortcuts act only on the offered rows. +- **Unchanged.** A field the set already carries an entry for still draws as written, also while the draft clears it. An author's own hidden field and the system `owner_id` still draw. How a save writes is unchanged, and the row-condition editor still knows every field name. diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.systemFields-27404.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.systemFields-27404.test.tsx new file mode 100644 index 00000000000..032600aa317 --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.systemFields-27404.test.tsx @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27404 — the field sub-table offers the object's own fields, not + * the columns the platform injects for its bookkeeping. Such a column carries + * `system: true` AND `hidden: true` on the served definition (`__search`, + * `organization_id`), the Data rail's predicate (`isStudioHiddenSystemField`, + * objectui#11780). A stored entry against one still draws as written (the + * objectui#13152 CONTROL); either mark alone is not enough to leave a field out. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +interface FieldEntry { + readable?: boolean; + editable?: boolean; +} +interface SavedSet { + fields?: Record; +} + +// `withPreviewDrafts` answers the client itself, not a promise (objectstack#26840). +type FakeClient = Record unknown>; + +let clientImpl: FakeClient; +let saved: SavedSet[] = []; + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => clientImpl, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'permission', label: 'Permission', allowEnvironmentOverlay: true }], + }), +})); + +vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null })); + +import { PermissionMatrixEditPage } from './PermissionMatrixEditor'; + +afterEach(() => { + cleanup(); + saved = []; +}); + +// The served definition: two bookkeeping columns (system + hidden), the +// reassignable owner (system only), an author's own hidden field, and two +// plain fields. +const SERVED_FIELDS = [ + { name: '__search', label: 'Search Index', system: true, hidden: true }, + { name: 'amount', label: 'Amount' }, + { name: 'internal_notes', label: 'Internal notes', hidden: true }, + { name: 'name', label: 'Name' }, + { name: 'organization_id', label: 'Organization', system: true, hidden: true }, + { name: 'owner_id', label: 'Owner', system: true }, +]; + +const EDIT_ON = { allowRead: true, allowCreate: true, allowEdit: true }; + +function makeClient(storedFields: Record): FakeClient { + return { + layered: async () => ({ + effective: { + name: 'project_editor', + label: 'Project editor', + objects: { a_project: EDIT_ON }, + fields: storedFields, + }, + code: null, + overlay: null, + overlayScope: null, + }), + getDraft: async () => null, + withPreviewDrafts() { + return this; + }, + list: async (type: string) => (type === 'object' ? [{ item: { name: 'a_project' } }] : []), + get: async (type: string) => (type === 'object' ? { fields: SERVED_FIELDS } : null), + save: async (_type: string, _name: string, payload: Record) => { + saved.push(JSON.parse(JSON.stringify(payload)) as SavedSet); + return payload; + }, + }; +} + +/** The package door autosaves 1.5s after the last edit (objectui#11787). */ +const AUTOSAVE = { timeout: 4000 }; + +async function renderExpanded(storedFields: Record = {}) { + clientImpl = makeClient(storedFields); + render( + + + , + ); + fireEvent.click(await screen.findByRole('button', { name: /a_project/ })); + await screen.findByLabelText('a_project.name readable'); +} + +/** The field names the sub-table draws, in the order it draws them. */ +const drawnRows = () => + screen + .getAllByTestId(/^perm-field-name-a_project-/) + .map((el) => el.getAttribute('data-testid')!.slice('perm-field-name-a_project-'.length)); + +describe('the field rows leave out the platform bookkeeping columns (objectstack#27404)', () => { + it('does not offer a field that is system AND hidden', async () => { + await renderExpanded(); + expect(screen.queryByTestId('perm-field-name-a_project-__search')).toBeNull(); + expect(screen.queryByTestId('perm-field-name-a_project-organization_id')).toBeNull(); + expect(screen.queryByLabelText('a_project.__search readable')).toBeNull(); + expect(screen.getByTestId('perm-field-count-a_project')).toHaveTextContent('4 / 4'); + }); + + it('CONTROL: one mark alone is not enough — an author-hidden field and the system owner still draw, in name order', async () => { + await renderExpanded(); + expect(drawnRows()).toEqual(['amount', 'internal_notes', 'name', 'owner_id']); + }); + + it('CONTROL: a stored entry against a bookkeeping column still draws, as written', async () => { + await renderExpanded({ 'a_project.__search': { readable: false, editable: false } }); + expect(drawnRows()).toEqual(['__search', 'amount', 'internal_notes', 'name', 'owner_id']); + const read = screen.getByLabelText('a_project.__search readable'); + expect(read).not.toBeChecked(); + expect(read).toHaveAttribute('data-field-rule', 'rule'); + expect(screen.queryByTestId('perm-field-name-a_project-organization_id')).toBeNull(); + }); + + it('CONTROL: a stored row the draft clears keeps its row until the save lands, then nothing is stored', async () => { + await renderExpanded({ 'a_project.__search': { readable: false, editable: false } }); + fireEvent.click(screen.getByRole('button', { name: 'Clear' })); + // Unsaved: the loaded set still carries the entry, so the row shows what the save drops. + expect(screen.getByTestId('perm-field-name-a_project-__search')).toBeInTheDocument(); + expect(screen.getByLabelText('a_project.__search readable')).toHaveAttribute('data-field-rule', 'inherited'); + await waitFor(() => expect(saved.length).toBeGreaterThan(0), AUTOSAVE); + expect(saved[saved.length - 1].fields?.['a_project.__search']).toBeUndefined(); + // Saved: no entry is stored any more, so the column is not offered. + await waitFor(() => expect(screen.queryByTestId('perm-field-name-a_project-__search')).toBeNull()); + }); + + it('a bulk shortcut writes only the offered rows, never a bookkeeping column', async () => { + await renderExpanded(); + fireEvent.click(screen.getByRole('button', { name: 'Read-only all' })); + await waitFor(() => expect(saved.length).toBeGreaterThan(0), AUTOSAVE); + const keys = Object.keys(saved[saved.length - 1].fields ?? {}).sort(); + expect(keys).toEqual(['a_project.amount', 'a_project.internal_notes', 'a_project.name', 'a_project.owner_id']); + }); +}); 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 82622382243..02be4e12ff7 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx @@ -123,6 +123,7 @@ import { useCanAuthorMetadata, useCanReadAuthoringTrail } from '../../hooks/useC import { errorCodeIs } from '@object-ui/types'; import type { MetadataError } from '@object-ui/data-objectstack'; import { PermissionSetCloneDialog } from './PermissionSetCloneDialog.js'; +import { isStudioHiddenSystemField } from '../studio-design/studioHiddenSystemField.js'; import type { PermissionSetCloned } from './permission-set-clone.js'; import { mergePermissionSlice, @@ -331,6 +332,13 @@ function owdLabel(t: (k: string) => string, value: string): string { interface FieldSummary { name: string; label?: string; + /** + * objectstack#27404 — the platform's two marks on the served definition, kept + * so the field rows can leave out its bookkeeping columns + * (`isStudioHiddenSystemField`, objectui#11780). + */ + system?: boolean; + hidden?: boolean; } /** @@ -751,6 +759,10 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, // Snapshot of the last loaded/saved draft — the anchor `isDirty` compares // against. `null` until the first load lands (nothing to be dirty against). const baselineRef = React.useRef(null); + // objectstack#27404 — the `${object}.${field}` keys the last loaded/saved set + // carries: a platform bookkeeping column with a stored entry keeps its row + // while the draft clears it, so the admin still sees what the save changes. + const [baselineFieldKeys, setBaselineFieldKeys] = React.useState>(() => new Set()); /** Set the draft AND re-anchor the dirty baseline to it (load + post-save). */ const resetDraftBaseline = React.useCallback((next: PermissionSetDraft) => { try { @@ -758,6 +770,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, } catch { baselineRef.current = null; } + setBaselineFieldKeys(new Set(Object.keys(next.fields ?? {}))); // objectui#9484 — a fresh identity, ALWAYS. `isDirty` below is a `useMemo` // keyed on `draft`, while the anchor it reads lives in a ref: hand // `setDraft` the object that is already the draft and React bails out on @@ -1028,11 +1041,18 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, const raw = obj?.fields; const list: FieldSummary[] = ( Array.isArray(raw) - ? raw.map((f: any) => ({ name: String(f?.name ?? ''), label: f?.label })) + ? raw.map((f: any) => ({ + name: String(f?.name ?? ''), + label: f?.label, + system: f?.system === true, + hidden: f?.hidden === true, + })) : raw && typeof raw === 'object' ? Object.entries(raw).map(([name, f]: [string, any]) => ({ name: String(f?.name ?? name), label: f?.label, + system: f?.system === true, + hidden: f?.hidden === true, })) : [] ) @@ -2192,6 +2212,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, draft={draft} expanded={expanded} fieldsByObject={fieldsByObject} + baselineFieldKeys={baselineFieldKeys} writable={writable} objectActions={OBJECT_ACTIONS} t={t} @@ -2382,6 +2403,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, draft={draft} expanded={expanded} fieldsByObject={fieldsByObject} + baselineFieldKeys={baselineFieldKeys} writable={writable} objectActions={OBJECT_ACTIONS} t={t} @@ -2440,6 +2462,8 @@ interface PermissionTableProps { draft: PermissionSetDraft; expanded: Set; fieldsByObject: Record; + /** objectstack#27404 — field-entry keys of the last loaded/saved set (`FieldsSubTable`). */ + baselineFieldKeys: ReadonlySet; writable: boolean; objectActions: ReturnType; t: (key: string) => string; @@ -2464,6 +2488,7 @@ function PermissionTable({ draft, expanded, fieldsByObject, + baselineFieldKeys, writable, objectActions, t, @@ -2654,6 +2679,7 @@ function PermissionTable({ objectPerm={perm} fields={fieldsByObject[o.name]} fieldsState={draft.fields ?? {}} + baselineFieldKeys={baselineFieldKeys} writable={writable} t={t} onFieldPerm={onFieldPerm} @@ -2762,6 +2788,7 @@ function StudioObjectMatrix({ draft, expanded, fieldsByObject, + baselineFieldKeys, writable, objectActions, t, @@ -2814,6 +2841,7 @@ function StudioObjectMatrix({ open={expanded.has(o.name)} fieldsState={draft.fields ?? {}} fields={fieldsByObject[o.name]} + baselineFieldKeys={baselineFieldKeys} writable={writable} objectActions={objectActions} divider={divider} @@ -2840,6 +2868,7 @@ function StudioObjectRow({ open, fieldsState, fields, + baselineFieldKeys, writable, objectActions, divider, @@ -2860,6 +2889,7 @@ function StudioObjectRow({ open: boolean; fieldsState: Record; fields: FieldSummary[] | undefined; + baselineFieldKeys: ReadonlySet; writable: boolean; objectActions: ReturnType; divider: (i: number) => string; @@ -3013,6 +3043,7 @@ function StudioObjectRow({ objectPerm={perm} fields={fields} fieldsState={fieldsState} + baselineFieldKeys={baselineFieldKeys} writable={writable} t={t} onFieldPerm={onFieldPerm} @@ -3138,6 +3169,7 @@ function FieldsSubTable({ objectPerm, fields, fieldsState, + baselineFieldKeys, writable, t, onFieldPerm, @@ -3150,6 +3182,8 @@ function FieldsSubTable({ objectPerm: ObjectPerm; fields: FieldSummary[] | undefined; fieldsState: Record; + /** Field-entry keys of the last loaded/saved set: such a row stays while the draft clears it. */ + baselineFieldKeys: ReadonlySet; writable: boolean; t: (key: string) => string; onFieldPerm: (objectName: string, fieldName: string, action: keyof FieldPerm, value: boolean) => void; @@ -3179,13 +3213,26 @@ function FieldsSubTable({ ); } - if (fields.length === 0) { + // objectstack#27404 — the rows OFFER the object's own fields. A column the + // platform injects and hides (the `__search` companion, + // `owning_business_unit_id`, `organization_id`) is left out, judged by the + // Data rail's own predicate (`isStudioHiddenSystemField`, objectui#11780). + // One the set already carries an entry for, in the draft or in the last + // loaded/saved set, still draws as written (the objectui#13152 CONTROL), so + // the bulk shortcuts never reach a bookkeeping column the admin cannot see. + const offered = fields.filter( + (f) => + !isStudioHiddenSystemField({ system: f.system, hidden: f.hidden }) || + fieldsState[`${objectName}.${f.name}`] !== undefined || + baselineFieldKeys.has(`${objectName}.${f.name}`), + ); + if (offered.length === 0) { return
{t('perm.field.empty')}
; } const q = fieldFilter.trim().toLowerCase(); const visible = q - ? fields.filter((f) => f.name.toLowerCase().includes(q) || (f.label ?? '').toLowerCase().includes(q)) - : fields; + ? offered.filter((f) => f.name.toLowerCase().includes(q) || (f.label ?? '').toLowerCase().includes(q)) + : offered; const visibleNames = visible.map((f) => f.name); return (
@@ -3196,7 +3243,7 @@ function FieldsSubTable({ matrix scrolled sideways (its floor is wider than the Studio content column at 1440 px). */}
- {fields.length > 6 && ( + {offered.length > 6 && ( setFieldFilter(e.target.value)} @@ -3209,7 +3256,7 @@ function FieldsSubTable({ data-testid={`perm-field-count-${objectName}`} className="shrink-0 whitespace-nowrap text-[11px] tabular-nums text-muted-foreground" > - {visible.length} / {fields.length} + {visible.length} / {offered.length} {writable && (
@@ -3341,6 +3388,8 @@ function FieldsSubTable({ disabled={!writable} placeholder="status == 'draft'" objectName={objectName} + // Every field, the left-out ones too: a condition may + // still name one, and lint must not call it unknown. fieldNames={fields.map((x) => x.name)} clause="check" onLintChange={(issues) => onFieldWhenLint(objectName, f.name, issues)}