Skip to content

Commit 5495331

Browse files
feat(plugin-approvals): serve the approval request page through the manifest pages, and declare the thread reply as approval_comment (#22473) (#22567)
Fixes #22473 Clause-②: no ## What this does `@objectstack/plugin-approvals` now serves `sys_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 request page is a slotted page in objectstack's plugin-approvals, registered through the manifest's `pages`; objectui registers the renderers and mounts `RecordDetailView` 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. | File (under `packages/plugins/plugin-approvals/`) | Change | | :--- | :--- | | `src/sys-approval-request.page.ts` (new) | `SysApprovalRequestDetailPage`: a slotted page, default for every request | | `src/approvals-plugin.ts` | the manifest registers `pages: [SysApprovalRequestDetailPage]` | | `src/sys-approval-request.object.ts` | adds a ninth declared action, `approval_comment` | | `src/translations/pages.ts` (new), `src/translations/index.ts` | `pages.sys_approval_request_detail.label` in en / zh-CN / ja-JP / es-ES, with recorded source digests | | `scripts/i18n-extract.config.ts` | declares the page and the `pages` baseline, so the coverage ratchet counts the label | | `src/translations/*.objects.generated.ts` | regenerated for `approval_comment` (`node scripts/check-i18n-bundles.mjs --write`); the zh-CN / ja-JP / es-ES leaves are translated by hand | | tests, `.changeset/22473-approval-request-page.md` (`minor`) | see Evidence below | The page is module-internal: `index.ts` exports nothing new, and `exports` has only `.`. So `Clause-②` is `no`. Nothing changes in `packages/spec` or `platform-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 at `69fef50a2`) 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_detail` on the showcase dev server (`pnpm dev -- --fresh`, HEAD `8f30e97c3`), signed in as the seeded admin: ```json { "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": [] } } ``` - **`actions`** holds one `record:approval_decision` node with no properties. The type was declared on `main` at `7731d7018f` with an empty, strict props row. - **`highlights`** is the object's `highlightFields` without `record_id`. This is the seat's choice on the ui seat's design input `6081889955`. The object's own `highlightFields` (`sys-approval-request.object.ts:57`) is unchanged. So `record_id` stays in the details grid, where the console draws the target-record card for the `record_id` / `object_name` pointer pair. - **`tabs`** has two tabs: Details (`record:details`) and Timeline (`sys_approval_action` as a related list on `request_id`, newest first). The page authors `tabs`, not `details`, because the console's builder (objectui `buildDefaultPageSchema`) takes `tabs` over `details` when both are present. The details body therefore lives in the first tab. - **`discussion: []`**: there is no `record:discussion` node. The thread's reply is `approval_comment`. - There is no aside, because the slot map has no right-rail slot. ## `approval_comment` beside the route's own check The declaration (`sys-approval-request.object.ts:676`): ```ts name: 'approval_comment', label: 'Reply', type: 'api', method: 'POST', target: '/api/v1/approvals/requests/{id}/comment', params: [comment (textarea, required), attachments (file, multiple, optional)], visible: 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true' + ' || has(record.status) && record.status == "pending"' + ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true', locations: ['record_section'], refreshAfter: true, ``` The route's own check: - **`packages/rest/src/rest-server.ts:12751`**: `threadRoute('comment', ...)` calls `ApprovalService.comment` with `actorId: body.actorId ?? body.actor_id ?? context?.userId`. The declared action sends no actor, so the service acts as the caller. - **`approval-service.ts:4828`**: `loadPendingRow` refuses with `INVALID_STATE` unless `status === 'pending'` (`:1440`). - **`:4830` and `:4833`**: the caller is admitted as the submitter, or as the holder of a pending slot (`takenSlot`, which calls `heldSlot`). - **`:4835`**: any other caller is refused with `FORBIDDEN`, unless the context is a system one. The viewer block (`attachViewers`, `approval-service.ts:7027`–`:7028`) carries two flags: - `can_act` is `status === 'pending' && heldSlot(pending, caller.userId, caller)`. That is the same slot test, already scoped to `pending`. - `is_submitter` has no status test, so the predicate adds the `pending` test that `loadPendingRow` applies. So `visible` is `can_act` OR (`pending` AND `is_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 in `actorId` (the action sends none). So the predicate is narrower than the route there, never wider. It has no `can_override` arm, because the route has none. An override admin who holds no slot gets `FORBIDDEN` from the route. The parity is pinned against the real service for eight caller shapes (see Tests). ## Done when: evidence **1. `os validate` passes on the plugin with the page.** `node packages/cli/bin/run.js validate packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts` ran at HEAD `8f30e97c3` and exited 0: ```text ✓ Validation passed (456ms) Data: 3 Objects 45 Fields UI: 1 Pages ``` 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-undeclared` 45, `field-no-consumers` 9, `title-format-retired` 3), and none of them is new in this diff. A control from a temporary `defineStack` config in the package (deleted afterwards; `git status` empty) ran the same page with the panel type misspelled `record:approval_decison`. It exited 1 with `rule: component-type-unknown at pages[0].slots.actions[0].type`, and its fix line was "Rename `record: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_comment` posts a reply that the timeline lists.** Both were measured on the showcase dev server (fresh database, seeded admin). A retitled announcement opens a `showcase_dynamic_approval` request, on which the admin is both the submitter and the pending approver: ```text HEAD 8f30e97 · showcase dev server --fresh on :42987 POST /api/v1/auth/sign-in/email -> 200 GET /api/v1/meta/page/sys_approval_request_detail -> 200 GET /api/v1/meta/pages?object=sys_approval_request -> 200 pages for sys_approval_request: ["sys_approval_request_detail"] GET page (Accept-Language: zh-CN) -> 200 label: 审批请求 POST /api/v1/data/showcase_announcement -> 201 PATCH /api/v1/data/showcase_announcement/ID (retitle: opens showcase_dynamic_approval) -> 200 GET /api/v1/approvals/requests?object=showcase_announcement&recordId=ID -> 200 GET /api/v1/approvals/requests/REQ -> 200 {"status":"pending","viewer":{"can_act":true,"is_submitter":true,"can_override":true}} GET /api/v1/meta/object/sys_approval_request -> 200 served approval_comment: target /api/v1/approvals/requests/{id}/comment · params [["comment","textarea",true,false],["attachments","file",false,true]] served approval_comment.visible on the served request row -> true POST /api/v1/approvals/requests/REQ/comment {comment} -> 200 {"status":"pending"} GET /api/v1/approvals/requests/REQ/actions -> 200 [{"action":"submit","actor_name":"Dev Admin","comment":null},{"action":"comment","actor_name":"Dev Admin","comment":"Reply posted through the declared approval_comment target."}] GET /api/v1/data/sys_approval_action?request_id=REQ (the Timeline related list's read) -> 200 [{"action":"comment","comment":"Reply posted through the declared approval_comment target."},{"action":"submit","comment":null}] POST /api/v1/approvals/requests/REQ/reject -> 200 GET /api/v1/approvals/requests/REQ -> 200 {"status":"rejected","viewer":{"can_act":false,"is_submitter":true,"can_override":false}} served approval_comment.visible on the rejected request row -> false POST /api/v1/approvals/requests/REQ/comment on the rejected request -> 409 {"code":"INVALID_STATE","error":"request is rejected"} ``` 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 own `sys_user_detail` page and the `sys_approval_request` object label, which fall back to English the same way there. ## Tests The same runs at HEAD `8f30e97c3` (after `git merge origin/main` and 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. - New, `src/sys-approval-request-page.test.ts` (16 cases): - the manifest carries the page, and the page parses; - the slot map: one decision node, highlights equal to `highlightFields` minus `record_id`, the two tabs, no discussion, and every named field exists; - `validateComponentTypes`, `validateComponentProps` and `runAuthoringRules('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; - the `en` label equals the page literal, and every locale serves its translation. - New, `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. - Updated: the `sys-approval-request.object.test.ts` roster, plus `comment` in its route verbs and the declaration pins; the `action-predicate-sparse-face.test.ts` count (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 empty `git diff HEAD`: | Mutation | Result | | :--- | :--- | | `visible` gains a `can_override` arm | the parity matrix goes red on exactly `pending · an override admin on no slot` (shown `true`, route `FORBIDDEN`), and so does the declaration pin | | the submitter arm loses its `pending` test | red on exactly `approved · the submitter` and `recalled · the submitter` (shown `true`, route `INVALID_STATE`). The first attempt's anchor missed, so the tool refused it and wrote nothing; it was re-run with the corrected anchor | | the `en` page label is edited | the served `es-ES` label becomes the edited English source. The recorded digests are consulted, and both translation pins go red | | the zh-CN `pages` entry is dropped from the extract config | `check:i18n-coverage` reports "untranslated declared strings grew 0 → 1" for this config, so the ratchet counts the label | ## Gates The gates were derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on `8f30e97c3`. That is 65 commands, plus `pnpm check:i18n-coverage` because the extract config moved. All 66 exited 0 at `8f30e97c3`. The reconciliation `--ran` read: "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-loads` exited 3 with PREREQUISITE NOT MET, because eight packages outside the closure had no `dist/`. 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-files` and `check:nul-bytes`. Lint was narrowed to the 14 changed `.ts` files: `eslint --no-inline-config --format json` gave 14 files, 0 errors, 0 warnings. The population is `eslint.config.mjs`'s `**/*.{ts,…}` set. The config never enables type-aware linting (its own note: "no `parserOptions.project`, no typed `@typescript-eslint` rules"), so this diff cannot move the verdict on any untouched file. The repo-wide `pnpm lint` is left to CI. ## Acceptance notes - **Translations: the PM's mechanism assumption 4 is falsified in part.** The page label is hand-authored (`src/translations/pages.ts`), the way `platform-objects` carries `pages.*` for `sys_user_detail` / `sys_organization_detail` / `sys_position_detail` in its `LOCALE.ts` files. The action's leaves did go through the tooling. The reasons, as measured: - This set's extract command is the objects-only one, so it emits the `objects` sub-tree alone. - No bundle set in the repo generates `pages`. - A config that documents the stack-wide spelling (`--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, so `check:i18n` would check an objects-only extract against a stack-shaped bundle. This is dormant: no config documents that spelling, and no owner is named. - **The Details tab's grouping.** `record:details` renders the object's field groups (`Target` / `State` / `System`), and `sys_approval_request` declares no `fieldGroups`. `os validate` already reports this as 20 `field-group-undeclared` warnings, so those fields render in the ungrouped bucket. This is pre-existing, unchanged here, and has no owner named. - **A public-door refusal for a non-participant is NOT MEASURED** on the dev server: the showcase refuses sign-up (`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). - The console's details/tabs precedence that shapes this page also bears on `platform-objects`' `SysUserDetailPage`. That page authors both `slots.details` and `slots.tabs`, so its `details` override is not drawn. This is reported to the seat, not changed here. --- _Generated by [Claude Code](https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 25be876 commit 5495331

17 files changed

Lines changed: 797 additions & 31 deletions
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/plugin-approvals': minor
3+
---
4+
5+
feat(plugin-approvals): serve the approval request's record page, and declare the thread reply as `approval_comment`
6+
7+
Clause-②: no
8+
9+
**The request's record page is now metadata this plugin serves.** `ApprovalsServicePlugin` registers `sys_approval_request_detail` through its manifest `pages`: a slotted record page, default for every `sys_approval_request` record, that the metadata API serves like any other page. Its slots:
10+
11+
- `actions`: one `record:approval_decision` node (the decision panel: the request's decision progress and its declared decision actions);
12+
- `highlights`: the object's `highlightFields` without `record_id`, so `record_id` stays in the details grid, where the console draws the target record of the `record_id` / `object_name` pointer pair. The object's own `highlightFields` is unchanged;
13+
- `tabs`: Details (`record:details`) and Timeline (`sys_approval_action` as a related list on `request_id`, newest first);
14+
- `discussion: []`: no generic discussion thread. The request's thread is its timeline.
15+
16+
The page label is translated in the plugin's bundle as `pages.sys_approval_request_detail.label` (en, zh-CN, ja-JP, es-ES). The page stays module-internal: nothing new is exported from the package entry.
17+
18+
**`approval_comment` declares the thread reply.** `sys_approval_request` now declares a ninth action, `approval_comment` ("Reply"). It posts `comment` (a required textarea) and `attachments` (optional, several files) to `POST /api/v1/approvals/requests/{id}/comment`, the route the console drawer posted to by hand. The service stores the reply as a `sys_approval_action` row with action `comment`, which the Timeline lists. Its `visible` gate is the route's own admission: a pending approver (`record.viewer.can_act`), or the submitter while the request is `pending`. It has no admin-override arm, because the comment route admits none.

‎content/docs/automation/approvals.mdx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,9 @@ You rarely call these routes by hand. The decision verbs above ship as
613613
**server-declared actions** on `sys_approval_request` (`actions[]` in its object
614614
metadata) — `approval_approve`, `approval_reject`, `approval_reassign`,
615615
`approval_send_back`, `approval_request_info`, `approval_remind`,
616-
`approval_recall`, `approval_resubmit`. The console's generic action runtime
616+
`approval_recall`, `approval_resubmit`, `approval_comment`. `approval_comment`
617+
is the thread reply: a pending approver sees it, and so does the submitter while
618+
the request is `pending`; it has no admin-override arm. The console's generic action runtime
617619
renders and executes them wherever the object is surfaced — the **Approvals
618620
inbox** included — so a decision (comment, a file-upload for `attachments`, the
619621
reassign user-picker) is collected by the generic action dialog with **no

‎packages/plugins/plugin-approvals/scripts/i18n-extract.config.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,28 @@
3939
* `@objectstack/platform-objects` — this plugin translates only the objects it
4040
* owns, so it must not commit a second copy. Add `--check` to run the same
4141
* command as a drift gate.
42+
*
43+
* `pages` declares the record page this plugin ships
44+
* (`sys_approval_request_detail`). The command above writes the objects
45+
* sub-tree only, so the page's one bundle key — its page-level `label` — is
46+
* HAND-AUTHORED in `src/translations/pages.ts`, the way
47+
* `@objectstack/platform-objects` carries its own record pages' labels, and
48+
* that section rides in the `translations` baseline below. Declaring the page
49+
* here is what puts its label in the coverage ratchet's population, counted
50+
* against that section: a page left out of this config is a label no gate
51+
* looks at.
4252
*/
4353

4454
import { defineStack, type ObjectStackDefinition } from '@objectstack/spec';
4555
import { SysApprovalRequest } from '../src/sys-approval-request.object.js';
4656
import { SysApprovalAction } from '../src/sys-approval-action.object.js';
4757
import { SysApprovalDelegation } from '../src/sys-approval-delegation.object.js';
58+
import { SysApprovalRequestDetailPage } from '../src/sys-approval-request.page.js';
4859
import { enObjects } from '../src/translations/en.objects.generated.js';
4960
import { zhCNObjects } from '../src/translations/zh-CN.objects.generated.js';
5061
import { jaJPObjects } from '../src/translations/ja-JP.objects.generated.js';
5162
import { esESObjects } from '../src/translations/es-ES.objects.generated.js';
63+
import { approvalsPageTranslations } from '../src/translations/pages.js';
5264

5365
/**
5466
* The annotation is load-bearing, not decoration (#10868). `defineStack`
@@ -66,11 +78,12 @@ import { esESObjects } from '../src/translations/es-ES.objects.generated.js';
6678
*/
6779
const config: ObjectStackDefinition = defineStack({
6880
objects: [SysApprovalRequest, SysApprovalAction, SysApprovalDelegation] as any,
81+
pages: [SysApprovalRequestDetailPage],
6982
translations: [
70-
{ en: { objects: enObjects } },
71-
{ 'zh-CN': { objects: zhCNObjects } },
72-
{ 'ja-JP': { objects: jaJPObjects } },
73-
{ 'es-ES': { objects: esESObjects } },
83+
{ en: { objects: enObjects, pages: approvalsPageTranslations.en } },
84+
{ 'zh-CN': { objects: zhCNObjects, pages: approvalsPageTranslations['zh-CN'] } },
85+
{ 'ja-JP': { objects: jaJPObjects, pages: approvalsPageTranslations['ja-JP'] } },
86+
{ 'es-ES': { objects: esESObjects, pages: approvalsPageTranslations['es-ES'] } },
7487
],
7588
});
7689

‎packages/plugins/plugin-approvals/src/action-predicate-sparse-face.test.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ const BINDINGS: Array<[string, Record<string, unknown>]> = [
7979
describe('#8990 — sys_approval_request decision actions never fault on a sparse binding', () => {
8080
it('every action predicate reads record.* and is has()-guarded on each path', () => {
8181
const predicates = ACTIONS.map((a) => sourceOf(a.visible)).filter((v): v is string => typeof v === 'string');
82-
expect(predicates.length).toBe(8);
82+
expect(predicates.length).toBe(9);
8383
const unguarded: string[] = [];
8484
for (const source of predicates) {
8585
for (const m of source.matchAll(/record((?:\.[a-z_][a-z0-9_]*)+)/gi)) {
@@ -107,14 +107,14 @@ describe('#8990 — sys_approval_request decision actions never fault on a spars
107107
describe('#8990 — the fail-closed intent survives as a real false, and the levers still open', () => {
108108
it('an absent viewer block closes every viewer-gated lever', () => {
109109
const row = { status: 'pending' };
110-
for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_send_back', 'approval_request_info', 'approval_remind', 'approval_recall', 'approval_resubmit']) {
110+
for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_send_back', 'approval_request_info', 'approval_remind', 'approval_recall', 'approval_resubmit', 'approval_comment']) {
111111
expect([name, evaluate(visibleOf(name), row)]).toEqual([name, false]);
112112
}
113113
});
114114

115-
it('a current pending approver still gets approve / reject / reassign / send back / request info', () => {
115+
it('a current pending approver still gets approve / reject / reassign / send back / request info, and the reply', () => {
116116
const approver = { status: 'pending', viewer: { can_act: true, can_override: false, is_submitter: false } };
117-
for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_send_back', 'approval_request_info']) {
117+
for (const name of ['approval_approve', 'approval_reject', 'approval_reassign', 'approval_send_back', 'approval_request_info', 'approval_comment']) {
118118
expect([name, evaluate(visibleOf(name), approver)]).toEqual([name, true]);
119119
}
120120
// Submitter levers stay closed for them.
@@ -144,6 +144,9 @@ describe('#8990 — the fail-closed intent survives as a real false, and the lev
144144
// moved, and remind/resubmit stay shut for an actor who is not the submitter.
145145
expect(evaluate(visibleOf('approval_remind'), admin)).toBe(false);
146146
expect(evaluate(visibleOf('approval_resubmit'), { ...admin, status: 'returned' })).toBe(false);
147+
// Nor into the thread reply: the comment route admits the submitter and the
148+
// slot holders only, so an override admin on no slot gets no reply button.
149+
expect(evaluate(visibleOf('approval_comment'), admin)).toBe(false);
147150
});
148151

149152
it('the recall override arm is pending-only in effect, because `can_override` is itself pending-scoped (#12716)', () => {
@@ -182,6 +185,16 @@ describe('#8990 — the fail-closed intent survives as a real false, and the lev
182185
expect(evaluate(visibleOf('approval_recall'), { status: 'approved', viewer: submitter })).toBe(false);
183186
expect(evaluate(visibleOf('approval_resubmit'), { status: 'pending', viewer: submitter })).toBe(false);
184187
});
188+
189+
it('the submitter gets the reply while the request is pending, and only then', () => {
190+
// `is_submitter` carries no status test where it is computed, so the
191+
// reply's submitter arm states the `pending` the comment route requires.
192+
const submitter = { can_act: false, can_override: false, is_submitter: true };
193+
expect(evaluate(visibleOf('approval_comment'), { status: 'pending', viewer: submitter })).toBe(true);
194+
for (const status of ['returned', 'approved', 'rejected', 'recalled', 'cancelled']) {
195+
expect([status, evaluate(visibleOf('approval_comment'), { status, viewer: submitter })]).toEqual([status, false]);
196+
}
197+
});
185198
});
186199

187200
describe('#8990 — why the guard form is leaf-first (measured, not preferred)', () => {

‎packages/plugins/plugin-approvals/src/approval-service.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { describe, it, expect, beforeEach, vi } from 'vitest';
1313
import { APPROVAL_REVISE_NODE_TYPE } from '@objectstack/spec/automation';
1414
import { ApprovalService, REMIND_COOLDOWN_MS, ESCALATION_ENABLED_FLIP_CUTOFF_MS, SLA_ACTOR_ID } from './approval-service.js';
1515
import { bindApprovalLockHook, bindDelegationWriteGuard, unbindAllHooks } from './lifecycle-hooks.js';
16+
import { celEngine } from '@objectstack/formula';
17+
import { SysApprovalRequest } from './sys-approval-request.object.js';
1618

1719
interface FakeRow { [k: string]: any }
1820

@@ -2048,6 +2050,122 @@ describe('ApprovalService — admin override (#3424)', () => {
20482050
});
20492051
});
20502052

2053+
// ── approval_comment: the declared reply against the route it posts to ──────
2054+
//
2055+
// `sys_approval_request` declares the thread reply as the action
2056+
// `approval_comment` (POST `/approvals/requests/{id}/comment`, body `comment` +
2057+
// `attachments`, no actor). Its `visible` gate is evaluated on the row the
2058+
// caller is served — `getRequest`, which attaches the `viewer` block — so the
2059+
// gate is only honest if, for every caller shape, "the button shows" is exactly
2060+
// "the route admits the post". This matrix measures both halves against the
2061+
// REAL service for each shape, and the agreement is the pin: a predicate looser
2062+
// than the route is a lever that fails on use, a narrower one a reply nobody
2063+
// can send. The REST route resolves the actor as `body.actorId ?? body.actor_id
2064+
// ?? context.userId`, and the declared action sends no actor, so the service is
2065+
// driven with the caller's own id exactly as the route would drive it.
2066+
describe('approval_comment — its visible gate is the comment route\'s admission', () => {
2067+
let engine: ReturnType<typeof makeFakeEngine>;
2068+
let svc: ApprovalService;
2069+
let n = 0;
2070+
const baseTime = new Date('2026-01-15T10:00:00Z').getTime();
2071+
2072+
const PLATFORM_ADMIN = { userId: 'root', tenantId: 't1', positions: [], permissions: ['admin_full_access'] } as any;
2073+
const SALES_MANAGER = { userId: 'u_sm', tenantId: 't1', positions: ['sales_manager'], permissions: [] } as any;
2074+
2075+
type Action = { name: string; visible?: { dialect: string; source: string } | string };
2076+
const reply = ((SysApprovalRequest as { actions?: Action[] }).actions ?? []).find((a) => a.name === 'approval_comment');
2077+
2078+
/** The predicate's verdict on the row this caller is served; no row, no button. */
2079+
const shown = (row: Record<string, unknown> | null, ctx: any): boolean => {
2080+
if (!row || !reply?.visible) return false;
2081+
const source = typeof reply.visible === 'string' ? reply.visible : reply.visible.source;
2082+
const r = celEngine.evaluate({ dialect: 'cel', source }, { record: row, user: { id: String(ctx.userId ?? '') } });
2083+
if (!r.ok) throw new Error(`approval_comment.visible faulted: ${r.error.message}`);
2084+
return r.value === true;
2085+
};
2086+
2087+
/** The route's verdict: the post as the declared action sends it, or the refusal's code. */
2088+
const admitted = async (id: string, ctx: any): Promise<true | string> => {
2089+
try {
2090+
await svc.comment(id, { actorId: ctx.userId, comment: 'a reply', attachments: ['file_1'] }, ctx);
2091+
return true;
2092+
} catch (err: any) {
2093+
return String(err?.message ?? err).split(':')[0];
2094+
}
2095+
};
2096+
2097+
async function open(input: ReturnType<typeof openInput>): Promise<string> {
2098+
const opened = await svc.openNodeRequest(input, CTX); // submitter u1
2099+
if ('autoApproved' in opened) throw new Error('fixture: the approver slate must not be empty');
2100+
return opened.id;
2101+
}
2102+
2103+
beforeEach(() => {
2104+
engine = makeFakeEngine();
2105+
n = 0;
2106+
svc = new ApprovalService({ engine: engine as any, clock: { now: () => new Date(baseTime + (n++) * 1000) } });
2107+
});
2108+
2109+
it('the action is declared, and posts to the comment route', () => {
2110+
expect(reply, 'sys_approval_request declares approval_comment').toBeDefined();
2111+
});
2112+
2113+
it('every caller shape: the button shows exactly when the route admits the post', async () => {
2114+
const pending = await open(openInput(['u9']));
2115+
const stuck = await open(openInput([], {
2116+
recordId: 'opp2', runId: 'run_2',
2117+
config: { approvers: [{ type: 'position' as const, value: 'sales_manager' }], behavior: 'first_response' as const, lockRecord: true },
2118+
}));
2119+
const approved = await open(openInput(['u9'], { recordId: 'opp3', runId: 'run_3' }));
2120+
await svc.decideNode(approved, { decision: 'approve', actorId: 'u9' }, SYS);
2121+
const recalled = await open(openInput(['u9'], { recordId: 'opp4', runId: 'run_4' }));
2122+
await svc.recall(recalled, { actorId: 'u1' }, CTX);
2123+
2124+
const cases: Array<[string, string, any, boolean, true | string]> = [
2125+
['pending · the submitter', pending, CTX, true, true],
2126+
['pending · a pending approver', pending, asUser('u9'), true, true],
2127+
['pending · a holder of a position slot', stuck, SALES_MANAGER, true, true],
2128+
['pending · an override admin on no slot', pending, PLATFORM_ADMIN, false, 'FORBIDDEN'],
2129+
['pending · a stranger', pending, asUser('u_stranger'), false, 'FORBIDDEN'],
2130+
['approved · the submitter', approved, CTX, false, 'INVALID_STATE'],
2131+
['approved · the approver who decided', approved, asUser('u9'), false, 'INVALID_STATE'],
2132+
['recalled · the submitter', recalled, CTX, false, 'INVALID_STATE'],
2133+
];
2134+
const measured: Array<[string, boolean, true | string]> = [];
2135+
for (const [label, id, ctx] of cases) {
2136+
const row = (await svc.getRequest(id, ctx)) as Record<string, unknown> | null;
2137+
measured.push([label, shown(row, ctx), await admitted(id, ctx)]);
2138+
}
2139+
expect(measured).toEqual(cases.map(([label, , , show, admit]) => [label, show, admit]));
2140+
// The agreement itself, stated without the expected table: shown ⇔ admitted.
2141+
for (const [label, show, admit] of measured) expect([label, show], label).toEqual([label, admit === true]);
2142+
});
2143+
2144+
it('the override admin is a real non-arm, not a reader who cannot see the request', async () => {
2145+
// Without this the admin row above could pass because the admin is simply
2146+
// not served the request. The admin IS served it, with the override flag
2147+
// the decision levers OR in — and the reply deliberately does not.
2148+
const id = await open(openInput(['u9']));
2149+
const row = await svc.getRequest(id, PLATFORM_ADMIN);
2150+
expect(row?.viewer).toMatchObject({ can_act: false, is_submitter: false, can_override: true });
2151+
});
2152+
2153+
it('a reply the route admits is a comment row on the request timeline, attachments included', async () => {
2154+
const id = await open(openInput(['u9']));
2155+
await svc.comment(id, { actorId: 'u1', comment: 'Receipts attached.', attachments: ['file_a', 'file_b'] }, CTX);
2156+
await svc.comment(id, { actorId: 'u9', comment: 'Thanks, reviewing.' }, asUser('u9'));
2157+
const timeline = await svc.listActions(id, asUser('u9'));
2158+
const replies = timeline.filter((a) => a.action === 'comment');
2159+
expect(replies.map((a) => [a.comment, (a.attachments ?? []).map((f) => f.id)])).toEqual([
2160+
['Receipts attached.', ['file_a', 'file_b']],
2161+
['Thanks, reviewing.', []],
2162+
]);
2163+
// A reply moves nothing: the request is still pending on the same slate.
2164+
const row = await svc.getRequest(id, SYS);
2165+
expect([row?.status, row?.pending_approvers]).toEqual(['pending', ['u9']]);
2166+
});
2167+
});
2168+
20512169
// #12769 — the read-back after a successful mutation narrows by the CALLER's
20522170
// org (`loadRequest`), so an org-less request row (every schedule /
20532171
// time-relative / api trigger run produces one — #10131, pinned rather than

‎packages/plugins/plugin-approvals/src/approvals-plugin.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { SysApprovalAction } from './sys-approval-action.object.js';
1212
import { SysApprovalApprover } from './sys-approval-approver.object.js';
1313
import { SysApprovalToken } from './sys-approval-token.object.js';
1414
import { SysApprovalDelegation } from './sys-approval-delegation.object.js';
15+
import { SysApprovalRequestDetailPage } from './sys-approval-request.page.js';
1516
import { renderConfirmPage, renderResultPage } from './action-link-pages.js';
1617
import {
1718
ApprovalService,
@@ -124,6 +125,12 @@ export class ApprovalsServicePlugin implements Plugin {
124125
defaultDatasource: 'cloud',
125126
namespace: 'sys',
126127
objects: [SysApprovalRequest, SysApprovalAction, SysApprovalApprover, SysApprovalToken, SysApprovalDelegation],
128+
// The request's record-detail page (a slotted page, default for every
129+
// sys_approval_request record): the decision panel, the request's
130+
// fields, and its timeline. This plugin owns the object, so it ships its
131+
// page too — the shape plugin-auth ships sys_user's. The page and its
132+
// slot map are documented on `SysApprovalRequestDetailPage`.
133+
pages: [SysApprovalRequestDetailPage],
127134
// ADR-0029 D7 — contribute the Approvals entries into the Setup app's
128135
// `group_approvals` slot. This plugin owns these objects (K2.b), so it
129136
// ships their menu too; when the plugin isn't installed the slot is empty.

0 commit comments

Comments
 (0)