diff --git a/.changeset/6595-retire-allowrestore-allowpurge-columns.md b/.changeset/6595-retire-allowrestore-allowpurge-columns.md new file mode 100644 index 0000000000..0fd0663db4 --- /dev/null +++ b/.changeset/6595-retire-allowrestore-allowpurge-columns.md @@ -0,0 +1,37 @@ +--- +'@object-ui/app-shell': patch +--- + +The metadata-admin permission matrix no longer authors the retired object-permission +bits `allowRestore` / `allowPurge` (objectui#6595). + +The `Re` and `Pu` columns, their two typed fields, the two preview rows, and the +"Purge (hard delete) granted without Delete" sanity check are gone, together with the +two column tooltips in both locale tables. `allowTransfer` is enforced upstream +(objectstack#3004) and is untouched — it stays a column. + +Both removed keys gated `restore` / `purge` ObjectQL operations that **have never +existed**: a dispatched restore/purge is denied unconditionally by the evaluator's +fail-closed destructive-operation backstop. So every tick of those checkboxes wrote a +grant no runtime has ever read, and the preview lint warned about a combination whose +danger was entirely notional. `@objectstack/spec` retired both keys as `retiredKey()` +tombstones (objectstack#12497; maintainer ruling 2026-08-26 accepting objectstack#1883 +recommendation B, ADR-0049 enforce-or-remove), which turns the same checkbox into a save +that hard-fails at publish once the bump carrying that retirement reaches this repo. + +**The return path is named in the code, not just here**: both keys come back with the M2 +lifecycle initiative, whose restart is recorded upstream on objectstack#1883. The +tombstone on `ObjectPerm` in `permission-slice.ts` states it, and the two `retiredLifecycleKeys` +pins name it again — a future reader who wonders where the columns went finds the answer +at each of the three sites the removal touched. + +**A stored legacy value is carried through, not stripped.** It is no longer modelled and +no longer authorable, so it rides through save untouched exactly as any key this editor +does not model does — the record-level index signature on `PermissionSetDraft` states +that rule, and `updateObjectPerm`'s spread applies it per row. Stripping was deliberately +left out: the installed `@objectstack/spec` (17.2.0, measured 2026-08-27) still **accepts** +both keys at permission parse, so a strip today would delete stored data the schema still +honours. Once the bump lands and a carried value becomes a body the schema refuses, +strip-on-load becomes correct — that is objectui#4644's resolution for `indexed`, and it +belongs to the bump PR. The pin that records today's posture says so in its own header, +so the bump replaces it deliberately rather than deleting a red. diff --git a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx new file mode 100644 index 0000000000..6641764b0a --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.retiredLifecycleKeys.test.tsx @@ -0,0 +1,171 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pins that the Access matrix never AUTHORS the retired lifecycle bits + * `allowRestore` / `allowPurge` (objectui#6595). + * + * Both keys gated `restore` / `purge` ObjectQL operations that have never + * existed — a dispatched restore/purge is denied unconditionally by the + * evaluator's fail-closed destructive-operation backstop — so every tick of + * the `Re` / `Pu` checkboxes was a grant no runtime has ever read. + * `@objectstack/spec` retired both as `retiredKey()` tombstones + * (objectstack#12497; maintainer ruling 2026-08-26 accepting objectstack#1883 + * recommendation B, ADR-0049 enforce-or-remove). They return with the M2 + * lifecycle initiative, whose restart is recorded on objectstack#1883. + * + * Two directions are pinned, because a missing column is only half of it: + * - the COLUMN SET, not merely the absence of two keys. A set assertion is + * what stops a retired key drifting back in beside a live one, and it is + * the direction that also proves `allowTransfer` — enforced upstream, and + * explicitly out of this removal — is still authorable. + * - what reaches the WIRE. "Grant all" seeds a row from the column list, so + * the key set it writes is the real product of this change; asserting on + * the saved payload catches a column list that drifts back into the seed + * without a header cell to show for it. + * + * ## The one assertion the spec bump is expected to revisit + * + * `carries a stored legacy value through untouched` pins TODAY's posture: the + * installed `@objectstack/spec` (17.2.0, measured 2026-08-27) still ACCEPTS + * both keys at permission parse, so stripping a stored value here would delete + * data the schema still honours. Once the bump carrying the retirement lands, + * a carried-through value becomes a body the schema REFUSES, and strip-on-load + * becomes correct (objectui#4644's resolution for `indexed`). That is the bump + * PR's change to make, deliberately, replacing this assertion — not a red to + * be quietly deleted. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +/** The object-permission keys the matrix may author, in column order. */ +const LIVE_COLUMN_SHORTS = ['C', 'R', 'U', 'D', 'Tr', 'VA', 'MA']; + +let clientImpl: any; +let saved: Record | null = null; + +function makeClient(set: Record) { + return { + layered: async () => ({ effective: set, code: null, overlay: null, overlayScope: null }), + getDraft: async () => null, + list: async (type: string) => (type === 'object' ? [{ item: { name: 'a_account' } }] : []), + get: async (type: string) => (type === 'object' ? { fields: [] } : null), + save: async (_t: string, _n: string, payload: Record) => { + saved = payload; + return payload; + }, + } as any; +} + +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => clientImpl, + useMetadataTypes: () => ({ + loading: false, + error: null, + entries: [{ type: 'permission', label: 'Permission', allowOrgOverride: true }], + }), +})); +vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null })); +vi.mock('@object-ui/fields', () => ({ + CapabilityMultiSelectField: () =>
, + parseCapabilityNames: (v: unknown) => (typeof v === 'string' ? JSON.parse(v) : []), +})); + +import { PermissionMatrixEditPage } from './PermissionMatrixEditor'; + +afterEach(() => { + cleanup(); + saved = null; +}); + +async function renderSet(objects: Record = {}) { + clientImpl = makeClient({ + name: 'sales_perms', + label: 'Sales', + objects, + fields: {}, + }); + render( + + + , + ); + await screen.findByText('Sales'); +} + +/** Click Save and return the payload the client was handed. */ +async function save() { + fireEvent.click(screen.getByRole('button', { name: /^Save$/ })); + await waitFor(() => expect(saved).not.toBeNull()); + return saved!; +} + +describe('PermissionMatrixEditor · retired lifecycle keys (objectui#6595)', () => { + it('offers exactly the live capability columns — no Re, no Pu', async () => { + await renderSet(); + + 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); + // 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'); + expect(headers).not.toContain('Pu'); + }); + + it('offers no Restore / Purge checkbox, and still offers Transfer', async () => { + await renderSet({ a_account: { allowRead: true } }); + + expect(screen.queryByLabelText(/restore/i)).toBeNull(); + expect(screen.queryByLabelText(/purge/i)).toBeNull(); + // Falsification: `allowTransfer` is enforced upstream and is NOT part of + // this removal — if it vanished too, the assertions above would be passing + // for the wrong reason. + expect(screen.getByLabelText('a_account Transfer ownership')).toBeTruthy(); + }); + + it('"Grant all" writes exactly the live keys — the retired pair cannot ride along', async () => { + await renderSet({ a_account: {} }); + + fireEvent.click(screen.getAllByRole('button', { name: /^All$/ })[0]); + const payload = await save(); + + const row = payload.objects.a_account; + expect(Object.keys(row).sort()).toEqual( + [ + 'allowCreate', + 'allowRead', + 'allowEdit', + 'allowDelete', + 'allowTransfer', + 'viewAllRecords', + 'modifyAllRecords', + ].sort(), + ); + expect('allowRestore' in row).toBe(false); + expect('allowPurge' in row).toBe(false); + }); + + it('carries a stored legacy value through untouched rather than authoring it', async () => { + // What an older build of this editor wrote. Read the header note above + // before changing this: it pins today's posture, and the spec bump that + // lands the retirement is the change that replaces it with strip-on-load. + await renderSet({ a_account: { allowRead: true, allowRestore: true, allowPurge: true } }); + + // Not authorable: no control renders for either key… + expect(screen.queryByLabelText(/restore/i)).toBeNull(); + expect(screen.queryByLabelText(/purge/i)).toBeNull(); + + // …and an unrelated edit does not silently delete them either. + fireEvent.click(screen.getByLabelText('a_account Create')); + const payload = await save(); + + const row = payload.objects.a_account; + expect(row.allowCreate).toBe(true); + expect(row.allowRestore).toBe(true); + expect(row.allowPurge).toBe(true); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx index ddaf05675d..e834b8b2fd 100644 --- a/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx +++ b/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.tsx @@ -7,7 +7,7 @@ * Set / Profile metadata item: * * • Top section — object-level CRUD + VAMA (View All / Modify All) - * + lifecycle (Transfer / Restore / Purge). + * + lifecycle (Transfer). * • Lower section — field-level R/W for the fields of any object * selected from the table above. * @@ -177,8 +177,11 @@ function getObjectActions( { key: 'allowEdit', short: 'U', tip: translate('perm.action.edit', locale) }, { key: 'allowDelete', short: 'D', tip: translate('perm.action.delete', locale) }, { key: 'allowTransfer', short: 'Tr', tip: translate('perm.action.transfer', locale) }, - { key: 'allowRestore', short: 'Re', tip: translate('perm.action.restore', locale) }, - { key: 'allowPurge', short: 'Pu', tip: translate('perm.action.purge', locale) }, + // No `Re` (allowRestore) / `Pu` (allowPurge) columns: both keys are retired + // (objectui#6595 — see the tombstone on `ObjectPerm` in `permission-slice` + // for the full account and the M2 return path on objectstack#1883). They + // gated ObjectQL operations that have never existed, so every tick was a + // grant no runtime read. `allowTransfer` is enforced upstream and stays. { key: 'viewAllRecords', short: 'VA', tip: translate('perm.action.viewAll', locale) }, { key: 'modifyAllRecords', short: 'MA', tip: translate('perm.action.modifyAll', locale) }, ]; @@ -1013,7 +1016,7 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved, {/* Column legend — the matrix header cells already carry a native `title` tooltip per column, but a hover-only affordance on - unfamiliar two-letter abbreviations (Tr/Re/Pu/VA/MA) is easy to + unfamiliar two-letter abbreviations (Tr/VA/MA) is easy to miss. Spell them out once, up front. */}
{OBJECT_ACTIONS.map((a) => ( @@ -1154,9 +1157,11 @@ function PermissionTable({ onOpenOwd, }: PermissionTableProps) { return ( - // objectui#2600 B3 — the fixed columns (object + 9 CRUD + bulk) need ~960px; + // objectui#2600 B3 — the fixed columns (object + 7 CRUD + bulk) need ~960px; // a min-width makes the enclosing overflow-auto container scroll instead of // 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. diff --git a/packages/app-shell/src/views/metadata-admin/i18n.ts b/packages/app-shell/src/views/metadata-admin/i18n.ts index 97fbcee723..b80f109b64 100644 --- a/packages/app-shell/src/views/metadata-admin/i18n.ts +++ b/packages/app-shell/src/views/metadata-admin/i18n.ts @@ -1144,8 +1144,6 @@ const ENGINE_STRINGS_EN: Record = { 'perm.action.edit': 'Edit', 'perm.action.delete': 'Delete', 'perm.action.transfer': 'Transfer ownership', - 'perm.action.restore': 'Restore deleted records (reserved — deletes are hard today)', - 'perm.action.purge': 'Hard delete (purge)', 'perm.action.viewAll': 'View All Records (bypass sharing)', 'perm.action.modifyAll': 'Modify All Records (bypass sharing)', 'perm.col.object': 'Object', @@ -3007,8 +3005,6 @@ const ENGINE_STRINGS_ZH: Record = { 'perm.action.edit': '编辑', 'perm.action.delete': '删除', 'perm.action.transfer': '转移所有者', - 'perm.action.restore': '恢复已删除记录（预留 — 当前为硬删除）', - 'perm.action.purge': '彻底删除', 'perm.action.viewAll': '查看所有记录（绕过共享规则）', 'perm.action.modifyAll': '修改所有记录（绕过共享规则）', 'perm.col.object': '对象', diff --git a/packages/app-shell/src/views/metadata-admin/permission-slice.ts b/packages/app-shell/src/views/metadata-admin/permission-slice.ts index 23d2173eb4..dd14740b58 100644 --- a/packages/app-shell/src/views/metadata-admin/permission-slice.ts +++ b/packages/app-shell/src/views/metadata-admin/permission-slice.ts @@ -24,8 +24,28 @@ export interface ObjectPerm { allowEdit?: boolean; allowDelete?: boolean; allowTransfer?: boolean; - allowRestore?: boolean; - allowPurge?: boolean; + // `allowRestore` / `allowPurge` were REMOVED here (objectui#6595). Both gated + // `restore` / `purge` ObjectQL operations that have never existed — a + // dispatched restore/purge is denied unconditionally by the evaluator's + // fail-closed destructive-operation backstop — so the matrix authored two + // checkboxes no runtime has ever read. `@objectstack/spec` retired both keys + // as `retiredKey()` tombstones (objectstack#12497; maintainer ruling + // 2026-08-26 accepting objectstack#1883 recommendation B, ADR-0049 + // enforce-or-remove). + // + // THE RETURN PATH: both keys come back with the M2 lifecycle initiative, + // whose restart is recorded upstream on objectstack#1883. Restoring them + // means restoring three things together — these fields, the columns in + // `PermissionMatrixEditor`, the rows in `previews/PermissionPreview` — and + // only alongside the operations that make them enforceable. + // + // A value written by an older editor is not modelled here and not authorable. + // It is carried through save untouched, as any key this editor does not model + // is (`PermissionSetDraft`'s index signature below states that rule for the + // record; `updateObjectPerm`'s spread applies it per row). Stripping it is + // NOT this change: the installed spec still accepts both keys, so a strip + // today would delete stored data the schema still honours. That belongs with + // the `@objectstack/spec` bump that lands the retirement. viewAllRecords?: boolean; modifyAllRecords?: boolean; } diff --git a/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.retiredLifecycleKeys.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.retiredLifecycleKeys.test.tsx new file mode 100644 index 0000000000..0f210c5163 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.retiredLifecycleKeys.test.tsx @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pins that the Permission preview no longer renders the retired lifecycle + * bits `allowRestore` / `allowPurge`, nor lints over them (objectui#6595). + * + * The preview is the reviewer's read of a permission set, so it carried two + * columns and one sanity check ("Purge (hard delete) granted without Delete") + * for keys whose `restore` / `purge` ObjectQL operations have never existed — + * a dispatched restore/purge is denied unconditionally by the evaluator's + * fail-closed destructive-operation backstop. `@objectstack/spec` retired both + * as `retiredKey()` tombstones (objectstack#12497; maintainer ruling + * 2026-08-26 accepting objectstack#1883 recommendation B, ADR-0049 + * enforce-or-remove); they return with the M2 lifecycle initiative, whose + * restart is recorded on objectstack#1883. + * + * The lint is the half worth stating: a warning over a key that can no longer + * be granted cannot fire for a real reason, but it CAN fire for a stored + * legacy value — telling a reviewer to go fix a grant the authoring surface no + * longer offers, with no control to fix it with. + * + * Every assertion here carries its falsification in the same render: the + * capability set is pinned whole (so a live column cannot go missing behind a + * green "no Restore column"), and the retired lint's removal is measured on a + * draft that still trips the lints that stayed. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { PermissionPreview } from './PermissionPreview'; + +afterEach(cleanup); + +/** Capability columns, in matrix order — the whole set, not a sample. */ +const LIVE_CAPS = ['C', 'R', 'U', 'D', 'E', 'T', 'V*', 'M*']; + +function renderPreview(objects: Record) { + return render( + , + ); +} + +describe('PermissionPreview · retired lifecycle keys (objectui#6595)', () => { + it('renders exactly the live capability columns — Restore and Purge are gone', () => { + renderPreview({ opportunity: { allowRead: true } }); + + const headers = screen.getAllByRole('columnheader').map((th) => th.textContent?.trim() ?? ''); + // Object + capabilities + Scope. + expect(headers.slice(1, -1)).toEqual(LIVE_CAPS); + expect(screen.queryByTitle('Restore')).toBeNull(); + expect(screen.queryByTitle('Purge')).toBeNull(); + // Falsification: the neighbours of the two removed rows must survive — + // `allowTransfer` is enforced upstream and explicitly stays, and + // `allowExport` sits directly beside it (objectstack#4115 added it). + expect(screen.getByTitle('Transfer')).toBeTruthy(); + expect(screen.getByTitle('Export')).toBeTruthy(); + }); + + it('drops the "Purge without Delete" lint while the surviving lints still fire', () => { + // A stored legacy grant, exactly the shape that used to trip the lint: + // purge granted, delete not. Typed loosely because the key is retired — + // once the spec bump lands, `ObjectPermission` will not name it. + renderPreview({ + opportunity: { allowRead: true, allowEdit: true, allowPurge: true, modifyAllRecords: true }, + }); + + expect(screen.queryByText(/Purge \(hard delete\) granted without Delete/)).toBeNull(); + + // Falsification in the same render: the lints that stayed still fire, so + // the assertion above cannot pass merely because the banner is missing. + expect(screen.getByText(/Modify All without View All/)).toBeTruthy(); + }); + + it('renders a stored legacy value as no column at all, not as a granted chip', () => { + renderPreview({ opportunity: { allowRead: true, allowRestore: true, allowPurge: true } }); + + const row = screen.getByText('opportunity').closest('tr')!; + // Object + 8 capabilities + Scope. A stale key adds no cell: it is not a + // capability this surface knows, so it renders nowhere rather than as an + // unlabelled grant. + expect(row.querySelectorAll('td')).toHaveLength(LIVE_CAPS.length + 2); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.tsx b/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.tsx index 1861fbaeea..b25ee9ad30 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/PermissionPreview.tsx @@ -11,8 +11,8 @@ * 1. Header strip: name, profile/permission-set flag, system perms * count, tab perms count, RLS rule count. * 2. Object × CRUD-VAMA grid. Each row is one object; each column - * is one capability (Create/Read/Edit/Delete/Transfer/Restore/ - * Purge/ViewAll/ModifyAll). Cells are colored chips — green when + * is one capability (Create/Read/Edit/Delete/Export/Transfer/ + * ViewAll/ModifyAll). Cells are colored chips — green when * granted, neutral when not, amber when "View All" or "Modify * All" is on (highlighting the bypass). * 3. Field-level security: grouped by object, only fields with a @@ -56,8 +56,11 @@ const CAPS: Array<{ key: keyof ObjectPermission; short: string; long: string; da { key: 'allowDelete', short: 'D', long: 'Delete' }, { key: 'allowExport', short: 'E', long: 'Export' }, { key: 'allowTransfer', short: 'T', long: 'Transfer' }, - { key: 'allowRestore', short: 'Re', long: 'Restore' }, - { key: 'allowPurge', short: 'P', long: 'Purge', danger: true }, + // Restore / Purge were removed with their authoring columns (objectui#6595): + // `@objectstack/spec` retired both keys, and the operations they claimed to + // gate have never existed. The `keyof ObjectPermission` typing above is what + // keeps this honest in the other direction — once the spec bump carrying the + // retirement lands, re-adding either row stops compiling. { key: 'viewAllRecords', short: 'V*', long: 'View All', danger: true }, { key: 'modifyAllRecords', short: 'M*', long: 'Modify All', danger: true }, ]; @@ -83,7 +86,8 @@ function findWarnings(objects: Record): Warning[] { if (p.allowEdit && !p.allowRead) out.push({ object: obj, message: 'Edit granted without Read (record updates will fail).' }); if (p.allowDelete && !p.allowRead) out.push({ object: obj, message: 'Delete granted without Read.' }); if (p.modifyAllRecords && !p.viewAllRecords) out.push({ object: obj, message: 'Modify All without View All — modifications may target invisible records.' }); - if (p.allowPurge && !p.allowDelete) out.push({ object: obj, message: 'Purge (hard delete) granted without Delete.' }); + // No "Purge without Delete" lint: `allowPurge` is retired (objectui#6595), + // so the combination it warned about can no longer be authored. // Same class as Modify-All-without-View-All, one axis down: a write scope // wider than the read scope lets a user edit records they cannot see. const read = p.readScope ? SCOPE_ORDER.indexOf(p.readScope) : -1; @@ -326,13 +330,13 @@ function Legend() { granted - bypass (View/Modify All, Purge) + bypass (View/Modify All) not granted - C R U D = CRUD · T Re P = Transfer/Restore/Purge · V* M* = View/Modify All + C R U D = CRUD · E T = Export/Transfer · V* M* = View/Modify All );