fix(app-shell): run sys_approval_request's declared decision actions on the record page (#3055) - #4077
Merged
Merged
Conversation
…on the record page (#3055) A business record with an approval pending on it offered exactly two buttons — Approve and Reject — hand-written into the record header behind a bespoke `type:'approval'` handler and a client-side approver test. The approvals list, over the same request and the same nine REST routes, offered five decisions plus the submitter's levers and took decision attachments. Reassign / send back / request info had NO entry point on a record, a decision could not carry a file, and the copy was forked. The record page now renders `sys_approval_request`'s own declared actions through DeclaredActionsBar: same metadata, same action runtime, same param dialogs as the approvals list. The dispatch record is the pending REQUEST row, which is what resolves `{id}` and what gives each action's `visible` the server-computed `viewer` block to read. - `useRecordApprovals` keeps only its read half. `canDecide` / `approve` / `reject` and the `currentUserId` parameter are retired: who may act is the server's answer on the row, and deciding is the declared action's POST. - The bar's predicate scope now binds the row the three ways the record header and list rows bind it (`record.status`, bare `status`, `data.status`). It bound only the bare form, so `record.viewer.can_act` threw `record is not defined` and the fail-closed gate hid it — which made EVERY declared action on `sys_approval_request` invisible on every surface the bar renders, the approvals inbox included. - `useMetadataItem` is imported from `@object-ui/react` rather than through `../providers/MetadataProvider`, whose module graph builds an authenticated fetch at import time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
…ord-approval-actions
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 10, 2026 04:44
This was referenced Aug 10, 2026
akarma-synetal
pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Aug 10, 2026
…objectstack-ai#4075) (objectstack-ai#4079) `action:button` / `action:icon` / `action:menu` / `action:group` gated their actions on `useCondition(pred, ctx)` with the row spread flat — or, on three of the leaves, with no row at all. Only the bare-field shorthand resolved, so the CANONICAL `record.` root threw `record is not defined`; a fail-closed `visible` turned that into "hidden" and a correctly-authored predicate deleted its own button. Every declared action on `sys_approval_request` gates on `record.viewer.*`, which is how the same binding suppressed the whole server-declared approval decision set until objectstack-ai#4077 fixed the declared-action bar. All four now bind `record.status` / bare `status` / `data.status` through one named helper (`usePredicateRecordContext`, beside `useCondition` in `@object-ui/react`) — `evalRowPredicate`'s rule, restated for this tier. `action:icon` reads its `data` prop at all; the menu/group leaves receive the row from their host; `action:bar` forwards the row into the overflow menu it builds, so a predicate no longer answers a different question because its action spilled past `maxVisible`. The evaluation entry and every site's error policy are untouched: a genuinely faulting predicate still fails closed where it did and soft where it did. A surface with no row binds nothing rather than an empty record, so a host that supplies the row through the ambient scope is not blanked out. Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3055
A business record with an approval pending on it offered exactly two buttons — Approve and Reject — hand-written into the record header behind a bespoke
type:'approval'handler and a client-side approver test. The approvals list, over the same request and the same nine REST routes, offered five decisions plus the submitter's levers and took decision attachments. On a business record, reassign / send back / request info had no entry point at all, a decision could not carry a file, and the copy on the two surfaces was maintained separately.The record page now renders
sys_approval_request's own declared actions throughDeclaredActionsBar— the same metadata, the same action runtime, the same param dialogs the approvals list uses. Adding a tenth decision action is a metadata change with no console work.The card's three steps
sys_approval_requestwith the serverviewerblockuseRecordApprovalsenriches the pending row viagetRequest, which is what attachesviewer. The block was fetched and then ignored.useRecordApprovals' decide /canDecide, keep status badge +lock_recordWhat changed
RecordDetailView— the hard-coded injection andbuildApprovalDecisionActionsare gone, and with them thetype:'approval'handler. ADeclaredActionsBarrenders above the page body when a request is pending, with the request row as the dispatch record.useRecordApprovals— read half only.canDecide/approve/rejectand thecurrentUserIdparameter are retired.DeclaredActionsBar— the predicate scope now binds the row the three ways the record header and list rows bind it. See below; this is the load-bearing part.recordApprovalActions.ts— the three coordinates of the wiring (object, location, the one excluded action), stated once.Why the header could not carry these actions
action:barevaluates avisiblepredicate against this page's record and stamps that same record intoparams._rowRecord(containers.tsx:1278-1300). A business record has noviewerblock, and a declared action targeting/api/v1/approvals/requests/{id}/…would resolve{id}to the business record's id. The request row has to be the dispatch record, which is what the bar does — so the card's suggestedrecord_section→ header/overflow mapping is not needed: the page consumes the declaredrecord_sectionlocation verbatim.The declared actions were not rendering anywhere
The bar passed the row in as the bare predicate scope, so only the shorthand spelling (
status == "pending") resolved. The canonical spelling is therecord.root — it is whatExpressionEvaluator's CEL path binds, whatevalRowPredicatebinds on the record header and list rows, and what the server enforces with. Under a root-only bagrecord.viewer.can_actdoes not read as false; it raisesrecord is not defined, and the fail-closed gate turns that into "hidden".Every declared action on
sys_approval_requestgates onrecord.viewer.*, so the whole server-declared decision set was invisible on every surface this bar renders — the approvals inbox included. Measured against the shipped evaluator:The row now binds three ways (
record.status, barestatus,data.status), so both spellings reach a verdict. The inbox page itself is untouched; it gains the decision levers it was already written to expect (its "why disabled" copy and its "no longer hand-wired here" comments both assume them).An acceptance-line correction, recorded rather than worked around
The triage acceptance line reads: a group approver (
position:xxx-style entry inpending_approvers) sees and can execute the same five decision actions. The first half is delivered; the parenthetical describes a state the platform treats as routed to nobody, and no consumer-side change reaches it:approval-service.ts:920-932, thepositionbranch at:928), soviewer.can_actis true and all five actions render — this is the acceptance case, and it is covered by tests;position:xxxliteral survives inpending_approversonly when the graph lookup produced nobody (:969). The service logs it as a routing bug ("the slot routes to no one and the request cannot advance", framework#3807) andcan_override(framework#3424) is the designed recovery — the admin rescue path, which the record page previously had zero entry to and now has.The card's stated root cause — that the record page's client-side
includesfails where serverviewer.can_actsucceeds — is therefore only half right:attachViewerscomputescan_actwith the same membership test on the same array (approval-service.ts:4017). What actually changes for gating is that override admins and submitters now reach the record page, and that the record page stops holding a second opinion at all. The five-action gap, the missing attachments and the forked copy — the bulk of the card — were real and are fixed.Tests
New
RecordDetailView.approvalDeclaredActions.test.tsxmounts the real record page against a pending request and asserts the surface an approver actually gets: all five decisions present forcan_act, the three that had no entry point specifically, submitter levers hidden, the POST landing on/approvals/requests/req_qif_1/revise(not on the business record id), the dottedoutputs.param folded into the nested body, override-admin and submitter matrices, and fail-closed on a backend with noviewerblock.Reverse verification: reverting only the predicate binding turns 9 of the 13 red — the four that survive are the two metadata-shape assertions and the two absence cases, which is the expected direction for a change whose whole effect is "the gate can now be evaluated".
Retired with their subject:
RecordDetailView.approvalDecisionActions.test.tsx(pinned the deleted builder) anduseRecordApprovals.decisionOutputs.test.tsx(pinned the deletedapprove()body — its concern, the nestedoutputs, is re-asserted end-to-end in the new file).Out-of-scope findings
visiblewritten asrecord.*faults and fails closed #4075 —action:button/action:icon/action:menu/action-groupstill bind the row only at the bare root, so the samerecord.*predicates fail closed there.sys_approval_request.approval_reject/approval_recalldeclare bothconfirmTextandparams, so one decision opens two sequential dialogs objectstack#7278 —approval_reject/approval_recalldeclare bothconfirmTextandparams, and the runner chains confirm then collect, so one decision opens two sequential dialogs (the shape objectui#3126 removed from the hand-written copy).origin/mainmerged before opening (carries #4068 / #4070 / #4071 / #4072); no file overlap with #4071's approvals-inbox component ref.Generated by Claude Code