From d99fcba2c13e8387401adecb3fde60bee9ac99e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 05:24:38 +0000 Subject: [PATCH 1/2] fix(setup): permission set field rows leave out the platform's bookkeeping columns A field the platform marks system AND hidden (__search, organization_id, owning_business_unit_id) is no longer offered in the permission set's field rows, judged by isStudioHiddenSystemField. A stored entry, in the draft or the last loaded/saved set, still draws as written. Claude-Session: https://claude.ai/code/session_01JQwxbWHPvxeSBbvjBYKZ8x Co-authored-by: Claude --- .../27404-permission-set-system-field-rows.md | 13 ++ ...onMatrixEditor.systemFields-27404.test.tsx | 151 ++++++++++++++++++ .../metadata-admin/PermissionMatrixEditor.tsx | 61 ++++++- 3 files changed, 219 insertions(+), 6 deletions(-) create mode 100644 .changeset/27404-permission-set-system-field-rows.md create mode 100644 ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.systemFields-27404.test.tsx 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..d3ba65c7cc4 --- /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, so the admin sees what the save drops', async () => { + await renderExpanded({ 'a_project.__search': { readable: false, editable: false } }); + fireEvent.click(screen.getByRole('button', { name: 'Clear' })); + const body = await (async () => { + await waitFor(() => expect(saved.length).toBeGreaterThan(0), AUTOSAVE); + return saved[saved.length - 1]; + })(); + expect(body.fields?.['a_project.__search']).toBeUndefined(); + expect(screen.getByTestId('perm-field-name-a_project-__search')).toBeInTheDocument(); + expect(screen.getByLabelText('a_project.__search readable')).toHaveAttribute('data-field-rule', 'inherited'); + }); + + 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)} From 15dca8d31848bb40f73dea93d47790cb449aa179 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 05:30:45 +0000 Subject: [PATCH 2/2] test(setup): a cleared stored bookkeeping row draws until the save lands Claude-Session: https://claude.ai/code/session_01JQwxbWHPvxeSBbvjBYKZ8x Co-authored-by: Claude --- ...ermissionMatrixEditor.systemFields-27404.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 index d3ba65c7cc4..032600aa317 100644 --- 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 @@ -129,16 +129,16 @@ describe('the field rows leave out the platform bookkeeping columns (objectstack expect(screen.queryByTestId('perm-field-name-a_project-organization_id')).toBeNull(); }); - it('CONTROL: a stored row the draft clears keeps its row, so the admin sees what the save drops', async () => { + 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' })); - const body = await (async () => { - await waitFor(() => expect(saved.length).toBeGreaterThan(0), AUTOSAVE); - return saved[saved.length - 1]; - })(); - expect(body.fields?.['a_project.__search']).toBeUndefined(); + // 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 () => {