Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

Commit 4069115

Browse files
committed
fix(app-shell): Position preview lists declared permissionSets (objectui#13304)
The callout read only name / label / description and always told the admin to bind the position to Permission Sets. A draft that declares `permissionSets` (ADR-0131 D4) now lists them by machine name in declared order; the "grants nothing on its own" note shows only when none are declared (absent, empty, or not an array). Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XVp19E1MYUTFM3dnkwz8gB
1 parent 3aafc0d commit 4069115

5 files changed

Lines changed: 132 additions & 15 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
The Position preview lists the permission sets a position declares in `permissionSets` (ADR-0131 D4), by machine name in declared order. The "positions don't grant data access on their own — bind it to Permission Sets" note now shows only for a position that declares none (objectui#13304).

‎packages/app-shell/src/views/metadata-admin/i18n.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1717,6 +1717,10 @@ const ENGINE_STRINGS_EN: Record<string, string> = {
17171717
'engine.positionPreview.bind':
17181718
'Bind {name} to one or more {permissionSets} to control CRUD-VAMA, field access, and tab visibility. Record visibility comes from the business-unit tree and sharing rules, not from the position.',
17191719
'engine.positionPreview.permissionSets': 'Permission Sets',
1720+
// objectui#13304 — a position that declares `permissionSets` (ADR-0131 D4).
1721+
'engine.positionPreview.grantsVia': 'This position grants access through these {permissionSets}:',
1722+
'engine.positionPreview.visibility':
1723+
'Record visibility comes from the business-unit tree and sharing rules, not from the position.',
17201724
// objectui#10202 — the read-only picklist page (the kind is package-owned).
17211725
'engine.picklistPreview.empty': 'This picklist has no name.',
17221726
'engine.picklistPreview.readOnly':
@@ -5306,6 +5310,9 @@ const ENGINE_STRINGS_ZH: Record<string, string> = {
53065310
'engine.positionPreview.bind':
53075311
'将 {name} 绑定到一个或多个 {permissionSets},以控制 CRUD-VAMA、字段访问和标签页可见性。记录的可见性来自业务单元树和共享规则,而不是岗位。',
53085312
'engine.positionPreview.permissionSets': '权限集',
5313+
// objectui#13304 — a position that declares `permissionSets` (zh).
5314+
'engine.positionPreview.grantsVia': '该岗位通过以下{permissionSets}授予访问权限:',
5315+
'engine.positionPreview.visibility': '记录的可见性来自业务单元树和共享规则,而不是岗位。',
53095316
// objectui#10202 — the read-only picklist page (zh).
53105317
'engine.picklistPreview.empty': '该选项列表没有名称。',
53115318
'engine.picklistPreview.readOnly': '只读:选项列表归声明它的软件包所有,不能在这里新建、编辑或删除。',
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* PositionPreview lists the permission sets a position declares (objectui#13304).
5+
*
6+
* `PositionSchema.permissionSets` (ADR-0131 D4) names the sets a position
7+
* distributes. The preview read only name / label / description and always
8+
* rendered "Positions don't grant data access on their own … bind it to
9+
* Permission Sets" — telling an admin to do what the draft had already done.
10+
*
11+
* The callout now lists the declared sets by machine name, in declared order,
12+
* and keeps the "grants nothing on its own" note for a position that declares
13+
* none: absent, empty, or not an array.
14+
*/
15+
16+
import { describe, it, expect, afterEach } from 'vitest';
17+
import { render, cleanup } from '@testing-library/react';
18+
import { PositionPreview } from './PositionPreview';
19+
import { t } from '../i18n';
20+
21+
afterEach(cleanup);
22+
23+
const EN = 'en-US';
24+
const NO_ACCESS = t('engine.positionPreview.noAccess', EN);
25+
26+
function mount(draft: Record<string, unknown>) {
27+
return render(<PositionPreview type="position" name="service_manager" draft={draft} locale={EN} />);
28+
}
29+
30+
/** The machine names the callout lists, in rendered order. */
31+
function listedSets(container: HTMLElement): string[] {
32+
return Array.from(container.querySelectorAll('li code')).map((el) => el.textContent ?? '');
33+
}
34+
35+
const BASE = { name: 'service_manager', label: 'Service Manager' };
36+
37+
describe('PositionPreview — declared permissionSets (objectui#13304)', () => {
38+
it('lists every declared set by machine name, in declared order, and drops the "grants nothing" note', () => {
39+
const { container } = mount({ ...BASE, permissionSets: ['service_agent', 'support_viewer'] });
40+
expect(listedSets(container)).toEqual(['service_agent', 'support_viewer']);
41+
expect(container.textContent).not.toContain(NO_ACCESS);
42+
});
43+
44+
it('CONTROL: a draft with no permissionSets renders the existing note', () => {
45+
const { container } = mount({ ...BASE });
46+
expect(container.textContent).toContain(NO_ACCESS);
47+
expect(listedSets(container)).toEqual([]);
48+
});
49+
50+
it('CONTROL: an empty permissionSets array renders the existing note', () => {
51+
const { container } = mount({ ...BASE, permissionSets: [] });
52+
expect(container.textContent).toContain(NO_ACCESS);
53+
expect(listedSets(container)).toEqual([]);
54+
});
55+
56+
it('a permissionSets value that is not an array (a string) declares none', () => {
57+
const { container } = mount({ ...BASE, permissionSets: 'service_agent' });
58+
expect(container.textContent).toContain(NO_ACCESS);
59+
expect(listedSets(container)).toEqual([]);
60+
});
61+
});

‎packages/app-shell/src/views/metadata-admin/previews/PositionPreview.tsx‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,23 @@
33
/**
44
* PositionPreview — read-only summary of a Position draft (ADR-0090 D3).
55
*
6-
* The Position schema is intentionally minimal — name / label /
7-
* description — and deliberately FLAT: no parent, no hierarchy. The
8-
* visibility hierarchy lives on the business-unit tree, not here, so the
9-
* preview's job is to put those fields into the mental model an operator
6+
* The Position schema is deliberately FLAT: no parent, no hierarchy. The
7+
* visibility hierarchy lives on the business-unit tree, not here. What a
8+
* position does carry, besides name / label / description, is
9+
* `permissionSets` (ADR-0131 D4): the permission sets it distributes, by
10+
* name. The preview puts those fields into the mental model an operator
1011
* carries:
1112
*
1213
* • Header card with label and machine name.
1314
* • Description block.
14-
* • A "permissions are managed separately" note pointing the user to
15-
* Permission Sets that grant CRUD-VAMA — this is the most common
16-
* confusion when first authoring a position.
15+
* • A permissions callout. When the draft declares `permissionSets`, it
16+
* lists them by machine name in declared order — that is where the
17+
* position's access comes from. When it declares none, it says the
18+
* position grants nothing on its own and points the user to Permission
19+
* Sets — the most common confusion when first authoring a position.
20+
*
21+
* The preview reads the draft only: set names are shown as written, with no
22+
* fetch for their labels.
1723
*/
1824

1925
import * as React from 'react';
@@ -28,6 +34,11 @@ export function PositionPreview({ name, draft, locale }: MetadataPreviewProps) {
2834
const positionName = String(d.name ?? name ?? '');
2935
const label = String(d.label ?? positionName);
3036
const description = (d.description as string | undefined) ?? '';
37+
// ADR-0131 D4 — the sets this position distributes, by name. Anything that
38+
// is not an array declares none.
39+
const permissionSets = Array.isArray(d.permissionSets)
40+
? d.permissionSets.filter((s): s is string => typeof s === 'string')
41+
: [];
3142

3243
if (!positionName) {
3344
return (
@@ -61,15 +72,33 @@ export function PositionPreview({ name, draft, locale }: MetadataPreviewProps) {
6172
<div className="rounded border border-blue-200 bg-blue-50 p-2.5 text-[11px] text-blue-900">
6273
<div className="flex items-start gap-1.5">
6374
<ShieldCheck className="h-3.5 w-3.5 mt-0.5 shrink-0" />
64-
<div>
65-
<div className="font-medium">{tr('engine.positionPreview.noAccess', locale)}</div>
66-
<div className="opacity-90 mt-0.5">
67-
{withNodes(tr('engine.positionPreview.bind', locale), {
68-
name: <code className="font-mono">{positionName}</code>,
69-
permissionSets: <strong>{tr('engine.positionPreview.permissionSets', locale)}</strong>,
70-
})}
75+
{permissionSets.length > 0 ? (
76+
<div>
77+
<div className="font-medium">
78+
{withNodes(tr('engine.positionPreview.grantsVia', locale), {
79+
permissionSets: <strong>{tr('engine.positionPreview.permissionSets', locale)}</strong>,
80+
})}
81+
</div>
82+
<ul className="mt-0.5 space-y-0.5">
83+
{permissionSets.map((set, i) => (
84+
<li key={`${i}:${set}`}>
85+
<code className="font-mono">{set}</code>
86+
</li>
87+
))}
88+
</ul>
89+
<div className="opacity-90 mt-0.5">{tr('engine.positionPreview.visibility', locale)}</div>
7190
</div>
72-
</div>
91+
) : (
92+
<div>
93+
<div className="font-medium">{tr('engine.positionPreview.noAccess', locale)}</div>
94+
<div className="opacity-90 mt-0.5">
95+
{withNodes(tr('engine.positionPreview.bind', locale), {
96+
name: <code className="font-mono">{positionName}</code>,
97+
permissionSets: <strong>{tr('engine.positionPreview.permissionSets', locale)}</strong>,
98+
})}
99+
</div>
100+
</div>
101+
)}
73102
</div>
74103
</div>
75104
</div>

‎packages/app-shell/src/views/metadata-admin/previews/adminPreviews.i18n-10862-s2.test.tsx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,21 @@ describe('PositionPreview reads the designer locale (objectui#10862)', () => {
410410
]);
411411
expectAsWritten(['Sales Rep', 'sales_rep', 'Works the pipeline.']);
412412
});
413+
414+
// objectui#13304 — a position that declares its sets lists them instead.
415+
it(`${lang}: the declared permission sets`, () => {
416+
const draft = { name: 'sales_rep', label: 'Sales Rep', permissionSets: ['sales_user', 'report_viewer'] };
417+
inLang(lang, <PositionPreview type="position" name="sales_rep" draft={draft} locale={LOCALE[lang]} />);
418+
expectSites(lang, [
419+
{ key: 'engine.positionPreview.permissionSets' },
420+
{
421+
key: 'engine.positionPreview.grantsVia',
422+
vars: { permissionSets: row(lang, 'engine.positionPreview.permissionSets') },
423+
},
424+
{ key: 'engine.positionPreview.visibility' },
425+
]);
426+
expectAsWritten(['Sales Rep', 'sales_rep', 'sales_user', 'report_viewer']);
427+
});
413428
}
414429
});
415430

0 commit comments

Comments
 (0)