Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/approval-band-quorum-progress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
"@object-ui/app-shell": minor
"@object-ui/plugin-detail": minor
"@object-ui/react": minor
"@object-ui/i18n": minor
---

A record's approval band now shows the quorum / per-group tally the server already computes.

The showcase's `showcase_committee_quorum` node declares `behavior: 'quorum'` with
`minApprovals: 2` over three approvers, and even ships a pre-rendered
`"Committee Sign-off (2 of 3)"` label; `showcase_expense_signoff` declares
`per_group` (会签) with named manager / finance groups. On the business record
the approval band rendered none of it — the lock badge, the recall button and
the approve/reject actions were all correct, but a two-of-three committee step
looked exactly like a one-approver step. An approver could not see whether their
own click finalized the node or was one of three, which is the single fact a
quorum node exists to express (objectstack#4478).

Nothing was wrong on the wire, and nothing here papers over the server. The
framework computes `decision_progress` — `{ behavior, got, need, groups? }`,
derived from the node's own `node_config_json` snapshot, so the count a client
shows is the count the engine will enforce. **It attaches that block in
`getRequest` only**: `listRequests` deliberately skips it, because the
`sys_approval_action` tally it costs is per row and a list read may return
hundreds. The record header's `useRecordApprovals` reads
`GET /approvals/requests?object=…&recordId=…` — the list route — so the
enrichment was never in the payload it had. The hook now follows up with one
single read for the ONE pending row and folds the result onto it; a failed or
mismatched follow-up leaves the row exactly as the list sent it, so a display-only
enrichment can never take the approval panel down and no tally is ever invented.

`InlineEditProvider` carries the block through as `approvalProgress`, and the
DetailView approval band renders it beside the existing badge: a labelled
`role="progressbar"` with one tick per required approval for `quorum` /
`unanimous`, and for `per_group` a chip per group marking which have signed
(`finance 1/1` ✓, `manager 0/1`). Group names come from the flow author's own
config, so they need no locale strings; the three new label keys are added to all
ten packs. `first_response` nodes carry no `decision_progress` and are unchanged —
one decision is the whole step there, and a "1 of 1" bar would be noise.

Scored `minor` rather than `patch`: this is new observable rendering plus a new
public `approvalProgress` prop / `ApprovalProgress` type on `@object-ui/react`,
not a behavior correction inside an existing surface.
165 changes: 165 additions & 0 deletions packages/app-shell/src/hooks/useRecordApprovals.quorum.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* The record header must SEE the pending node's quorum tally (objectstack#4478).
*
* The showcase's `showcase_committee_quorum` node declares `behavior: 'quorum'`
* with `minApprovals: 2` over three approvers, and `showcase_expense_signoff`
* declares `behavior: 'per_group'` with named manager / finance groups. The
* framework turns both into a `decision_progress` block — but it attaches that
* block in `getRequest` ONLY: `listRequests`, which is the call this hook makes
* (`GET /approvals/requests?object=…&recordId=…`), deliberately skips the
* per-row `sys_approval_action` tally it costs.
*
* So the hook had the request and none of the progress, and the record's
* approval band could only ever render the lock badge. The fix is not a
* fallback — the server contract is right — it is to make the consumer read the
* enrichment where the server publishes it, with one follow-up single read for
* the ONE pending row.
*/

import { describe, it, expect, beforeEach, vi } from 'vitest';
import { renderHook, waitFor } from '@testing-library/react';
import { useRecordApprovals } from './useRecordApprovals';

/** The list row, exactly as `listRequests` sends it — no progress on it. */
const LIST_ROW = {
id: 'req_committee_1',
process_name: 'flow:showcase_committee_quorum',
object_name: 'showcase_expense_report',
record_id: 'AyG40_bAHSP_gi8T',
status: 'pending',
current_step: 'committee_signoff',
step_label: 'Committee Sign-off (2 of 3)',
lock_record: true,
pending_approvers: ['u_manager', 'u_finance', 'u_legal'],
};

/**
* The same row from `getRequest`, carrying the single-read enrichment the
* server computes from `node_config_json` (`behavior: 'quorum'`,
* `minApprovals: 2`, the three-approver `__approverGroups` slate).
*/
const DETAIL_ROW = {
...LIST_ROW,
decision_progress: { behavior: 'quorum', got: 1, need: 2 },
};

/** A 会签 node: per-group behavior, one group signed, one outstanding. */
const PER_GROUP_DETAIL = {
...LIST_ROW,
id: 'req_signoff_1',
process_name: 'flow:showcase_expense_signoff',
pending_approvers: ['u_devadmin', 'u_devadmin'],
decision_progress: {
behavior: 'per_group',
got: 1,
need: 2,
groups: [
{ group: 'finance', got: 1, need: 1, satisfied: true },
{ group: 'manager', got: 0, need: 1, satisfied: false },
],
},
pending_approver_groups: { u_devadmin: ['manager'] },
};

/** Every GET the hook issued, in order. */
let gets: string[];

function stubApi(detail: unknown, opts: { detailStatus?: number } = {}) {
gets = [];
vi.stubGlobal(
'fetch',
vi.fn(async (url: string) => {
const u = String(url);
gets.push(u);
if (/\/approvals\/requests\/[^?]+$/.test(u)) {
if (opts.detailStatus) {
return { ok: false, status: opts.detailStatus, json: async () => ({ error: 'nope' }) } as any;
}
return { ok: true, json: async () => detail } as any;
}
const list = (detail as any)?.id === PER_GROUP_DETAIL.id
? { ...LIST_ROW, id: PER_GROUP_DETAIL.id }
: LIST_ROW;
return { ok: true, json: async () => ({ data: [list] }) } as any;
}),
);
}

const mount = () =>
renderHook(() => useRecordApprovals('showcase_expense_report', 'AyG40_bAHSP_gi8T', 'u_manager'));

describe('useRecordApprovals — quorum progress (objectstack#4478)', () => {
beforeEach(() => stubApi(DETAIL_ROW));

it('exposes the quorum tally the list read does not carry', async () => {
const { result } = mount();
await waitFor(() =>
expect(result.current.pendingRequest?.decision_progress).toEqual({
behavior: 'quorum',
got: 1,
need: 2,
}),
);
});

it('reads the enrichment from the single-request endpoint', async () => {
const { result } = mount();
await waitFor(() => expect(result.current.pendingRequest?.decision_progress).toBeTruthy());
expect(gets.some((u) => u.includes('/approvals/requests?object='))).toBe(true);
expect(gets.some((u) => u.endsWith('/approvals/requests/req_committee_1'))).toBe(true);
});

it('keeps the rest of the list row — the follow-up read only adds', async () => {
const { result } = mount();
await waitFor(() => expect(result.current.pendingRequest?.decision_progress).toBeTruthy());
expect(result.current.pendingRequest?.lock_record).toBe(true);
expect(result.current.pendingRequest?.pending_approvers).toHaveLength(3);
});

it('surfaces the per-group tally and each pending approver\'s group', async () => {
stubApi(PER_GROUP_DETAIL);
const { result } = mount();
await waitFor(() => expect(result.current.pendingRequest?.decision_progress).toBeTruthy());
expect(result.current.pendingRequest?.decision_progress?.groups).toEqual([
{ group: 'finance', got: 1, need: 1, satisfied: true },
{ group: 'manager', got: 0, need: 1, satisfied: false },
]);
expect(result.current.pendingRequest?.pending_approver_groups).toEqual({
u_devadmin: ['manager'],
});
});

it('leaves the row untouched when the follow-up read fails', async () => {
// Display-only enrichment: a 500 must not take the approval panel down,
// and must not invent a tally either.
stubApi(DETAIL_ROW, { detailStatus: 500 });
const { result } = mount();
await waitFor(() => expect(result.current.pendingRequest).toBeTruthy());
expect(result.current.pendingRequest?.decision_progress).toBeUndefined();
// …and the decision surface the list read does support is still live.
expect(result.current.canDecide).toBe(true);
expect(result.current.pendingRequest?.lock_record).toBe(true);
});

it('makes no follow-up read when nothing is pending', async () => {
gets = [];
vi.stubGlobal(
'fetch',
vi.fn(async (url: string) => {
gets.push(String(url));
return { ok: true, json: async () => ({ data: [{ ...LIST_ROW, status: 'approved' }] }) } as any;
}),
);
const { result } = mount();
await waitFor(() => expect(result.current.latestRequest).toBeTruthy());
expect(gets.filter((u) => /\/approvals\/requests\/[^?]+$/.test(u))).toHaveLength(0);
});
});
75 changes: 74 additions & 1 deletion packages/app-shell/src/hooks/useRecordApprovals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,42 @@ export interface ApprovalRequestLite {
*/
decision_outputs?: string[] | null;
decision_output_defs?: DecisionOutputDef[] | null;
/**
* Server-computed decision tally of THIS pending node (framework#3266),
* present only for behaviors that aggregate more than one decision:
* `unanimous` / `quorum` (`got` / `need` approvals) and `per_group` (satisfied
* groups, plus `groups[]` detail). `first_response` nodes carry none — one
* decision finalizes them, so there is nothing to count.
*
* The server derives it from the node's own `node_config_json` snapshot
* (`behavior`, `minApprovals`, the `__approverGroups` slate), so the count
* the record header shows is the one the engine will enforce. See
* {@link fetchProgressEnrichment} for why it takes a second request.
*/
decision_progress?: ApprovalDecisionProgress;
/**
* Group membership of each still-pending approver on a `per_group` (会签)
* node — approver id → the named group(s) the slot fills. Lets a surface that
* lists pending approvers label each one; absent for other behaviors.
*/
pending_approver_groups?: Record<string, string[]> | null;
/** Display names for the ids in `pending_approvers` (id → name). */
pending_approver_names?: Record<string, string> | null;
}

/**
* Decision aggregation progress of a pending approval node — the `2 of 3` the
* approver needs in order to know whether their own decision closes the step.
* Mirrors the framework's `decision_progress` enrichment verbatim.
*/
export interface ApprovalDecisionProgress {
behavior: 'unanimous' | 'quorum' | 'per_group';
/** Approvals recorded — satisfied GROUPS when `behavior` is `per_group`. */
got: number;
/** Approvals required — total GROUPS when `behavior` is `per_group`. */
need: number;
/** Per-group tally, `per_group` only. */
groups?: Array<{ group: string; got: number; need: number; satisfied: boolean }>;
}

/**
Expand Down Expand Up @@ -127,6 +163,38 @@ async function fetchJson<T>(path: string, init?: RequestInit): Promise<T> {
return payload as T;
}

/**
* Pull the pending request's single-read enrichment and fold it onto the row.
*
* `decision_progress` (and the `pending_approver_groups` that ride with it) are
* attached by the framework's `getRequest` ONLY — `listRequests` deliberately
* skips them, because each one costs a `sys_approval_action` tally per row and
* a list read may return hundreds. So `GET /approvals/requests?object=…` — the
* call this hook makes — never carries the quorum data, and the record header
* had nothing to render even though the node's config had it all along
* (objectstack#4478: `minApprovals: 2` over a 3-approver slate showed no
* progress at all). One extra request for the ONE pending row is the shape the
* server contract asks for.
*
* Best-effort and non-fatal: a failure, or a payload that isn't the row we
* asked for, leaves the list row exactly as it came. Never invent a tally —
* a wrong "1 of 2" is worse than none.
*/
async function fetchProgressEnrichment(
request: ApprovalRequestLite,
): Promise<ApprovalRequestLite> {
try {
// `getRequest` answers with the row itself, not `{ data: row }`.
const row = await fetchJson<ApprovalRequestLite>(
`/approvals/requests/${encodeURIComponent(request.id)}`,
);
if (!row || row.id !== request.id) return request;
return { ...request, ...row };
} catch {
return request;
}
}

export function useRecordApprovals(
objectName: string | undefined,
recordId: string | undefined,
Expand All @@ -145,7 +213,12 @@ export function useRecordApprovals(
const reqResp = await fetchJson<{ data: ApprovalRequestLite[] }>(
`/approvals/requests?object=${encodeURIComponent(objectName)}&recordId=${encodeURIComponent(recordId)}`,
);
setRequests(reqResp?.data ?? []);
const rows = reqResp?.data ?? [];
// Only the pending row can have a live tally, and only it drives the
// header — so exactly one follow-up read, never one per row.
const pending = rows.find((r) => r.status === 'pending');
const full = pending ? await fetchProgressEnrichment(pending) : null;
setRequests(full ? rows.map((r) => (r === pending ? full : r)) : rows);
setAvailable(true);
} catch (err: any) {
if (err?.status === 404 || err?.status === 501) {
Expand Down
7 changes: 7 additions & 0 deletions packages/app-shell/src/views/RecordDetailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -996,6 +996,12 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
const approvalLocked = approvals.pendingRequest
? recordLockedByApproval(approvals.pendingRequest)
: approvalStatusPending;
// How far the pending node's tally has got (objectstack#4478). Multi-approver
// nodes — `quorum`, `unanimous`, `per_group` — do not finalize on one
// decision, so an approver standing on the record needs the count to know
// whether their click closes the step. Server-computed; `first_response`
// nodes carry none and the band then shows nothing extra.
const approvalProgress = approvals.pendingRequest?.decision_progress;

const approvalHandler = useCallback(async (action: ActionDef) => {
const target = action.target || action.name;
Expand Down Expand Up @@ -2060,6 +2066,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && recordWriteAllowed && !approvalLocked}
locked={approvalLocked}
approvalPending={approvalPending}
approvalProgress={approvalProgress}
lockedReason={t('detail.lockedTooltip', {
defaultValue: 'This record has a pending approval request; editing is locked',
})}
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,9 @@ const ar = {
writeStrippedByState: "غير قابلة للتعديل في الحالة الحالية لهذا السجل، لذلك لم يتم حفظها: {{fields}}",
approvalPendingEditable: "قيد الموافقة · قابل للتعديل",
approvalPendingTooltip: "يحتوي هذا السجل على طلب موافقة معلق، لكن هذه الخطوة لا تزال تسمح بالتعديل",
approvalProgress: "الموافقات — {{got}} من {{need}}",
approvalProgressGroups: "التوقيعات — {{got}} من {{need}} مجموعات",
approvalProgressLabel: "تقدّم الموافقة",
cancelApproval: "إلغاء الموافقة",
cancelApprovalInFlight: "جارٍ الإلغاء…",
cancelApprovalTooltip: "إلغاء طلب الموافقة المعلق لفتح قفل السجل",
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,9 @@ const de = {
writeStrippedByState: "Im aktuellen Status dieses Datensatzes nicht bearbeitbar und daher nicht gespeichert: {{fields}}",
approvalPendingEditable: "In Genehmigung · bearbeitbar",
approvalPendingTooltip: "Dieser Datensatz hat eine ausstehende Genehmigungsanfrage; dieser Schritt erlaubt weiterhin die Bearbeitung",
approvalProgress: "Genehmigungen — {{got}} von {{need}}",
approvalProgressGroups: "Freigabe — {{got}} von {{need}} Gruppen",
approvalProgressLabel: "Genehmigungsfortschritt",
cancelApproval: "Genehmigung zurückziehen",
cancelApprovalInFlight: "Zurückziehen…",
cancelApprovalTooltip: "Ausstehende Genehmigungsanfrage zurückziehen, um den Datensatz zu entsperren",
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,9 @@ const en = {
writeStrippedByState: "Not editable in this record's current state, so it was not saved: {{fields}}",
approvalPendingEditable: 'In approval · editable',
approvalPendingTooltip: 'This record has a pending approval request; this step still allows editing',
approvalProgress: 'Approvals — {{got}} of {{need}}',
approvalProgressGroups: 'Sign-off — {{got}} of {{need}} groups',
approvalProgressLabel: 'Approval progress',
cancelApproval: 'Recall approval',
cancelApprovalInFlight: 'Recalling…',
cancelApprovalTooltip: 'Recall the pending approval request to unlock this record',
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,9 @@ const es = {
writeStrippedByState: "No editable en el estado actual de este registro, por lo que no se guardó: {{fields}}",
approvalPendingEditable: "En aprobación · editable",
approvalPendingTooltip: "Este registro tiene una solicitud de aprobación pendiente; este paso todavía permite la edición",
approvalProgress: "Aprobaciones — {{got}} de {{need}}",
approvalProgressGroups: "Firmas — {{got}} de {{need}} grupos",
approvalProgressLabel: "Progreso de la aprobación",
cancelApproval: "Cancelar aprobación",
cancelApprovalInFlight: "Cancelando…",
cancelApprovalTooltip: "Cancelar la solicitud de aprobación pendiente para desbloquear el registro",
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,9 @@ const fr = {
writeStrippedByState: "Non modifiable dans l'état actuel de cet enregistrement, donc non enregistré : {{fields}}",
approvalPendingEditable: "En approbation · modifiable",
approvalPendingTooltip: "Cet enregistrement a une demande d'approbation en attente ; cette étape autorise encore la modification",
approvalProgress: "Approbations — {{got}} sur {{need}}",
approvalProgressGroups: "Validation — {{got}} sur {{need}} groupes",
approvalProgressLabel: "Progression de l'approbation",
cancelApproval: "Annuler l'approbation",
cancelApprovalInFlight: "Annulation…",
cancelApprovalTooltip: "Annuler la demande d'approbation en attente pour déverrouiller l'enregistrement",
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,9 @@ const ja = {
writeStrippedByState: "次の項目はこのレコードの現在の状態では編集できないため保存されませんでした: {{fields}}",
approvalPendingEditable: "承認中 · 編集可能",
approvalPendingTooltip: "このレコードには承認待ちのリクエストがありますが、このステップでは編集できます",
approvalProgress: "承認 — {{need}} 件中 {{got}} 件",
approvalProgressGroups: "合議 — {{need}} グループ中 {{got}} グループ",
approvalProgressLabel: "承認の進捗",
cancelApproval: "承認を取り消す",
cancelApprovalInFlight: "取り消し中…",
cancelApprovalTooltip: "承認待ちリクエストを取り消してレコードのロックを解除する",
Expand Down
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/ko.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,9 @@ const ko = {
writeStrippedByState: "다음 필드는 이 레코드의 현재 상태에서 편집할 수 없으므로 저장되지 않았습니다: {{fields}}",
approvalPendingEditable: "승인 진행 중 · 편집 가능",
approvalPendingTooltip: "이 레코드에 대기 중인 승인 요청이 있지만 이 단계에서는 편집할 수 있습니다",
approvalProgress: "승인 — {{need}}건 중 {{got}}건",
approvalProgressGroups: "합의 — {{need}}개 그룹 중 {{got}}개",
approvalProgressLabel: "승인 진행 상황",
cancelApproval: "승인 취소",
cancelApprovalInFlight: "취소 중…",
cancelApprovalTooltip: "대기 중인 승인 요청을 취소하여 레코드 잠금 해제",
Expand Down
Loading
Loading