Repository navigation
feat(plugin-approvals): serve the approval request page through the manifest pages, and declare the thread reply as approval_comment (#22473) - #22567
Conversation
…age and declare approval_comment (WIP) Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…inst the comment route (WIP) Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…proval-request-page
📓 Docs Drift CheckThis PR changes 1 package(s): 31 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 45b80a920fad598bcd22d49bfde072b24e93dae6 && git checkout 45b80a920fad598bcd22d49bfde072b24e93dae6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fbb065fd4b52dce103776e49fefd90f0138059a7 69fef50a225effda259adbc7db0bd247c551d259 && git checkout -B drift-repro fbb065fd4b52dce103776e49fefd90f0138059a7 && git merge --no-ff 69fef50a225effda259adbc7db0bd247c551d259
node scripts/docs-audit/affected-docs.mjs --json fbb065fd4b52dce103776e49fefd90f0138059a7
|
…dden and refused for an override-only admin Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…eclared actions Claude-Session: https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ Co-authored-by: Claude <noreply@anthropic.com>
…proval-request-page
Fixes #22473
Clause-②: no
What this does
@objectstack/plugin-approvalsnow servessys_approval_request's record page as metadata, and it declares the request thread's reply as an action. This is the backend half that the maintainer's ruling 乙 on objectstack-ai/objectui#12045 (6079807016) assigns here:The consumer of both halves is the console's approval request page, built on objectui#12045. Its page-mount step waits on this PR. This PR does not close that card.
packages/plugins/plugin-approvals/)src/sys-approval-request.page.ts(new)SysApprovalRequestDetailPage: a slotted page, default for every requestsrc/approvals-plugin.tspages: [SysApprovalRequestDetailPage]src/sys-approval-request.object.tsapproval_commentsrc/translations/pages.ts(new),src/translations/index.tspages.sys_approval_request_detail.labelin en / zh-CN / ja-JP / es-ES, with recorded source digestsscripts/i18n-extract.config.tspagesbaseline, so the coverage ratchet counts the labelsrc/translations/*.objects.generated.tsapproval_comment(node scripts/check-i18n-bundles.mjs --write); the zh-CN / ja-JP / es-ES leaves are translated by hand.changeset/22473-approval-request-page.md(minor)The page is module-internal:
index.tsexports nothing new, andexportshas only.. SoClause-②isno. Nothing changes inpackages/specorplatform-objects. Outside the plugin, this PR touches one test (packages/qa/dogfood/test/approval-override-composite-pin.dogfood.test.ts, which pins this object's served actions; patch round 1 at69fef50a2) and one docs paragraph (content/docs/automation/approvals.mdx, the list of declared actions; cross-lane declaration on #6023).The page's slot map, as served
This was read back from
GET /api/v1/meta/page/sys_approval_request_detailon the showcase dev server (pnpm dev -- --fresh, HEAD8f30e97c3), signed in as the seeded admin:{ "name": "sys_approval_request_detail", "label": "Approval Request", "type": "record", "object": "sys_approval_request", "kind": "slotted", "isDefault": true, "_packageId": "com.objectstack.service.approvals", "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": [] } }actionsholds onerecord:approval_decisionnode with no properties. The type was declared onmainat7731d7018fwith an empty, strict props row.highlightsis the object'shighlightFieldswithoutrecord_id. This is the seat's choice on the ui seat's design input6081889955. The object's ownhighlightFields(sys-approval-request.object.ts:57) is unchanged. Sorecord_idstays in the details grid, where the console draws the target-record card for therecord_id/object_namepointer pair.tabshas two tabs: Details (record:details) and Timeline (sys_approval_actionas a related list onrequest_id, newest first). The page authorstabs, notdetails, because the console's builder (objectuibuildDefaultPageSchema) takestabsoverdetailswhen both are present. The details body therefore lives in the first tab.discussion: []: there is norecord:discussionnode. The thread's reply isapproval_comment.approval_commentbeside the route's own checkThe declaration (
sys-approval-request.object.ts:676):The route's own check:
packages/rest/src/rest-server.ts:12751:threadRoute('comment', ...)callsApprovalService.commentwithactorId: body.actorId ?? body.actor_id ?? context?.userId. The declared action sends no actor, so the service acts as the caller.approval-service.ts:4828:loadPendingRowrefuses withINVALID_STATEunlessstatus === 'pending'(:1440).:4830and:4833: the caller is admitted as the submitter, or as the holder of a pending slot (takenSlot, which callsheldSlot).:4835: any other caller is refused withFORBIDDEN, unless the context is a system one.The viewer block (
attachViewers,approval-service.ts:7027–:7028) carries two flags:can_actisstatus === 'pending' && heldSlot(pending, caller.userId, caller). That is the same slot test, already scoped topending.is_submitterhas no status test, so the predicate adds thependingtest thatloadPendingRowapplies.So
visibleiscan_actOR (pendingANDis_submitter). That is the route's admission for every caller the declared action can produce. The route admits two more callers, and the predicate leaves both out: a system context (its viewer block is all false), and a caller who names another identity inactorId(the action sends none). So the predicate is narrower than the route there, never wider. It has nocan_overridearm, because the route has none. An override admin who holds no slot getsFORBIDDENfrom the route. The parity is pinned against the real service for eight caller shapes (see Tests).Done when: evidence
1.
os validatepasses on the plugin with the page.node packages/cli/bin/run.js validate packages/plugins/plugin-approvals/scripts/i18n-extract.config.tsran at HEAD8f30e97c3and exited 0:None of the findings is located on the page:
grep -c 'at pages\['counts 0. The 57 warnings are all on the three objects (field-group-undeclared45,field-no-consumers9,title-format-retired3), and none of them is new in this diff. A control from a temporarydefineStackconfig in the package (deleted afterwards;git statusempty) ran the same page with the panel type misspelledrecord:approval_decison. It exited 1 withrule: component-type-unknown at pages[0].slots.actions[0].type, and its fix line was "Renamerecord:approval_decison→record:approval_decision". The correct spelling of that config exited 0 (UI: 1 Pages).2. The metadata API serves the page for
sys_approval_request. 3.approval_commentposts a reply that the timeline lists. Both were measured on the showcase dev server (fresh database, seeded admin). A retitled announcement opens ashowcase_dynamic_approvalrequest, on which the admin is both the submitter and the pending approver:zh-CN is the one translated locale this server serves (
supportedLocales: ['en', 'zh-CN']). For ja-JP and es-ES it serves the English label. The control is the platform's ownsys_user_detailpage and thesys_approval_requestobject label, which fall back to English the same way there.Tests
The same runs at HEAD
8f30e97c3(aftergit merge origin/mainand a rebuild of the closure):pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: Test Files 65 passed (65) · Tests 939 passed (939),VERDICT command-exit 0.pnpm --filter @objectstack/plugin-approvals typecheck: exit 0,check:test-typecheck: OK — 8 file(s) / 324 error(s). The ledger is unchanged, and no new file carries an error.src/sys-approval-request-page.test.ts(16 cases):highlightFieldsminusrecord_id, the two tabs, no discussion, and every named field exists;validateComponentTypes,validateComponentPropsandrunAuthoringRules('build')are clean on the page. Each has a control that goes red: a misspelled type, a prop on the decision node, and the build rules on the misspelled type;enlabel equals the page literal, and every locale serves its translation.approval-service.test.ts→ "approval_comment — its visible gate is the comment route's admission". It runs eight caller shapes against the real service: shown equals admitted for each, and the admin row is a real non-arm. A reply lands on the timeline with its attachments.sys-approval-request.object.test.tsroster, pluscommentin its route verbs and the declaration pins; theaction-predicate-sparse-face.test.tscount (8 → 9), plus the reply's pending-only submitter arm.Ablations each ran through
node scripts/ablation-replace.mjs. Each landed (anchor 1 → 0, blob moved), and each was restored with blob == HEAD and an emptygit diff HEAD:visiblegains acan_overridearmpending · an override admin on no slot(showntrue, routeFORBIDDEN), and so does the declaration pinpendingtestapproved · the submitterandrecalled · the submitter(showntrue, routeINVALID_STATE). The first attempt's anchor missed, so the tool refused it and wrote nothing; it was re-run with the corrected anchorenpage label is editedes-ESlabel becomes the edited English source. The recorded digests are consulted, and both translation pins go redpagesentry is dropped from the extract configcheck:i18n-coveragereports "untranslated declared strings grew 0 → 1" for this config, so the ratchet counts the labelGates
The gates were derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackon8f30e97c3. That is 65 commands, pluspnpm check:i18n-coveragebecause the extract config moved. All 66 exited 0 at8f30e97c3. The reconciliation--ranread: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN … a DERIVED zero — all 65 recorded an exit code".On the pre-merge head
de7b645ee,check:dual-build-cjs-loadsexited 3 with PREREQUISITE NOT MET, because eight packages outside the closure had nodist/. It passed once those were built. Among the gates:check:i18n(all nine bundle sets in sync),check:i18n-stale-fill,check:engine-double-contract,check:test-source-alias,check:page-declaration-shape,check:published-filesandcheck:nul-bytes.Lint was narrowed to the 14 changed
.tsfiles:eslint --no-inline-config --format jsongave 14 files, 0 errors, 0 warnings. The population iseslint.config.mjs's**/*.{ts,…}set. The config never enables type-aware linting (its own note: "noparserOptions.project, no typed@typescript-eslintrules"), so this diff cannot move the verdict on any untouched file. The repo-widepnpm lintis left to CI.Acceptance notes
src/translations/pages.ts), the wayplatform-objectscarriespages.*forsys_user_detail/sys_organization_detail/sys_position_detailin itsLOCALE.tsfiles. The action's leaves did go through the tooling. The reasons, as measured:objectssub-tree alone.pages.--no-objects-only) is not carried by the gate:flagsFromDocstring(scripts/i18n-bundle-surface.mjs) returns["--locales=zh-CN","--fill=default","--no-metadata-forms","--source-hashes","--out=o"]for it. The flag is dropped, socheck:i18nwould check an objects-only extract against a stack-shaped bundle. This is dormant: no config documents that spelling, and no owner is named.record:detailsrenders the object's field groups (Target/State/System), andsys_approval_requestdeclares nofieldGroups.os validatealready reports this as 20field-group-undeclaredwarnings, so those fields render in the ungrouped bucket. This is pre-existing, unchanged here, and has no owner named.POST /api/v1/auth/sign-up/email→ 403), so there was no second account to try. The refusal is pinned against the real service instead (pending · a stranger→FORBIDDEN, alongside the existing outsider test).platform-objects'SysUserDetailPage. That page authors bothslots.detailsandslots.tabs, so itsdetailsoverride is not drawn. This is reported to the seat, not changed here.Generated by Claude Code