Skip to content

Commit 5141617

Browse files
os-zhuangclaude
andauthored
test: pin the detail/form edit/delete gate to the server's effective operation set (#3546) (#2876)
Adds the regression coverage objectui#2832 left open on issue objectstack#3546: the detail header's Edit/Delete, the record-body inline-edit gate, and the form's blanket field lock were pinned only by "existing suites still pass". - app-shell RecordDetailView.headerActionGates (11 cases) - plugin-detail record-details.effectiveOps (7 cases) - plugin-form ObjectForm.effectiveOps (7 cases) Each suite pins the two properties that make this an intersection and not a union — a server grant never re-opens a bucket-closed affordance, and a userActions opt-in never survives a server denial — plus the backward-compatible undefined-effective-set fallback. The header gate is extracted from the RecordDetailView body into an exported resolveRecordHeaderActionGates() so it is reachable from a unit test, mirroring ObjectView's defaultListColumnsFromObject. Behavior-preserving; not added to the package's public export list. No changeset. Verified by mutation: dropping the effective-ops argument at each of the three call sites fails 14 of the new tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c992915 commit 5141617

4 files changed

Lines changed: 451 additions & 4 deletions

File tree

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* RecordDetailView — the detail header's Edit/Delete gate vs the server's
11+
* effective API operation set (#3546).
12+
*
13+
* PR-4 (objectui#2823) intersected the LIST/TOOLBAR affordances with the
14+
* server-resolved effective operation set (`/me/permissions` `apiOperations`).
15+
* #3546 extends that intersection to the DETAIL surface: the synthesized
16+
* `sys_edit` CTA (which also gates the record-body inline-edit session) and the
17+
* `sys_delete` overflow item must never be offered for an operation the server
18+
* would 405.
19+
*
20+
* These pin the resulting show/hide matrix, including the two properties that
21+
* make this an INTERSECTION and not a union:
22+
* • a server grant never re-opens an affordance the lifecycle bucket closed;
23+
* • a permissive bucket default never survives a server denial.
24+
* ...and the backward-compatible fallback: a missing effective set (unrestricted
25+
* object / old backend / no PermissionProvider) leaves today's bucket +
26+
* `userActions` decision untouched.
27+
*/
28+
29+
import { describe, it, expect } from 'vitest';
30+
import { resolveRecordHeaderActionGates } from './RecordDetailView';
31+
32+
// `platform` is the permissive bucket — edit + delete both default open, so it
33+
// isolates the effective-set intersection as the only variable.
34+
const platform = { name: 'crm_lead', managedBy: 'platform' };
35+
36+
describe('resolveRecordHeaderActionGates — effective API operations (#3546)', () => {
37+
it('full-CRUD effective set → Edit and Delete both offered', () => {
38+
expect(
39+
resolveRecordHeaderActionGates(platform, ['get', 'list', 'create', 'update', 'delete']),
40+
).toEqual({ edit: true, delete: true });
41+
});
42+
43+
it('read-only effective set → neither Edit nor Delete', () => {
44+
expect(resolveRecordHeaderActionGates(platform, ['get', 'list'])).toEqual({
45+
edit: false,
46+
delete: false,
47+
});
48+
});
49+
50+
it('update-only effective set → Edit offered, Delete hidden', () => {
51+
expect(resolveRecordHeaderActionGates(platform, ['get', 'list', 'update'])).toEqual({
52+
edit: true,
53+
delete: false,
54+
});
55+
});
56+
57+
it('delete-only effective set → Delete offered, Edit hidden', () => {
58+
expect(resolveRecordHeaderActionGates(platform, ['get', 'list', 'delete'])).toEqual({
59+
edit: false,
60+
delete: true,
61+
});
62+
});
63+
64+
it('empty effective set ("expose nothing") → neither', () => {
65+
expect(resolveRecordHeaderActionGates(platform, [])).toEqual({ edit: false, delete: false });
66+
});
67+
68+
it('undefined effective set (unrestricted / old backend) → bucket wins, both offered', () => {
69+
expect(resolveRecordHeaderActionGates(platform, undefined)).toEqual({
70+
edit: true,
71+
delete: true,
72+
});
73+
// Omitting the argument entirely must behave identically — this is the
74+
// pre-#3546 call shape every non-permission-aware caller still uses.
75+
expect(resolveRecordHeaderActionGates(platform)).toEqual({ edit: true, delete: true });
76+
});
77+
78+
it('null effective set is treated as absent, not as an empty set', () => {
79+
expect(resolveRecordHeaderActionGates(platform, null)).toEqual({ edit: true, delete: true });
80+
});
81+
82+
// ── Intersection, never union ────────────────────────────────────────────
83+
it('bucket-locked object stays locked even when the server allows update/delete', () => {
84+
// `system` (engine-owned) closes edit + delete by default. A permissive
85+
// server set must NOT re-open them — the server governs what it will
86+
// accept, the bucket governs what the product offers.
87+
expect(
88+
resolveRecordHeaderActionGates({ name: 'sys_automation_run', managedBy: 'system' }, [
89+
'get',
90+
'list',
91+
'create',
92+
'update',
93+
'delete',
94+
]),
95+
).toEqual({ edit: false, delete: false });
96+
});
97+
98+
it('userActions opt-in still loses to a server denial', () => {
99+
// sys_user opens `edit` via userActions (ADR-0103), but a caller whose
100+
// effective set lacks `update` must not see the Edit CTA.
101+
const sysUser = { name: 'sys_user', managedBy: 'better-auth', userActions: { edit: true } };
102+
expect(resolveRecordHeaderActionGates(sysUser, ['get', 'list', 'update'])).toEqual({
103+
edit: true,
104+
delete: false,
105+
});
106+
expect(resolveRecordHeaderActionGates(sysUser, ['get', 'list'])).toEqual({
107+
edit: false,
108+
delete: false,
109+
});
110+
});
111+
112+
it('admin-editable `config` bucket intersects the same way', () => {
113+
const webhook = { name: 'sys_webhook', managedBy: 'config' };
114+
expect(resolveRecordHeaderActionGates(webhook, ['get', 'list', 'update', 'delete'])).toEqual({
115+
edit: true,
116+
delete: true,
117+
});
118+
expect(resolveRecordHeaderActionGates(webhook, ['get', 'list', 'update'])).toEqual({
119+
edit: true,
120+
delete: false,
121+
});
122+
});
123+
124+
it('a missing objectDef falls back to the platform default (no crash)', () => {
125+
expect(resolveRecordHeaderActionGates(undefined, ['get', 'list', 'update'])).toEqual({
126+
edit: true,
127+
delete: false,
128+
});
129+
});
130+
});

packages/app-shell/src/views/RecordDetailView.tsx

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,34 @@ function isSecondaryField(fieldName: string, fieldDef: any): boolean {
112112
return SECONDARY_FIELD_NAME_HINTS.some((hint) => lc === hint || lc.endsWith(`_${hint}`));
113113
}
114114

115+
/**
116+
* Which system record-header affordances the record page may offer for this
117+
* object — the primary `sys_edit` CTA (which also gates the record-body
118+
* inline-edit session) and the `sys_delete` overflow item.
119+
*
120+
* [#3546] Each bit is the object's resolved CRUD affordance (lifecycle bucket +
121+
* `userActions`) INTERSECTED with the server-resolved effective API operation
122+
* set (`/me/permissions` `apiOperations`) — never a union. So a server grant can
123+
* never re-open an affordance the object's bucket closed, and a permissive
124+
* bucket default never survives the server denying `update` / `delete`. This is
125+
* the detail-surface end of the same intersection the list/toolbar surface
126+
* applies (objectui#2823). Passing `undefined` for `effectiveApiOperations`
127+
* (unrestricted object / old backend / no `PermissionProvider`) leaves the
128+
* bucket + `userActions` decision untouched — backward-compatible.
129+
*
130+
* Exported so the gate can be unit-tested directly: the record page itself is
131+
* wired into routing, auth, presence and data fetching too deeply to render in
132+
* a unit test, and this matrix is the part that must not regress. Same pattern
133+
* as `defaultListColumnsFromObject` in `ObjectView`.
134+
*/
135+
export function resolveRecordHeaderActionGates(
136+
objectDef: unknown,
137+
effectiveApiOperations?: readonly string[] | null,
138+
): { edit: boolean; delete: boolean } {
139+
const affordances = resolveCrudAffordances(objectDef as any, effectiveApiOperations);
140+
return { edit: affordances.edit, delete: affordances.delete };
141+
}
142+
115143
export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverride, recordIdOverride, embedded }: RecordDetailViewProps) {
116144

117145
const params = useParams<{
@@ -897,8 +925,8 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
897925
const { can: canOnObject, isLoaded: permissionsLoaded, getObjectApiOperations } = usePermissions();
898926
// [#3546] Server-resolved effective API operation set for this object
899927
// (`/me/permissions` `apiOperations`). Threaded as the 2nd arg into
900-
// `resolveCrudAffordances` for the detail header's Edit/Delete and the
901-
// record-body inline-edit gate, so the detail surface never offers an
928+
// `resolveRecordHeaderActionGates` for the detail header's Edit/Delete and
929+
// the record-body inline-edit gate, so the detail surface never offers an
902930
// operation the server would 405 — the same intersection the list/toolbar
903931
// surface already applies (objectui#2823). `undefined` (unrestricted object
904932
// / old backend) leaves the bucket affordances untouched (backward-compatible).
@@ -1748,7 +1776,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
17481776
// menu permanently — Delete must never surface as an inline red button
17491777
// just because an object has few actions.
17501778
const synthSystemActions: ActionDef[] = (() => {
1751-
const affordances = resolveCrudAffordances(objectDef as any, effectiveApiOperations);
1779+
const affordances = resolveRecordHeaderActionGates(objectDef, effectiveApiOperations);
17521780
const items: ActionDef[] = [];
17531781
if (affordances.edit) {
17541782
// Single primary Edit CTA → opens the full record form. Inline editing
@@ -1893,7 +1921,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
18931921
on backends that track the lock via approval requests only and
18941922
never materialize an `approval_status` field (objectui#2618). */}
18951923
<InlineEditProvider
1896-
canEdit={resolveCrudAffordances(objectDef as any, effectiveApiOperations).edit && !approvalLocked}
1924+
canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && !approvalLocked}
18971925
locked={approvalLocked}
18981926
lockedReason={t('detail.lockedTooltip', {
18991927
defaultValue: 'This record has a pending approval request; editing is locked',
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `record:details` — inline-edit vs the server's effective API operation set
11+
* (#3546).
12+
*
13+
* The record BODY offers per-field editing through a double-click / hover
14+
* pencil, gated by `isObjectInlineEditable`. #3546 intersects that gate with the
15+
* server-resolved effective operation set (`/me/permissions` `apiOperations`),
16+
* so a record page never offers inline editing the server would 405 on save.
17+
*
18+
* The renderer hands the resolved verdict to `<DetailView>` as
19+
* `schema.inlineEdit`; these stub DetailView and assert that flag across the
20+
* effective-set matrix — including that the intersection never becomes a union
21+
* (a server `update` grant can't re-open a bucket-locked object) and that a
22+
* missing effective set preserves today's behavior.
23+
*/
24+
25+
import * as React from 'react';
26+
import { describe, it, expect, vi, beforeEach } from 'vitest';
27+
import { render, cleanup } from '@testing-library/react';
28+
29+
const stub = {
30+
/** Server-resolved effective API operations for the bound object. */
31+
effectiveOps: undefined as string[] | undefined,
32+
/** The bound object's schema (lifecycle bucket + userActions). */
33+
objectSchema: { managedBy: 'platform' } as any,
34+
};
35+
36+
vi.mock('@object-ui/react', () => ({
37+
useRecordContext: () => ({
38+
objectName: 'crm_lead',
39+
recordId: 'rec_1',
40+
data: { id: 'rec_1', name: 'Acme' },
41+
dataSource: { update: async () => ({}) },
42+
refresh: async () => {},
43+
objectSchema: stub.objectSchema,
44+
}),
45+
useHighlightFieldNames: () => [] as string[],
46+
useSafeFieldLabel: () => ({
47+
sectionLabel: (_obj: string, _name: string, fallback: string) => fallback,
48+
}),
49+
}));
50+
51+
vi.mock('@object-ui/permissions', () => ({
52+
usePermissions: () => ({
53+
can: () => true,
54+
getObjectApiOperations: () => stub.effectiveOps,
55+
}),
56+
useFieldPermissions: () => ({ readableFields: (names: string[]) => names }),
57+
}));
58+
59+
// Capture the schema the renderer synthesizes — `inlineEdit` is the gate under
60+
// test. Stubbing DetailView also keeps the suite off the heavy field/registry
61+
// graph, which this behavior does not depend on.
62+
let captured: any = null;
63+
vi.mock('../../DetailView', () => ({
64+
DetailView: (props: any) => {
65+
captured = props?.schema;
66+
return <div data-testid="detail-view" />;
67+
},
68+
}));
69+
70+
import { RecordDetailsRenderer } from '../record-details';
71+
72+
/** Render the renderer and return the `inlineEdit` flag it resolved. */
73+
function inlineEditFor(schema: any = { fields: ['name'] }): boolean {
74+
captured = null;
75+
render(<RecordDetailsRenderer schema={schema} />);
76+
return captured?.inlineEdit;
77+
}
78+
79+
beforeEach(() => {
80+
stub.effectiveOps = undefined;
81+
stub.objectSchema = { managedBy: 'platform' };
82+
captured = null;
83+
cleanup();
84+
});
85+
86+
describe('record:details — inline-edit vs effective API operations (#3546)', () => {
87+
it('effective set WITH `update` → inline-edit offered', () => {
88+
stub.effectiveOps = ['get', 'list', 'update'];
89+
expect(inlineEditFor()).toBe(true);
90+
});
91+
92+
it('effective set WITHOUT `update` → inline-edit withheld', () => {
93+
// The #3546 behavior change: pre-fix this stayed `true` on a platform
94+
// object and the body offered a pencil the server would 405 on save.
95+
stub.effectiveOps = ['get', 'list'];
96+
expect(inlineEditFor()).toBe(false);
97+
});
98+
99+
it('empty effective set ("expose nothing") → inline-edit withheld', () => {
100+
stub.effectiveOps = [];
101+
expect(inlineEditFor()).toBe(false);
102+
});
103+
104+
it('undefined effective set (unrestricted / old backend) → bucket wins, offered', () => {
105+
stub.effectiveOps = undefined;
106+
expect(inlineEditFor()).toBe(true);
107+
});
108+
109+
it('bucket-locked object stays locked even when the server allows `update`', () => {
110+
// Intersection, never union: an engine-owned object is not user-editable
111+
// regardless of what the caller's effective set permits.
112+
stub.objectSchema = { managedBy: 'system' };
113+
stub.effectiveOps = ['get', 'list', 'create', 'update', 'delete'];
114+
expect(inlineEditFor()).toBe(false);
115+
});
116+
117+
it('userActions.edit opt-in still loses to a server denial', () => {
118+
stub.objectSchema = { managedBy: 'better-auth', userActions: { edit: true } };
119+
stub.effectiveOps = ['get', 'list', 'update'];
120+
expect(inlineEditFor()).toBe(true);
121+
122+
stub.effectiveOps = ['get', 'list'];
123+
expect(inlineEditFor()).toBe(false);
124+
});
125+
126+
it('author opt-out (`inlineEdit: false`) still wins over a permissive server set', () => {
127+
stub.effectiveOps = ['get', 'list', 'update'];
128+
expect(inlineEditFor({ fields: ['name'], inlineEdit: false })).toBe(false);
129+
});
130+
});

0 commit comments

Comments
 (0)