Repository navigation
fix(app-shell): eligible approvers count distinct people, not pending_approvers slots (objectui#12781) - #12831
Conversation
…_approvers slots The record approvals panel and the approval decision panel handed DecisionProgressIndicator the length of the request's pending_approvers list. The approvals service keeps one slot per resolved approver entry, so a person holding two of a node's positions was counted twice: the stock showcase committee quorum read "3 eligible approvers" beside one "Dev Admin x3" chip. Both callers now use one module-local helper beside the indicator, eligibleApproverCount, which counts the distinct addresses of the slate as served. Nothing is resolved in the console and the server tally (got / need) is untouched. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
main moved under apps/console/src/pages/system/ApprovalsInboxPage.tsx, which round 2 of objectui#12781 adds to this branch's surface. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
… distinct people The request drawer's decision-progress block computed its own eligible count from the length of pending_approvers, so one person holding several slots was counted once per slot: the stock showcase committee quorum read "3 eligible approvers" for one person. It now counts the distinct served addresses with the page's own new Set idiom, the rule app-shell's eligibleApproverCount applies (not exported for this). The presence check and the slot list stay as they were. Claude-Session: https://claude.ai/code/session_01Vk15bz8xHtKGUfXfCm16h4 Co-authored-by: Claude <noreply@anthropic.com>
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
席位核对 — ACCEPT ·
|
| surface | main 33ee9de |
this PR |
|---|---|---|
| record Approvals tab | "0 of 1 · 3 eligible approvers" | "0 of 1 · 1 eligible approver" |
record:approval_decision page |
"0 of 1 · 3 eligible approvers" | "0 of 1 · 1 eligible approver" |
| Approval Center drawer | "0 of 1 · 3 eligible approvers" (also on round 1's a42606c) |
"0 of 1 · 1 eligible approver" (f132e95) |
- The Invoice Dual Sign-off request read the same way, with its id twice. The tally ("0 of 1") is unchanged everywhere.
3. Gates (check-runs pinned to this head).
- 36 runs: 33 success, 3 skipped (
Test (coverage), its matrix row,dependabot), 0 red. - Among the successes:
Lint,Type Check,Repo Gates,Test,Test (shard 1/16)…(16/16),Test (dist pins),Build & E2E,Build Docs,Bundle Analysis(PASS,6227188527: eager closure 3131.6 KB of 3744.1 KB),Live E2E (informational), the changeset checks,Line Citation GateandGoverned Surface Queue Guard. - The dev's local subset at
f132e95:- 36 files / 232 tests over every suite that names the two panels, the indicator, the helper or the inbox;
- the console closure build, and the console and app-shell type-checks;
- eslint, with 0 errors on the changed lines;
- changeset presence and new-line citations.
- Ablations, each restored and proven:
- round 1: the callers back to the slot length (4 red, the duplicate-slot rows only), the helper capped at 1 (7 red), the tally fed the count (2 red, the tally pins only);
- round 2 on the drawer: B1 2 red, B2 2 red, B3 1 red.
4. Accepted deviations.
- The claim was re-stated in place (
6227052880) for the drawer: the same count from the same slot list. The four conditions were read in the re-statement. - Round 1 created a private ref
refs/os-dev/12781-mainin the shared.git, which the dispatch forbids. It was deleted at once, and round 2 readmainwithfetchandls-remoteonly (0 such refs at the end). This is the second dev this shift to do it; the seat's later dispatches name the forbidden commands. - The PR body is stale, as noted above. It is not patched, under the dev's write budget; this record is the review's text.
5. Out of scope (where each went).
- The card's title says "5 eligible shown for 3 people", while the QA evidence it cites lists 5 slots for 4 people. The defect is the same. → Acceptance notes.
6. Landing.
- Not governed:
check-governed-merges.mjs --rootobjectui--testover the 9 paths reads NOT governed (0 of 9 paths hit the register). - 513 lines changed, under the 3000-line threshold.
Clause-②: no. Every check is green or an expected skip.- ⇒ The seat lands it: relay
pr_ready+automerge_enable, through the merge queue. - On MERGED,
Fixescloses objectui#12781.
Generated by Claude Code
Fixes #12781
Clause-②: no (no export, prop or type changes;
DecisionProgressIndicator'seligibleApproversprop keeps its type and meaning)What changed
RecordApprovalsPanelandApprovalDecisionPanelhandedDecisionProgressIndicatorthe length of the request'spending_approverslist aseligibleApprovers. The prop's own doc says "how many approvers may still decide", which counts people. The approvals service keeps one slot per resolved approver entry, so a person holding two of a node's positions fills two slots under the same id and was counted twice.Both callers now use one module-local helper beside the indicator,
eligibleApproverCountinpackages/app-shell/src/views/approval-progress/eligibleApprovers.ts. It counts the distinct addresses in the slate as the server serves it. It resolves nothing and makes no second read: no position, team or group lookup. It is not on the package entry. Unchanged:decision_progress.got/need);pending_approversreads: the record panel's presence check that gates "Waiting on", and the chip list.What a slot carries (measured)
Measured live on objectstack
66d456d01,examples/app-showcase,objectstack dev --seed-admin --fresh, signed in as the seeded admin, readingGET /api/v1/approvals/requests/ID:pending_approverspending_approver_namesdecision_progresssys_userid, 3 timessys_userid, 2 timesEach slot is a bare address string, here a
sys_userid. A repeated person is the same string served again. The showcase seed puts the admin onmanager,financeandlegal, and these nodes route to those positions. So distinct strings are the distinct principals, as served.The spec contract (
ApprovalRequestRowin@objectstack/spec/contracts) also names an email and aposition:NAMEreference as slot addresses. Those are counted as the server left them. An email and a user id for the same person would count as two, because merging them would mean re-deriving the slate in the console.Before / after (live, rendered)
The console ran from this worktree (Vite dev,
srcaliases), proxied to that backend, in Chromium at/opt/pw-browsers/chromium. Each cell is the indicator's header row, read from the DOM:main33ee9dea42606cRecordApprovalsPanel)record:approval_decision)The tally reads
0 of 1on both sides.Pins
The pins sit beside each caller and render under a real i18next instance. Each label is asserted as the pack row (
approvalsInbox.progressEligible,approvalsInbox.progressApprovals) rendered with its count:RecordApprovalsPanel.eligibleApprovers-12781.test.tsxapproval-decision/ApprovalDecisionPanel.eligibleApprovers-12781.test.tsxEach file pins four cases:
aria-valuemax,aria-valuenowand tick count.approval-progress/eligibleApprovers.test.tspins the helper itself.Ablations
Each ablation ran once, through objectstack's
scripts/ablation-replace.mjs. The anchor hit went 1 to 0 and the blob changed; the restore was proven (blob equals HEAD,git diff HEADempty):main's callersMath.min(1, distinct)need: eligibleThe tests import the subject by relative path from
src, so nodistsits between a mutation and its run.Local gates (at
a42606c)pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build(29 packages): exit 0.pnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json): exit 0.--listFilesOnlyshows the test program includes the helper and all three new test files.pnpm exec vitest runover 20 files: 20 files, 148 tests passed. The set is the 12 tracked tests namingRecordApprovalsPanel,ApprovalDecisionPanel,DecisionProgressIndicatoror the helper, plus the 8 consoleapprovalRequestsDataSourcesuites that draw the decision panel on the served request page.pnpm exec eslint --no-inline-configon the six changed source files: 0 errors and 6 warnings, all on lines this diff does not touch.node scripts/check-changeset-presence.mjs: exit 0.pnpm check:new-line-citations: exit 0, 0 new citations.Acceptance notes
apps/console/src/pages/system/ApprovalsInboxPage.tsx, the decision-progress block) computes its ownconst eligible = (selected.pending_approvers || []).lengthand draws the sameprogressEligiblerow. On the same backend it still reads "Approvals — 0 of 1 · 3 eligible approvers" for the committee quorum on this branch. It is outside this card's claimed surface, and the dispatch said to report it rather than widen into it. It is reported to the seat.Generated by Claude Code