From 233965379b8878e75b5b51794773e8b65cde883c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 12:42:29 +0000 Subject: [PATCH] feat(app-shell): the served approval request page names who the pending step waits on (objectui#12482) The decision panel (`record:approval_decision`), which objectstack's served `sys_approval_request_detail` page already mounts, draws a "Waiting on" row: each pending approver named through app-shell's one approver-naming path (`approverChips` over the row's `pending_approver_names` and `pending_approver_groups`, and `useApproverDirectory` for a reference the server left unnamed), with a position's registry label and the countersign group each slot fills. One person in several quorum slots is one chip with a count (objectui#12077's case); an id nothing can name keeps the identity formatter's middle-truncated face, as the inbox's sheet draws it. No page edit, no spec row, no prop and no export: the panel reads the row it is bound to. `DecisionProgressIndicator` does not carry the names because it is drawn only with a tally, and a first_response node has none. Pins: the panel's own suite, and the served page on the real RecordDetailView over the console's routed approvals source. Claude-Session: https://claude.ai/code/session_01JZnppgHRp4sREcouhkQYDB Co-authored-by: Claude --- .../12482-decision-panel-names-approvers.md | 5 + ...aSource.servedPageApprovers-12482.test.tsx | 373 ++++++++++++++++++ .../ApprovalDecisionPanel.test.tsx | 157 +++++++- .../ApprovalDecisionPanel.tsx | 97 ++++- 4 files changed, 625 insertions(+), 7 deletions(-) create mode 100644 .changeset/12482-decision-panel-names-approvers.md create mode 100644 apps/console/src/services/approvalRequestsDataSource.servedPageApprovers-12482.test.tsx diff --git a/.changeset/12482-decision-panel-names-approvers.md b/.changeset/12482-decision-panel-names-approvers.md new file mode 100644 index 0000000000..4c91844232 --- /dev/null +++ b/.changeset/12482-decision-panel-names-approvers.md @@ -0,0 +1,5 @@ +--- +'@object-ui/app-shell': minor +--- + +The approval decision panel (`record:approval_decision`) names who a pending request waits on (objectui#12482, folding objectui#12077). Each pending approver is one chip under "Waiting on": the server's own name for a user id, a position's registry label (with the people who hold it) for a `position:` slot the server left unnamed, and the countersign group the slot fills. One person holding several quorum slots is one chip with a count; two people sharing a name in two groups stay two chips. An id nothing can name keeps the identity formatter's middle-truncated face, and every raw slot stays on hover only. So the request page objectstack serves now shows the approvers the approvals inbox's sheet shows, with no page, prop or spec change: the panel reads the request row it is already bound to. diff --git a/apps/console/src/services/approvalRequestsDataSource.servedPageApprovers-12482.test.tsx b/apps/console/src/services/approvalRequestsDataSource.servedPageApprovers-12482.test.tsx new file mode 100644 index 0000000000..0518216ed6 --- /dev/null +++ b/apps/console/src/services/approvalRequestsDataSource.servedPageApprovers-12482.test.tsx @@ -0,0 +1,373 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#12482 (face 1 of ruling C on objectui#12438; folds objectui#12077): + * the page objectstack serves for an approval request names each pending + * approver, with a position's label and the countersign group the slot fills, + * as the approvals inbox's request sheet names them. + * + * Mounted as objectui#12077's measurement mounted it: the REAL + * `RecordDetailView`, opened by the generic record route under the console's + * `ApprovalRecordReads`, so the request is read from the approvals get route + * exactly as the shipped code reads it. `SERVED_PAGE` is objectstack + * `plugin-approvals`' `sys_approval_request_detail` as it ships (copied whole, + * as `approvalRequestsDataSource.servedPage.test.tsx` copies it); the object + * definition is abridged to the fields that page reads, plus + * `pending_approvers` as the object declares it (a `textarea` in the `State` + * group). Only the HTTP transport, the console adapter and the metadata store + * are doubled. + * + * The names come from the row the get serves (`pending_approver_names`, + * `pending_approver_groups`) and, for a `position:` slot the server left + * unnamed, from the `position` registry the metadata store lists and the + * seat's holders the adapter reads. The face the sheet asserts is the + * reference: two people sharing a name in two groups stay two labelled chips + * (`ApprovalsInboxPage.characterizationPins.test.tsx`), and a position reads its + * registry label, never `positi…ager` (`ApprovalsInboxPage.approverHeadline-12110.test.tsx`). + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, within } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1', name: 'Ada', image: null }, activeOrganization: null }), +})); +vi.mock('@object-ui/collaboration', async (importOriginal) => ({ + ...(await importOriginal>()), + useRecordPresence: () => [], + PresenceAvatars: () => null, +})); +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +import { AdapterCtx, MetadataCtx, useAdapter } from '@object-ui/react'; +import { I18nProvider, type I18nConfig } from '@object-ui/i18n'; +import { RecordDetailView } from '@object-ui/app-shell'; +import { PageSchema } from '@objectstack/spec/ui'; +import { APPROVAL_ACTION_OBJECT, APPROVAL_REQUEST_OBJECT } from './approvalRequestsDataSource'; +import { ApprovalRecordReads } from '../AppContent'; + +const REQ = 'req_12482'; + +/** A user id of the length the auth service mints, which the sheet middle-truncates. */ +const ADA = 'usr_qFpo8Zk2LmN4pR7t'; + +/** The request as the approvals get serves it; each case overrides the slate. */ +const BASE_ROW = { + id: REQ, + display_title: 'invoice_approval · inv_1', + process_name: 'invoice_approval', + object_name: 'invoice', + record_id: 'inv_1', + status: 'pending', + current_step: 'manager_review', + updated_at: '2026-10-02T09:00:00Z', + viewer: { can_act: true, is_submitter: false, can_override: false }, +}; + +/** The countersign case: the sheet's characterization fixture, plus a position slot. */ +const COUNTERSIGN_ROW = { + ...BASE_ROW, + pending_approvers: ['u_fin', 'u_leg', 'position:sales_manager'], + pending_approver_names: { u_fin: 'Dev Admin', u_leg: 'Dev Admin' }, + pending_approver_groups: { u_fin: ['Finance'], u_leg: ['Legal'], 'position:sales_manager': ['Legal'] }, + decision_progress: { + behavior: 'per_group', + got: 0, + need: 2, + groups: [ + { group: 'Finance', got: 0, need: 1, satisfied: false }, + { group: 'Legal', got: 0, need: 1, satisfied: false }, + ], + }, +}; + +/** objectui#12077's case: one approver holding three quorum slots. */ +const QUORUM_ROW = { + ...BASE_ROW, + pending_approvers: [ADA, ADA, ADA], + pending_approver_names: { [ADA]: 'Ada Lovelace' }, + decision_progress: { behavior: 'quorum', got: 0, need: 2 }, +}; + +const CAN_ACT = { + dialect: 'cel', + source: 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true', +}; +const DECISION_PARAMS = [ + { name: 'comment', label: 'Comment', type: 'textarea', required: false }, + { name: 'attachments', label: 'Attachments', type: 'file', multiple: true, required: false }, +]; +const REQUEST_DEF = { + name: APPROVAL_REQUEST_OBJECT, + label: 'Approval Request', + pluralLabel: 'Approval Requests', + nameField: 'display_title', + titleFormat: '{process_name} · {record_id}', + highlightFields: ['process_name', 'object_name', 'record_id', 'status', 'current_step', 'submitter_id', 'updated_at'], + fields: { + id: { label: 'Request ID', type: 'text', readonly: true, group: 'System' }, + display_title: { label: 'Title', type: 'formula', returnType: 'text', group: 'Target' }, + process_name: { label: 'Source', type: 'text', group: 'Target' }, + object_name: { label: 'Object', type: 'text', group: 'Target' }, + record_id: { label: 'Record ID', type: 'text', referenceVia: 'object_name', group: 'Target' }, + submitter_id: { label: 'Submitter', type: 'lookup', reference: 'sys_user', group: 'Target' }, + status: { + label: 'Status', type: 'select', group: 'State', + options: [{ value: 'pending', label: 'Pending' }, { value: 'approved', label: 'Approved' }], + }, + current_step: { label: 'Current Step', type: 'text', group: 'State' }, + pending_approvers: { label: 'Pending Approvers', type: 'textarea', group: 'State' }, + updated_at: { label: 'Updated At', type: 'datetime', group: 'System' }, + }, + actions: [ + { + name: 'approval_approve', label: 'Approve', type: 'api', method: 'POST', + target: '/api/v1/approvals/requests/{id}/approve', params: DECISION_PARAMS, + visible: CAN_ACT, locations: ['record_section', 'list_item'], refreshAfter: true, + }, + { + name: 'approval_reject', label: 'Reject', type: 'api', method: 'POST', + target: '/api/v1/approvals/requests/{id}/reject', params: DECISION_PARAMS, + visible: CAN_ACT, locations: ['record_section', 'list_item'], refreshAfter: true, + }, + ], + enable: { apiMethods: ['get', 'list'] }, +}; +const ACTION_DEF = { + name: APPROVAL_ACTION_OBJECT, + label: 'Approval Action', + pluralLabel: 'Approval Actions', + nameField: 'display_title', + fields: { + id: { label: 'Action ID', type: 'text', readonly: true }, + request_id: { label: 'Request', type: 'lookup', reference: APPROVAL_REQUEST_OBJECT }, + step_name: { label: 'Step', type: 'text' }, + action: { label: 'Action', type: 'select', options: [{ value: 'submit', label: 'Submitted' }] }, + actor_id: { label: 'Actor', type: 'lookup', reference: 'sys_user' }, + comment: { label: 'Comment', type: 'textarea' }, + attachments: { label: 'Attachments', type: 'file', multiple: true }, + created_at: { label: 'Created At', type: 'datetime' }, + }, + enable: { apiMethods: ['get', 'list'] }, +}; +const USER_DEF = { name: 'sys_user', label: 'User', nameField: 'name', fields: { name: { label: 'Name', type: 'text' } } }; +const DEFS: Record = { + [APPROVAL_REQUEST_OBJECT]: REQUEST_DEF, + [APPROVAL_ACTION_OBJECT]: ACTION_DEF, + sys_user: USER_DEF, +}; + +/** objectstack `plugin-approvals`' `SysApprovalRequestDetailPage`, whole. */ +const SERVED_PAGE = { + name: 'sys_approval_request_detail', + label: 'Approval Request', + type: 'record', + object: 'sys_approval_request', + template: 'default', + kind: 'slotted', + isDefault: true, + regions: [], + slots: { + actions: [{ type: 'record:approval_decision' }], + highlights: { + type: 'record:highlights', + properties: { + fields: ['process_name', 'object_name', 'status', 'current_step', 'submitter_id', 'updated_at'], + }, + }, + tabs: { + type: 'page:tabs', + properties: { + tabStyle: 'line', + position: 'top', + items: [ + { + label: { en: 'Details', 'zh-CN': '详情', 'ja-JP': '詳細', 'es-ES': 'Detalles' }, + icon: 'file-text', + children: [{ type: 'record:details' }], + }, + { + label: { en: 'Timeline', 'zh-CN': '时间线', 'ja-JP': 'タイムライン', 'es-ES': 'Cronología' }, + icon: 'history', + children: [ + { + type: 'record:related_list', + properties: { + objectName: 'sys_approval_action', + relationshipField: 'request_id', + columns: ['created_at', 'action', 'actor_id', 'step_name', 'comment', 'attachments'], + sort: [{ field: 'created_at', order: 'desc' }], + limit: 50, + showViewAll: true, + title: { en: 'Timeline', 'zh-CN': '时间线', 'ja-JP': 'タイムライン', 'es-ES': 'Cronología' }, + }, + }, + ], + }, + ], + }, + }, + discussion: [], + }, +}; + +/** The `position` registry, as `GET /api/v1/meta/position` serves it. */ +const POSITIONS = [{ name: 'sales_manager', label: 'Regional Sales Manager' }]; + +function json(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { status, headers: { 'content-type': 'application/json' } }); +} + +/** The row the approvals get serves for this case. */ +let served: Record = BASE_ROW; + +const transport = vi.fn(async (input: RequestInfo | URL, init?: RequestInit): Promise => { + const raw = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url; + const url = new URL(raw, 'http://test.local'); + const method = (init?.method ?? (input instanceof Request ? input.method : 'GET')).toUpperCase(); + if (method === 'GET' && url.pathname === `/api/v1/approvals/requests/${REQ}`) return json(200, served); + if (method === 'GET' && url.pathname === `/api/v1/approvals/requests/${REQ}/actions`) return json(200, { data: [] }); + return json(200, { success: true, data: [] }); +}); +vi.stubGlobal('fetch', transport); + +/** The console adapter: the directory reads a position slot needs, and nothing else answered. */ +const host = { + find: vi.fn(async (resource: string, _params?: unknown) => { + if (resource === 'sys_user_position') return { data: [{ position: 'sales_manager', user_id: 'u_7' }], total: 1 }; + if (resource === 'sys_user') return { data: [{ id: 'u_7', full_name: 'Grace Hopper' }], total: 1 }; + return { data: [], total: 0 }; + }), + findOne: vi.fn(async (_resource: string, _id: string) => null), + create: vi.fn(), update: vi.fn(), delete: vi.fn(), + getObjectSchema: vi.fn(async (name: string) => DEFS[name] ?? null), +}; + +const ensureType = vi.fn(async (type: string) => (type === 'position' ? POSITIONS : [])); +const metadata = { + apps: [], objects: [REQUEST_DEF, ACTION_DEF, USER_DEF], dashboards: [], reports: [], + pages: [SERVED_PAGE], loading: false, error: null, + refresh: async () => {}, invalidate: () => {}, ensureType, + getItem: vi.fn(async (type: string, name: string) => (type === 'object' ? (DEFS[name] ?? null) : null)), + getItemsByType: () => [], + getTypeStatus: () => 'ready' as const, +}; + +const EN: I18nConfig = { defaultLanguage: 'en', detectBrowserLanguage: false }; + +/** `DefaultAppContent`'s generic record route: the view over `useAdapter()`. */ +function GenericRecordRoute() { + return {}} />; +} + +async function openServedPage(row: Record): Promise { + served = row; + render( + + + + + + + } /> + + + + + + , + ); + const panel = await screen.findByTestId('approval-decision-panel', undefined, { timeout: 8000 }); + await within(panel).findByTestId('declared-action-approval_approve', undefined, { timeout: 8000 }); + return panel; +} + +const chips = (panel: HTMLElement) => within(panel).queryAllByTestId('approver-chip'); +const pageText = () => document.body.textContent ?? ''; + +afterEach(() => { + cleanup(); + served = BASE_ROW; + host.find.mockClear(); + ensureType.mockClear(); +}); + +describe('the served request page names who the pending step waits on (objectui#12482)', () => { + it('the served page parses as a spec page, with no node or prop added for the names', () => { + const parsed = PageSchema.safeParse(SERVED_PAGE); + expect(parsed.success, JSON.stringify(parsed.error?.issues ?? [])).toBe(true); + expect(SERVED_PAGE.slots.actions).toEqual([{ type: 'record:approval_decision' }]); + }); + + it('names each pending approver, with the countersign group it fills and a position by its registry label', async () => { + const panel = await openServedPage(COUNTERSIGN_ROW); + const row = within(panel).getByTestId('approval-decision-pending-approvers'); + expect(row.textContent).toContain('Waiting on'); + + // The sheet's characterization case: two people sharing one name, in two + // groups, are two labelled chips, not one chip with a count. + expect(within(panel).getByTitle('u_fin').textContent).toBe('Dev Admin· Finance'); + expect(within(panel).getByTitle('u_leg').textContent).toBe('Dev Admin· Legal'); + + // The sheet's objectui#12110 case: a position reads its registry label. + // Re-queried each time: the chip is keyed by its label, so it is a new + // element once the label resolves. + await waitFor( + () => + expect(within(panel).getByTitle('position:sales_manager').textContent).toBe( + 'Regional Sales Manager· Grace Hopper· Legal', + ), + { timeout: 8000 }, + ); + expect(chips(panel)).toHaveLength(3); + }); + + it('no pending approver id is shown anywhere on the page, only on hover', async () => { + const panel = await openServedPage(COUNTERSIGN_ROW); + await waitFor( + () => expect(within(panel).getByTitle('position:sales_manager').textContent).toContain('Regional Sales Manager'), + { timeout: 8000 }, + ); + for (const raw of ['u_fin', 'u_leg', 'position:', 'positi…']) { + expect(pageText(), `${raw} is not printed`).not.toContain(raw); + } + }); + + it("objectui#12077's case: one approver in three quorum slots is named once, with a count, and the id is printed nowhere", async () => { + const panel = await openServedPage(QUORUM_ROW); + expect(chips(panel).map((chip) => chip.textContent)).toEqual(['Ada Lovelace×3']); + expect(pageText()).not.toContain('qFpo'); + expect(pageText().match(/Ada Lovelace/g) ?? []).toHaveLength(1); + }); + + it('a slate the approvals get named whole costs no directory read', async () => { + await openServedPage(QUORUM_ROW); + expect(ensureType).not.toHaveBeenCalledWith('position'); + const directoryReads = host.find.mock.calls.filter(([resource]) => + ['sys_user_position', 'sys_team', 'sys_business_unit'].includes(String(resource)), + ); + expect(directoryReads).toEqual([]); + }); + + it('CONTROL: a request with no pending approvers draws no "Waiting on" row, and still its tally and decisions', async () => { + const panel = await openServedPage({ ...QUORUM_ROW, pending_approvers: [], pending_approver_names: {} }); + expect(within(panel).queryByTestId('approval-decision-pending-approvers')).toBeNull(); + expect(chips(panel)).toHaveLength(0); + expect(within(panel).getByRole('progressbar')).toBeTruthy(); + expect(within(panel).getByTestId('declared-action-approval_reject')).toBeTruthy(); + }); + + it('CONTROL: an approver id nothing can name is drawn as the inbox sheet draws it, middle-truncated, whole only on hover', async () => { + const unnamed = 'zzUnresolvedIdXyz123'; + const panel = await openServedPage({ ...QUORUM_ROW, pending_approvers: [unnamed], pending_approver_names: {} }); + expect(within(panel).getByTitle(unnamed).textContent).toBe('zzUnre…z123'); + expect(pageText()).not.toContain(unnamed); + }); +}); diff --git a/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.test.tsx b/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.test.tsx index 6a28d082e2..afc13be13f 100644 --- a/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.test.tsx +++ b/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.test.tsx @@ -22,11 +22,17 @@ * Off a request page the node draws a localized notice instead of nothing * (objectui#12072), so a block placed on the wrong page is visible to its * author. On a request page whose row has not loaded it still draws nothing. + * + * On a pending request the panel names who the step waits on (objectui#12482), + * from the row it is bound to, as the approvals inbox's request sheet names + * them. The same face on the real record page, over the console's routed + * approvals source and objectstack's served page, is pinned in the console's + * `approvalRequestsDataSource.servedPageApprovers-12482.test.tsx`. */ import * as React from 'react'; -import { describe, it, expect, vi, afterEach } from 'vitest'; -import { cleanup, render, screen } from '@testing-library/react'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { cleanup, render, screen, waitFor, within } from '@testing-library/react'; const barProps: Array> = []; vi.mock('../DeclaredActionsBar.js', () => ({ @@ -37,10 +43,18 @@ vi.mock('../DeclaredActionsBar.js', () => ({ })); import { ComponentRegistry } from '@object-ui/core'; -import { RecordContextProvider, SchemaRenderer, subscribeDataChanges, type DataChange } from '@object-ui/react'; +import { + AdapterCtx, + MetadataCtx, + RecordContextProvider, + SchemaRenderer, + subscribeDataChanges, + type DataChange, +} from '@object-ui/react'; import { I18nProvider } from '@object-ui/i18n'; import { builtInLocales } from '@object-ui/i18n/locales'; import { ApprovalDecisionRenderer } from './ApprovalDecisionPanel'; +import { resetApproverDirectoryCache } from '../../hooks/useApproverDirectory'; // The package's registration leaf, imported for its side effect as the // package entry imports it (objectui#12342). import '../record-approval-decision-renderer'; @@ -192,3 +206,140 @@ describe('off an approval request page, a localized notice instead of nothing (o expect(packNotice('zh')).not.toBe(packNotice('en')); }); }); + +describe('names who the pending step waits on (objectui#12482)', () => { + /** The `position` registry, as the console's metadata store lists it. */ + const POSITIONS = [{ name: 'sales_manager', label: 'Regional Sales Manager' }]; + + /** The directory reads behind a position: who holds the seat. */ + function directoryAdapter() { + return { + find: vi.fn(async (object: string) => { + if (object === 'sys_user_position') return { data: [{ position: 'sales_manager', user_id: 'u_7' }] }; + if (object === 'sys_user') return { data: [{ id: 'u_7', full_name: 'Grace Hopper' }] }; + return { data: [] }; + }), + }; + } + + function mountNamed(data: Record) { + const adapter = directoryAdapter(); + const ensureType = vi.fn(async (type: string) => (type === 'position' ? POSITIONS : [])); + const store = { ensureType } as unknown as React.ContextType; + render( + + + + + + + , + ); + return { adapter, ensureType }; + } + + const chips = () => screen.queryAllByTestId('approver-chip'); + const panelText = () => screen.getByTestId('approval-decision-panel').textContent ?? ''; + + beforeEach(() => resetApproverDirectoryCache()); + + it('two people sharing one name in two countersign groups stay two chips, each named and labelled with its group', () => { + mountNamed({ + ...REQUEST, + pending_approvers: ['u_fin', 'u_leg'], + pending_approver_names: { u_fin: 'Dev Admin', u_leg: 'Dev Admin' }, + pending_approver_groups: { u_fin: ['Finance'], u_leg: ['Legal'] }, + decision_progress: { + behavior: 'per_group', + got: 0, + need: 2, + groups: [ + { group: 'Finance', got: 0, need: 1, satisfied: false }, + { group: 'Legal', got: 0, need: 1, satisfied: false }, + ], + }, + }); + const row = screen.getByTestId('approval-decision-pending-approvers'); + expect(row.textContent).toContain('Waiting on'); + expect(chips().map((chip) => chip.textContent)).toEqual(['Dev Admin· Finance', 'Dev Admin· Legal']); + expect(chips().map((chip) => chip.getAttribute('title'))).toEqual(['u_fin', 'u_leg']); + expect(panelText()).not.toContain('u_fin'); + expect(panelText()).not.toContain('u_leg'); + }); + + it('one person holding three quorum slots is one named chip with a count, never the raw id (objectui#12077)', () => { + const id = 'usr_qFpo8Zk2LmN4pR7t'; + mountNamed({ + ...REQUEST, + pending_approvers: [id, id, id], + pending_approver_names: { [id]: 'Ada Lovelace' }, + }); + expect(chips().map((chip) => chip.textContent)).toEqual(['Ada Lovelace×3']); + expect(chips()[0].getAttribute('title')).toBe(id); + expect(panelText()).not.toContain('qFpo'); + }); + + it("a position the server left unnamed reads its registry label and its holder, never the raw reference", async () => { + const { ensureType, adapter } = mountNamed({ ...REQUEST, pending_approvers: ['position:sales_manager'] }); + // Re-queried each time: the chip is keyed by its label, so it is a new + // element once the label resolves. + await waitFor(() => + expect(screen.getByTitle('position:sales_manager').textContent).toBe('Regional Sales Manager· Grace Hopper'), + ); + expect(ensureType).toHaveBeenCalledWith('position'); + expect(adapter.find.mock.calls.map(([object]) => object)).toEqual(['sys_user_position', 'sys_user']); + expect(panelText()).not.toContain('position:'); + expect(panelText()).not.toContain('positi…'); + }); + + it("CONTROL: the server's own name wins, and the directory is asked nothing", () => { + const { ensureType, adapter } = mountNamed({ + ...REQUEST, + pending_approvers: ['position:sales_manager'], + pending_approver_names: { 'position:sales_manager': 'Sales Manager (served)' }, + }); + expect(screen.getByTitle('position:sales_manager').textContent).toBe('Sales Manager (served)'); + expect(ensureType).not.toHaveBeenCalled(); + expect(adapter.find).not.toHaveBeenCalled(); + }); + + it('CONTROL: an id nothing can name keeps the face the inbox sheet draws for it, the raw id on hover only', () => { + mountNamed({ ...REQUEST, pending_approvers: ['zzUnresolvedIdXyz123'] }); + const chip = screen.getByTitle('zzUnresolvedIdXyz123'); + expect(chip.textContent).toBe('zzUnre…z123'); + expect(panelText()).not.toContain('zzUnresolvedIdXyz123'); + }); + + it('CONTROL: a request with no pending approvers draws no "Waiting on" row, and still its tally and decisions', () => { + mountNamed({ ...REQUEST, pending_approvers: [] }); + expect(screen.queryByTestId('approval-decision-pending-approvers')).toBeNull(); + expect(chips()).toHaveLength(0); + expect(screen.getByRole('progressbar')).toBeTruthy(); + expect(screen.getByTestId('declared-actions-probe')).toBeTruthy(); + }); + + it('CONTROL: a decided request names no one, even when its row still lists slots', () => { + const { ensureType, adapter } = mountNamed({ + ...REQUEST, + status: 'approved', + decision_progress: undefined, + pending_approvers: ['u_fin', 'position:sales_manager'], + pending_approver_names: { u_fin: 'Dev Admin' }, + }); + expect(screen.queryByTestId('approval-decision-pending-approvers')).toBeNull(); + expect(within(screen.getByTestId('approval-decision-panel')).queryByText(/Dev Admin/)).toBeNull(); + expect(ensureType).not.toHaveBeenCalled(); + expect(adapter.find).not.toHaveBeenCalled(); + }); + + it('a first_response node, which carries no tally, still names who it waits on', () => { + mountNamed({ + ...REQUEST, + decision_progress: undefined, + pending_approvers: ['u_fin'], + pending_approver_names: { u_fin: 'Dev Admin' }, + }); + expect(screen.queryByRole('progressbar')).toBeNull(); + expect(chips().map((chip) => chip.textContent)).toEqual(['Dev Admin']); + }); +}); diff --git a/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.tsx b/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.tsx index 8ec18349d8..7ca961a601 100644 --- a/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.tsx +++ b/packages/app-shell/src/views/approval-decision/ApprovalDecisionPanel.tsx @@ -11,13 +11,17 @@ * of objectui#2763; ruling 6079807016, letter 乙), the page component type * `record:approval_decision`. * - * One node, two parts, both already shipped and both reused as they are: + * One node, three parts, each drawn by a piece already shipped: * * 1. **Progress** — `DecisionProgressIndicator` (A3), drawn from the request's * server-computed `decision_progress` tally. It is present only on a * pending node that aggregates more than one decision, and the panel draws * it only then; nothing is re-derived here. - * 2. **Decisions** — `DeclaredActionsBar` over `sys_approval_request`'s OWN + * 2. **Who the pending step waits on** (objectui#12482) — each pending + * approver named, with a position's label and the countersign group the + * slot fills, as the approvals inbox's request sheet names them. See + * "Naming the pending approvers" below. + * 3. **Decisions** — `DeclaredActionsBar` over `sys_approval_request`'s OWN * declared actions at `record_section`, the location and the bar the * business record page already runs them through (objectui#3055). Every * decision is an ordinary declared `type:'api'` action, its form is @@ -26,6 +30,33 @@ * the request's server-computed `viewer` block. The panel adds no * per-action code, so a new decision action is metadata alone. * + * ## Naming the pending approvers (objectui#12482) + * + * The request row the page binds already carries everything a name needs: + * `pending_approvers` (the slots), `pending_approver_names` (the server's own + * id -> name answer for user ids, from the same approvals get) and, on a + * `per_group` node, `pending_approver_groups` (slot -> the group or groups it + * fills). So the names are drawn here, from that row, with no prop, no page + * edit and no spec row. + * + * The drawing is app-shell's one approver-naming path, the one the business + * record's approvals panel already draws its "Waiting on" chips with: + * `approverChips` (collapsed by name and group, so one person holding three + * quorum slots is one chip with a count, and two people sharing a name in two + * groups stay two chips) over `approverDisplay`. A reference the server left + * unnamed (`position:`, `team:`, …) is looked up once per slate by + * `useApproverDirectory`, whose module cache the decision buttons on the same + * request share; a slate the server named whole asks nothing. An id nothing + * can name keeps the identity formatter's face, the one the inbox's sheet + * draws for it, and every raw slot stays in the chip's `title`. + * + * Why not inside `DecisionProgressIndicator`: the indicator draws a tally, and + * a `first_response` node carries none, so the indicator is not drawn there at + * all while its approvers still wait. The inbox's sheet and the business + * record's panel both draw the tally and the names as two parts under two + * gates (a tally present; a pending request with a slot), and so does this + * panel. + * * ## No authorable props * * The panel reads the record context and nothing else: the request row the @@ -69,11 +100,18 @@ import * as React from 'react'; import { notifyDataChanged, useObjectTranslation, useRecordContext } from '@object-ui/react'; -import { cn } from '@object-ui/components'; +import { cn, Badge } from '@object-ui/components'; import { DecisionProgressIndicator } from '../approval-progress/DecisionProgressIndicator.js'; import { DeclaredActionsBar } from '../DeclaredActionsBar.js'; +import { approverChips } from '../RecordApprovalsPanel.js'; import { RECORD_APPROVAL_ACTION_LOCATION, SYS_APPROVAL_REQUEST_OBJECT } from '../recordApprovalActions.js'; import type { ApprovalRequestLite } from '../../hooks/useRecordApprovals.js'; +import { useApproverDirectory } from '../../hooks/useApproverDirectory.js'; +import { + approverCopyFrom, + unresolvedApproverRefs, + type ApproverDisplayCopy, +} from '../../utils/approverIdentity.js'; /** The timeline object a decision appends a row to. */ const APPROVAL_ACTION_OBJECT = 'sys_approval_action'; @@ -88,7 +126,57 @@ export interface ApprovalDecisionPanelProps { className?: string; } -/** The two parts over one request row. Module-internal: no package export. */ +/** + * Who the pending step waits on: one chip per approver, named, with a + * position's label and the countersign group the slot fills (objectui#12482). + * Drawn on a pending request with at least one slot, as the inbox's sheet + * draws its "Waiting on" row; nothing otherwise. + */ +function PendingApprovers({ request }: { request: ApprovalRequestLite }) { + const { t } = useObjectTranslation(); + const pending = request.status === 'pending'; + // Only the references the server left unnamed, and none on a decided + // request, so a slate the approvals get named whole costs no request. + const refs = React.useMemo( + () => (pending ? unresolvedApproverRefs(request) : []), + [pending, request], + ); + const directory = useApproverDirectory(refs); + const copy = React.useMemo( + () => approverCopyFrom(t as unknown as Parameters[0]), + [t], + ); + if (!pending) return null; + const chips = approverChips(request, { directory, copy }); + if (chips.length === 0) return null; + return ( +
+
+ {String(t('approvalsInbox.waitingOn', { defaultValue: 'Waiting on' }))} +
+
+ {chips.map((chip, i) => ( + + {chip.label} + {chip.detail && · {chip.detail}} + {chip.group && · {chip.group}} + {chip.count > 1 && ×{chip.count}} + + ))} +
+
+ ); +} + +/** The three parts over one request row. Module-internal: no package export. */ export function ApprovalDecisionPanel({ request, className }: ApprovalDecisionPanelProps) { const progress = request.decision_progress; const eligible = (request.pending_approvers || []).length; @@ -102,6 +190,7 @@ export function ApprovalDecisionPanel({ request, className }: ApprovalDecisionPa return (
{progress && } +