Skip to content

Commit 1a03af6

Browse files
os-zhuangclaude
andauthored
fix(approvals): Approval Center triage + drawer readability pass (#2762 P1-2/3/4/5, P2) (#2803)
- Inbox/cards surface the request amount inline (from the snapshot, preferring server-formatted payload_display); sort adds oldest-first and amount high→low next to newest-first (P1-3) - Flow-/system-initiated requests read "Flow-initiated" with a workflow icon instead of a bare person icon + "—" (desktop, mobile, drawer) (P1-4) - "Waiting on" chips dedupe a repeated approver to one chip with a ×N count, tooltip keeping every underlying id (P1-2) - DeclaredActionsBar maps the spec action variant enum onto Button variants (primary→default, danger→destructive) so Approve/Reject get hierarchy (P1-5) - Resolved lookup keys render as "Owner", not "Owner Id" (P2) - New approvalsInbox keys (flowOrigin, sort*) in all ten locales Refs objectui#2762 Claude-Session: https://claude.ai/code/session_01Cu48mLFUdRBmMh8Z8R3CVz Co-authored-by: Claude <noreply@anthropic.com>
1 parent ddea597 commit 1a03af6

14 files changed

Lines changed: 275 additions & 19 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@object-ui/app-shell": patch
3+
"@object-ui/i18n": patch
4+
---
5+
6+
fix(approvals): Approval Center triage + drawer readability pass (#2762 P1-2/P1-3/P1-4/P1-5/P2)
7+
8+
- **Decision-relevant data in the queue (P1-3)** — list rows and mobile cards
9+
now surface the request's amount/total inline (detected from the snapshot,
10+
preferring the server-formatted `payload_display` value), so a reviewer can
11+
triage without opening each request. A sort control adds "Oldest first" and
12+
"Amount (high→low)" alongside the default newest-first.
13+
- **Empty applicant column (P1-4)** — flow-/system-initiated requests (no human
14+
submitter) now read "Flow-initiated" with a workflow icon instead of a bare
15+
person icon + "—", in the desktop table, mobile card, and drawer.
16+
- **Approver chips deduped (P1-2)** — a person filling more than one approver
17+
slot rendered as N identical "Waiting on" chips; they collapse to one chip
18+
with a ×N count, the tooltip keeping every underlying id.
19+
- **Action hierarchy (P1-5)**`DeclaredActionsBar` maps the spec action
20+
`variant` enum onto the Button variants (`primary` → filled default,
21+
`danger` → destructive), so the drawer's Approve stands out and Reject reads
22+
as destructive once `@objectstack/plugin-approvals` declares them.
23+
- **Label polish (P2)**`owner_id`-style resolved lookup keys render as
24+
"Owner", not the awkward "Owner Id", in the drawer summary.
25+
26+
New `approvalsInbox` keys (`flowOrigin`, `sortBy`/`sortRecent`/`sortOldest`/
27+
`sortAmount`) added to all ten locales.

apps/console/src/pages/system/ApprovalsInboxPage.tsx

Lines changed: 165 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,12 @@ import {
8282
AlertCircle,
8383
CheckSquare,
8484
Search,
85+
ArrowUpDown,
8586
Copy,
8687
X,
8788
ExternalLink,
8889
User as UserIcon,
90+
Workflow,
8991
ChevronLeft,
9092
ChevronRight,
9193
Send,
@@ -159,6 +161,38 @@ function submitterDisplay(r: ApprovalRequestRow): string {
159161
function approverDisplay(a: string, r: ApprovalRequestRow): string {
160162
return r.pending_approver_names?.[a] || formatIdentity(a);
161163
}
164+
/**
165+
* Dedupe the pending-approver chips by display label (#2762 P1-2): a person
166+
* who fills more than one approver slot showed up as N identical chips. Collapse
167+
* them to one chip carrying a count, preserving first-seen order; the tooltip
168+
* keeps every underlying id so the raw slots stay inspectable.
169+
*/
170+
function approverChips(r: ApprovalRequestRow): Array<{ label: string; count: number; title: string }> {
171+
const order: string[] = [];
172+
const byLabel = new Map<string, { label: string; count: number; title: string }>();
173+
for (const a of r.pending_approvers || []) {
174+
const label = approverDisplay(a, r);
175+
const seen = byLabel.get(label);
176+
if (seen) {
177+
seen.count += 1;
178+
if (a && !seen.title.split(', ').includes(a)) seen.title += `, ${a}`;
179+
} else {
180+
byLabel.set(label, { label, count: 1, title: a || label });
181+
order.push(label);
182+
}
183+
}
184+
return order.map((l) => byLabel.get(l)!);
185+
}
186+
/**
187+
* A request with no human submitter — flow- or system-initiated (#2762 P1-4).
188+
* These rows have an empty `submitter_id` or a synthetic `flow:` / `system:`
189+
* actor, and rendering a bare person icon + "—" reads as missing data.
190+
*/
191+
function isSystemSubmitter(r: ApprovalRequestRow): boolean {
192+
if (r.submitter_name) return false;
193+
const id = (r.submitter_id || '').trim();
194+
return !id || id.startsWith('flow:') || id.startsWith('system:');
195+
}
162196
/** Object subtitle: schema label when resolved, else the machine name. */
163197
function objectDisplay(r: ApprovalRequestRow): string {
164198
return r.object_label || r.object_name;
@@ -197,7 +231,12 @@ const PAYLOAD_SYSTEM_KEYS = new Set([
197231
]);
198232

199233
function prettifyKey(k: string): string {
200-
return k.split('_').filter(Boolean).map(w => w.charAt(0).toUpperCase() + w.slice(1)).join(' ');
234+
const tokens = k.split('_').filter(Boolean);
235+
// Drop a trailing `id` token so a resolved lookup key reads as its subject —
236+
// `owner_id` → "Owner", not the awkward "Owner Id" (#2762 P2). Keep at least
237+
// one token (a bare `id` is already dropped as a system key upstream).
238+
if (tokens.length > 1 && tokens[tokens.length - 1].toLowerCase() === 'id') tokens.pop();
239+
return tokens.map(w => w.charAt(0).toUpperCase() + w.slice(1)).join(' ');
201240
}
202241

203242
function formatPayloadValue(key: string, v: unknown): string {
@@ -246,6 +285,36 @@ function payloadSummary(
246285
return out;
247286
}
248287

288+
/**
289+
* Amount-like keys worth surfacing in the queue so a reviewer can triage
290+
* without opening each request (#2762 P1-3). Deliberately narrow — a decision
291+
* turns on the amount/total/budget, not on every numeric field.
292+
*/
293+
const AMOUNT_KEY_RE = /(amount|total|price|value|cost|sum|budget|salary|fee|revenue|balance||||||)/i;
294+
295+
/**
296+
* The one decision-relevant numeric field (amount/total/…) of the snapshot,
297+
* for the inline list display and amount sort. Prefers the server-formatted
298+
* `payload_display` value (currency, etc.) but always keeps the raw number for
299+
* ordering. Null when the snapshot has no such field.
300+
*/
301+
function decisionAmountEntry(
302+
r: ApprovalRequestRow,
303+
): { label: string; value: number; display: string } | null {
304+
const payload = r.payload;
305+
if (!payload || typeof payload !== 'object' || Array.isArray(payload)) return null;
306+
for (const [k, v] of Object.entries(payload as Record<string, unknown>)) {
307+
if (PAYLOAD_SYSTEM_KEYS.has(k)) continue;
308+
if (!AMOUNT_KEY_RE.test(k)) continue;
309+
const num = typeof v === 'number'
310+
? v
311+
: (typeof v === 'string' && v.trim() !== '' && Number.isFinite(Number(v)) ? Number(v) : null);
312+
if (num == null || !Number.isFinite(num)) continue;
313+
return { label: prettifyKey(k), value: num, display: r.payload_display?.[k] ?? num.toLocaleString() };
314+
}
315+
return null;
316+
}
317+
249318
export function ApprovalsInboxPage() {
250319
const { t, language } = useObjectTranslation();
251320
const { user } = useAuth();
@@ -389,6 +458,10 @@ export function ApprovalsInboxPage() {
389458
const [processFilter, setProcessFilter] = useState<string>('all');
390459
const [objectFilter, setObjectFilter] = useState<string>('all');
391460
const [statusFilter, setStatusFilter] = useState<string>('all');
461+
// Client-side ordering of the visible rows (#2762 P1-3). Default keeps the
462+
// server's newest-first; the others let a reviewer triage by wait time or by
463+
// the decision-relevant amount.
464+
const [sortKey, setSortKey] = useState<'recent' | 'oldest' | 'amount'>('recent');
392465

393466
// Bulk selection (only meaningful on "pending" tab where the user can act)
394467
const [selectedRowIds, setSelectedRowIds] = useState<Set<string>>(new Set());
@@ -648,10 +721,10 @@ export function ApprovalsInboxPage() {
648721
return Array.from(set).sort();
649722
}, [rows]);
650723

651-
/** Client-side filtered rows shown in table. */
724+
/** Client-side filtered + sorted rows shown in table. */
652725
const filteredRows = useMemo(() => {
653726
const q = query.trim().toLowerCase();
654-
return rows.filter(r => {
727+
const matched = rows.filter(r => {
655728
if (processFilter !== 'all' && processLabel(r) !== processFilter) return false;
656729
if (objectFilter !== 'all' && r.object_name !== objectFilter) return false;
657730
if (statusFilter !== 'all' && r.status !== statusFilter) return false;
@@ -667,7 +740,25 @@ export function ApprovalsInboxPage() {
667740
].filter(Boolean).join(' ').toLowerCase();
668741
return hay.includes(q);
669742
});
670-
}, [rows, query, processFilter, objectFilter, statusFilter, tab]);
743+
if (sortKey === 'recent') return matched; // server order is already newest-first
744+
const sorted = [...matched];
745+
if (sortKey === 'amount') {
746+
// Highest amount first; rows without a detectable amount sink to the
747+
// bottom (keeping their relative newest-first order).
748+
sorted.sort((a, b) => {
749+
const av = decisionAmountEntry(a)?.value;
750+
const bv = decisionAmountEntry(b)?.value;
751+
if (av == null && bv == null) return 0;
752+
if (av == null) return 1;
753+
if (bv == null) return -1;
754+
return bv - av;
755+
});
756+
} else {
757+
// Oldest first — flip the newest-first submitted timestamp.
758+
sorted.sort((a, b) => (submittedAt(a) || '').localeCompare(submittedAt(b) || ''));
759+
}
760+
return sorted;
761+
}, [rows, query, processFilter, objectFilter, statusFilter, tab, sortKey]);
671762
/** Position of the open request within the visible list (drawer prev/next). */
672763
const drawerIndex = useMemo(
673764
() => (selectedId ? filteredRows.findIndex(r => r.id === selectedId) : -1),
@@ -876,6 +967,7 @@ export function ApprovalsInboxPage() {
876967
setProcessFilter('all');
877968
setObjectFilter('all');
878969
setQuery('');
970+
setSortKey('recent');
879971
setFocusIndex(-1);
880972
};
881973

@@ -898,6 +990,9 @@ export function ApprovalsInboxPage() {
898990
}
899991

900992
function RecordCell({ r }: { r: ApprovalRequestRow }) {
993+
// Surface the decision-relevant amount inline so a reviewer can triage the
994+
// queue without opening each request (#2762 P1-3).
995+
const amount = decisionAmountEntry(r);
901996
return (
902997
<div className="min-w-0">
903998
<Link
@@ -909,7 +1004,14 @@ export function ApprovalsInboxPage() {
9091004
<span className="truncate">{r.record_title || formatIdentity(r.record_id)}</span>
9101005
<ExternalLink className="h-3 w-3 shrink-0 text-muted-foreground" />
9111006
</Link>
912-
<div className="text-xs text-muted-foreground truncate">{objectDisplay(r)}</div>
1007+
<div className="text-xs text-muted-foreground truncate">
1008+
{objectDisplay(r)}
1009+
{amount && (
1010+
<span className="ml-1.5 font-medium text-foreground" title={`${amount.label}: ${amount.display}`}>
1011+
· {amount.display}
1012+
</span>
1013+
)}
1014+
</div>
9131015
</div>
9141016
);
9151017
}
@@ -1047,6 +1149,19 @@ export function ApprovalsInboxPage() {
10471149
</SelectContent>
10481150
</Select>
10491151
)}
1152+
{/* Triage ordering (#2762 P1-3): newest by default, or by wait
1153+
time / decision amount. */}
1154+
<Select value={sortKey} onValueChange={(v) => setSortKey(v as typeof sortKey)}>
1155+
<SelectTrigger className="h-8 w-auto min-w-[120px] text-sm">
1156+
<ArrowUpDown className="h-3.5 w-3.5 mr-1 text-muted-foreground" />
1157+
<SelectValue placeholder={tr('sortBy', 'Sort')} />
1158+
</SelectTrigger>
1159+
<SelectContent>
1160+
<SelectItem value="recent">{tr('sortRecent', 'Newest first')}</SelectItem>
1161+
<SelectItem value="oldest">{tr('sortOldest', 'Oldest first')}</SelectItem>
1162+
<SelectItem value="amount">{tr('sortAmount', 'Amount (high→low)')}</SelectItem>
1163+
</SelectContent>
1164+
</Select>
10501165
{hasFilters && (
10511166
<span className="text-xs text-muted-foreground">
10521167
{tr('filterCount', '{{shown}} of {{total}}', { shown: filteredRows.length, total: rows.length })}
@@ -1211,10 +1326,19 @@ export function ApprovalsInboxPage() {
12111326
<TableCell><RequestCell r={r} /></TableCell>
12121327
<TableCell><RecordCell r={r} /></TableCell>
12131328
<TableCell>
1214-
<div className="flex items-center gap-1.5 text-sm">
1215-
<UserIcon className="h-3.5 w-3.5 text-muted-foreground shrink-0" />
1216-
<span className="truncate" title={r.submitter_id || ''}>{submitterDisplay(r)}</span>
1217-
</div>
1329+
{isSystemSubmitter(r) ? (
1330+
// Flow-/system-initiated: name the origin instead of a
1331+
// bare person icon + "—" (#2762 P1-4).
1332+
<div className="flex items-center gap-1.5 text-sm text-muted-foreground">
1333+
<Workflow className="h-3.5 w-3.5 shrink-0" />
1334+
<span className="truncate italic">{tr('flowOrigin', 'Flow-initiated')}</span>
1335+
</div>
1336+
) : (
1337+
<div className="flex items-center gap-1.5 text-sm">
1338+
<UserIcon className="h-3.5 w-3.5 text-muted-foreground shrink-0" />
1339+
<span className="truncate" title={r.submitter_id || ''}>{submitterDisplay(r)}</span>
1340+
</div>
1341+
)}
12181342
</TableCell>
12191343
<TableCell><StatusBadge status={r.status} /></TableCell>
12201344
<TableCell
@@ -1254,11 +1378,23 @@ export function ApprovalsInboxPage() {
12541378
<div className="text-sm truncate">
12551379
{r.record_title || formatIdentity(r.record_id)}
12561380
<span className="text-muted-foreground text-xs ml-1.5">{objectDisplay(r)}</span>
1381+
{(() => {
1382+
const amount = decisionAmountEntry(r);
1383+
return amount ? (
1384+
<span className="text-xs ml-1.5 font-medium" title={amount.label}>· {amount.display}</span>
1385+
) : null;
1386+
})()}
12571387
</div>
12581388
<div className="flex items-center justify-between text-xs text-muted-foreground">
1259-
<span className="inline-flex items-center gap-1 truncate">
1260-
<UserIcon className="h-3 w-3" />{submitterDisplay(r)}
1261-
</span>
1389+
{isSystemSubmitter(r) ? (
1390+
<span className="inline-flex items-center gap-1 truncate italic">
1391+
<Workflow className="h-3 w-3" />{tr('flowOrigin', 'Flow-initiated')}
1392+
</span>
1393+
) : (
1394+
<span className="inline-flex items-center gap-1 truncate">
1395+
<UserIcon className="h-3 w-3" />{submitterDisplay(r)}
1396+
</span>
1397+
)}
12621398
<span className={cn('inline-flex items-center gap-1 whitespace-nowrap', agingClass(r))}>
12631399
<Clock className="h-3 w-3" />{formatRelative(submittedAt(r))}
12641400
</span>
@@ -1437,8 +1573,17 @@ export function ApprovalsInboxPage() {
14371573
</div>
14381574
<div className="text-right text-xs text-muted-foreground shrink-0">
14391575
<div className="inline-flex items-center gap-1">
1440-
<UserIcon className="h-3 w-3" />
1441-
<span title={selected.submitter_id || ''}>{submitterDisplay(selected)}</span>
1576+
{isSystemSubmitter(selected) ? (
1577+
<>
1578+
<Workflow className="h-3 w-3" />
1579+
<span className="italic">{tr('flowOrigin', 'Flow-initiated')}</span>
1580+
</>
1581+
) : (
1582+
<>
1583+
<UserIcon className="h-3 w-3" />
1584+
<span title={selected.submitter_id || ''}>{submitterDisplay(selected)}</span>
1585+
</>
1586+
)}
14421587
</div>
14431588
</div>
14441589
</div>
@@ -1537,9 +1682,12 @@ export function ApprovalsInboxPage() {
15371682
{tr('waitingOn', 'Waiting on')}
15381683
</div>
15391684
<div className="flex flex-wrap gap-1">
1540-
{(selected.pending_approvers || []).map((a, i) => (
1541-
<Badge key={i} variant="outline" className="text-[11px]" title={a}>
1542-
{approverDisplay(a, selected)}
1685+
{approverChips(selected).map((chip) => (
1686+
<Badge key={chip.label} variant="outline" className="text-[11px]" title={chip.title}>
1687+
{chip.label}
1688+
{chip.count > 1 && (
1689+
<span className="ml-1 text-muted-foreground">×{chip.count}</span>
1690+
)}
15431691
</Badge>
15441692
))}
15451693
</div>

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,17 @@ const DeclaredActionButton: React.FC<{
153153
if ((action as any).visible && !isVisible) return null;
154154

155155
const iconName = typeof (action as any).icon === 'string' ? (action as any).icon as string : undefined;
156-
const variant = (action as any).variant === 'primary'
156+
// Map the spec's action `variant` enum (primary|secondary|danger|ghost|link)
157+
// onto the Button's variants. `primary` → the filled default, `danger` →
158+
// `destructive` (the two names the enum and the Button component spell
159+
// differently); the rest pass through, and an undeclared variant stays
160+
// `outline` so a plain declared action still reads as a secondary button.
161+
const declaredVariant = (action as any).variant;
162+
const variant = declaredVariant === 'primary'
157163
? 'default'
158-
: ((action as any).variant || 'outline');
164+
: declaredVariant === 'danger'
165+
? 'destructive'
166+
: (declaredVariant || 'outline');
159167
const fallbackLabel = action.label || action.name || '';
160168
const label = action.name ? actionLabel(objectName, action.name, fallbackLabel) : fallbackLabel;
161169

packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,29 @@ describe('DeclaredActionsBar', () => {
172172
expect(dispatch.params).toEqual({ _rowRecord: REQUEST });
173173
});
174174

175+
it('maps the spec action `variant` onto Button variants', () => {
176+
render(
177+
<DeclaredActionsBar
178+
objectName="sys_approval_request"
179+
record={REQUEST}
180+
location="record_section"
181+
actions={[
182+
{ name: 'a_primary', type: 'api', label: 'P', target: '/x', locations: ['record_section'], variant: 'primary' },
183+
{ name: 'a_danger', type: 'api', label: 'D', target: '/x', locations: ['record_section'], variant: 'danger' },
184+
{ name: 'a_plain', type: 'api', label: 'N', target: '/x', locations: ['record_section'] },
185+
{ name: 'a_ghost', type: 'api', label: 'G', target: '/x', locations: ['record_section'], variant: 'ghost' },
186+
] as any}
187+
/>,
188+
);
189+
// primary → the filled default; danger → destructive (the two names the
190+
// spec enum and Button component spell differently); undeclared → outline;
191+
// the rest pass through unchanged.
192+
expect(screen.getByTestId('declared-action-a_primary')).toHaveAttribute('variant', 'default');
193+
expect(screen.getByTestId('declared-action-a_danger')).toHaveAttribute('variant', 'destructive');
194+
expect(screen.getByTestId('declared-action-a_plain')).toHaveAttribute('variant', 'outline');
195+
expect(screen.getByTestId('declared-action-a_ghost')).toHaveAttribute('variant', 'ghost');
196+
});
197+
175198
it('renders a labeled divider only when actions are present', () => {
176199
render(
177200
<DeclaredActionsBar

packages/i18n/src/locales/ar.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,6 +1986,11 @@ const ar = {
19861986
attachmentChip: 'مرفق',
19871987
approveOneTitle: 'الموافقة على "{{title}}"؟',
19881988
approveOneBody: 'ستتم الموافقة على الطلب بهويتك. لإضافة تعليق أو مرفق، افتح الطلب.',
1989+
flowOrigin: 'بدأها التدفق',
1990+
sortBy: 'ترتيب',
1991+
sortRecent: 'الأحدث أولاً',
1992+
sortOldest: 'الأقدم أولاً',
1993+
sortAmount: 'المبلغ (تنازلي)',
19891994
quickPhrase1: 'موافق — مستوفٍ للمتطلبات.',
19901995
quickPhrase2: 'موافقة مشروطة — يرجى متابعة التنفيذ.',
19911996
quickPhrase3: 'يرجى إضافة مستندات داعمة وإعادة التقديم.',

0 commit comments

Comments
 (0)