Skip to content

Commit 8a078da

Browse files
fix(app-shell,console): an approver who opens an approval request sees its decision panel again, read from the approvals service (#28619)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9Ubb22AjUGXrSeDsZKcAbfc4AY)_ Fixes #28546 Clause-②: no Metadata-protocol: none domain:ui#4 card thread · os-dev · session_01QsR2jUw276TncPo635XS3B · claim 6554793732 ## 维护者速读 - **改了什么:** 审批请求(`sys_approval_request`)自己的记录页不再走通用数据接口的一次性页面载荷(`POST /api/v1/data/:object/:id/page`),而是像 #27713 之前那样各部分分别读取;控制台把请求本身路由到 `GET /api/v1/approvals/requests/:id`。控制台的两个审批数据源还会直接拒绝请求对象的 `getRecordPage`,预取则什么都不发。 - **为什么改:** 非管理员审批人在数据接口上没有该对象的读权限,页面载荷答 403,从审批收件箱、`?request=` 链接或直接打开记录页都只看到“无权访问”,没有同意/拒绝;管理员审批人拿到的是数据库原样行,没有 `viewer`,所以决策按钮不出现,`pending_approvers` 是逗号拼接的字符串,“等待”一栏按字符一个一个画芯片。这是 18.3.0 的发版阻塞项。 - **风险与回滚:** 只影响审批请求这一个对象的记录页:多了几个分开的请求(与 #27713 之前相同),其它对象的记录页仍是一个请求。数据接口的权限没有放宽。回滚 = revert 本 PR。 - **先看:** `ui/packages/app-shell/src/views/RecordDetailView.tsx`(`recordPagePlan` 里对请求对象提前返回空计划,以及抽屉里相关计数预热的条件);`ui/apps/console/src/services/approvalRequestsDataSource.ts`(`guardRecordPageRead`);`ui/apps/console/src/services/approvalRequestsDataSource.recordPagePayload-28546.test.tsx`(真实 `ObjectStackAdapter` 下的验收用例)。 - **两分钟验证:** `cd ui && pnpm exec vitest run apps/console/src/services/approvalRequestsDataSource.recordPagePayload-28546.test.tsx` → 10 个用例全绿:非管理员从收件箱点开、冷加载记录页、`?request=` 深链都看到决策面板,同意一次后按钮消失;管理员审批人看到同意/拒绝和每人一个芯片;整个过程没有任何发往 `/api/v1/data/sys_approval_request` 的请求。 - **需要你做的:** 无,席位落地。 ## What changes **The defect, measured.** Since #27713 the record page takes its record from `DataSource.getRecordPage`, the data API's `POST /data/:object/:id/page`. `routeApprovalRecordReads` overrides only `find` and `findOne`, so on a request's own page that read reached the console's adapter unchanged: - a **non-admin approver** is refused `sys_approval_request` on the data API, so the payload answered 403 and the page drew `record-access-denied` ("You don't have access to Approval Request records"), on the inbox row, on a cold load of the record route and on the `?request=` deep link; - an **admin approver** was answered the stored row: no `viewer` block, so the decision actions' `record.viewer.can_act` predicates failed closed (no Approve, no Reject), and `pending_approvers` as the comma-joined string, so the "Waiting on" strip drew one chip per character. Both are reproduced by the new console test on the base sources (below), over the real `ObjectStackAdapter`. **The fix routes the read, not the grant** (triage 6554139580). Nothing about what the data API lets an approver read changes. - `RecordDetailView`: `recordPagePlan` is empty for `sys_approval_request`, so a request's page asks no payload. Every part reads on its own through the adapter, as before #27713: the record through `findOne`, which the console routes to `GET /approvals/requests/:id` (with `viewer`, the decision tally and the approvers as an array); the timeline through the actions route; the header verdicts and the field projection through their own reads. The now-constant `approvals` flag of the plan reads `true`. - `RecordDetailView`, drawer: the related-count warm-up for the next record was skipped whenever the adapter has `prefetchRecordPage`. It is now skipped only when the open actually asks a payload, so a request opened in a drawer keeps warming its neighbour's counts (otherwise this change would leave it with neither). - `approvalRequestsDataSource.ts`: both routed sources (`routeApprovalRecordReads` and the inbox's scoped `createApprovalRequestsDataSource`) refuse a request's `getRecordPage` with the existing `UNSUPPORTED_QUERY_PARAM` refusal before any request goes out, and send nothing for a request's `prefetchRecordPage`. Every other object's call is the host's, and over a host without the payload neither member appears. This guard is not reachable from today's UI (the page asks no payload for a request); it keeps a future caller from drawing a request from the data API's row through this door, which is how this card and #25228 both happened. **File surface:** exactly the claim's (`ui/apps/console/src/services/**`, `ui/packages/app-shell/src/views/RecordDetailView.tsx`) plus one new app-shell test file beside it and the changeset. No producer change elsewhere was needed: the approvals get already serves the request correctly. ## Tests - New `ui/apps/console/src/services/approvalRequestsDataSource.recordPagePayload-28546.test.tsx` (10 cases), the console's composition over the **real** `ObjectStackAdapter` under `ApprovalRecordReads`, the real inbox page on the account app's `component/approvals/inbox` route and the real `RecordDetailView` with the served request page. Only `fetch` (answered as the server does, per persona) and the inbox's queue read are doubled. - `approvals.account-app-entry` acceptance[3]: a row click opens `/apps/com.objectstack.account/sys_approval_request/record/ID`, the decision panel draws Approve and Reject, one approval POSTs through the action door, the request is re-read from the approvals get and the decisions stop rendering. - acceptance[3], cold load of the record route. - acceptance[4]: `?request=ID` on the component route replaces itself with the record route, which shows that request; Back leaves to the previous entry. - an admin pending approver, whom the data API's page route answers 200: Approve and Reject draw, and the strip has one chip per approver (`Dev Admin`, `Ada Auditor`, `Lee Legal`). - every case asserts the page sent **no** request to `/api/v1/data/sys_approval_request` at all, and no `record-access-denied`. - both routed sources: a request's `getRecordPage` rejects with `UNSUPPORTED_QUERY_PARAM` and never reaches the host; a request's prefetch is dropped; another object's calls are the host's; CONTROL: over a host without the payload, neither member exists. - New `ui/packages/app-shell/src/views/RecordDetailView.approvalRequestPage-28546.test.tsx` (3 cases, real adapter): a request's page reads its record by `findOne` and sends no page request; CONTROL: an ordinary record on the same adapter opens with its page request; in a drawer, a request's next neighbour has its related counts warmed and no page is asked. <details><summary>Readings (head 6d6aea8 unless named)</summary> - New files on the fix, `6d6aea80`: `pnpm exec vitest run` of both new files plus `RecordDetailView.recordPagePayload-27627.test.tsx` and `RecordDetailView.recordReadFailure-11902.test.tsx` (the two existing payload pins): **4 files, 28 tests passed**. - **Red on the old code** (reverse verification, `d8234f05` with `RecordDetailView.tsx` and `approvalRequestsDataSource.ts` restored to the base `617dcf43`, blob hashes checked on disk, restored from `HEAD` with `git diff HEAD` empty): **10 failed, 3 passed** (the 3 are the CONTROL cases). The member cases drew `record-access-denied` ("You don't have access to Approval Request records"); the admin case drew the panel with no `declared-action-approval_approve` and five `approver-chip`s for the slate `u1,u2,u3`, the issue's per-character chips. - Ablation, drawer warm-up gate alone (`recordPagePlan &&` removed through `scripts/ablation-replace.mjs`, anchor 1 to 0, restored blob equal to `HEAD`): exactly the drawer case red (1 failed, 2 passed). - Ablation, `routeApprovalRecordReads`' guard alone: exactly its two pins red (2 failed, 8 passed); the integration cases stay green because the page asks no payload for a request. The guard is declared above as unreachable from today's UI. - Neighbouring suites, every `ui/packages/app-shell` and `ui/apps/console` test file that names `RecordDetailView`, `recordPagePayload`, `approvalRequestsDataSource`, `routeApprovalRecordReads` or `ApprovalRecordReads` (`git grep -l`, 227 files) at `010fbf63`, two locked batches at `--maxWorkers=2`: **114 files / 903 tests passed** and **113 files / 817 tests passed**. A narrowing: CI's Test UI runs the rest. The merge of main at `6d6aea80` touched no file under `ui/`. - `pnpm --filter @object-ui/app-shell --filter @object-ui/console run type-check`: both `Done`, exit 0 (`010fbf63`). </details> ## Acceptance notes - The live retest on the P0 item (`approvals.account-app-entry`, a booted showcase with the invited member) is QA's, per the triage. NOT MEASURED here: no browser run against a booted showcase; the jsdom harness runs the shipped adapter, routed source, inbox page and record view, with only the HTTP transport doubled. - #28470 (open, unclaimed) proposes adding more parts to the same record page payload. With this PR a request's page asks none of it, so that proposal does not reach approval requests. ## Verification <details><summary>Derived gates, local subset (34 of 34, head 6d6aea8)</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 34 families for this diff; every one ran on `6d6aea80` and exited 0, with `GITHUB_EVENT_PATH` holding this body for the body-reading gates. `dispatch-gates --ran`: "34 derived famil(ies) accounted for — 34 run, 0 NOT-MEASURED". Changed lines 765, under the 3000 human-merge threshold; no governed surface touched. - `check-adr-0087-registration --base origin/main` and `--self-test`, `check-changeset-no-major --base origin/main` (and `--event`), `check-closing-keyword-parity` and `--self-test`, `check-comment-mask-corpus`, `check-empty-changeset --base origin/main` and `--self-test`, `check-sdui-manifest` and `--self-test`, `release-rehearsal-clone --self-test`, `release-pending-publish --self-test`: exit 0. - `pnpm --dir ui exec vitest run scripts/` (locked): 169 files passed, 2 skipped; 4900 tests passed. - `check:duration-unit-keys`, `check:changeset-gate-self-tests`, `check:closed-ceremony-text`, `check:cross-package-test-inputs`, `check:dispatcher-error-vocabulary`, `check:driver-memory-census`, `check:engine-double-contract`, `check:gitlink-declared`, `check:nul-bytes`, `check:pm-changeset-deadline-census`, `check:published-files`, `check:query-options-erasure` (locked), `check:refd-timer-probe`, `check:skill-identifier-liveness`, `check:test-source-alias`, `check:tier-file-adoption`, `check:type-check-coverage`, `check:type-check-debt` (locked), `check:undeclared-dep-imports`, `check:watch-hint-literal`, `check:where-matcher`: exit 0. - Not derived, run for the body: `check-partof-closing-keyword` exit 0. - Every family CI runs is read from this PR's check-runs, not re-run here. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jUw276TncPo635XS3B)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 909c8c9 commit 8a078da

5 files changed

Lines changed: 756 additions & 9 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/console': patch
4+
---
5+
6+
fix(app-shell,console): an approver who opens an approval request sees its decision panel again, read from the approvals service
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
Since the record page started opening with one request (`POST /api/v1/data/:object/:id/page`), an approval request's own page took the request from that data API read. A non-admin approver is refused `sys_approval_request` there, so opening a request from the Approvals inbox, by its `?request=` link or by a cold load of its record route drew "You don't have access to Approval Request records" with no Approve, Reject or Reassign. An admin approver was answered the stored row: it carries no per-caller `viewer` block, so the decision actions stayed hidden, and its `pending_approvers` is the comma-joined string, so the "Waiting on" strip drew one chip per character.
12+
13+
A request's page now asks no record page payload. Each part reads on its own through the adapter, as before the payload, and the Console routes the request read to `GET /api/v1/approvals/requests/:id`, which serves `viewer`, the decision tally and the approvers as an array. Nothing about what the data API lets an approver read changes. Every other record page still opens with its one request.
14+
15+
The Console's routed adapters (`routeApprovalRecordReads` and the inbox's scoped source) now refuse a request's `getRecordPage` with the `UNSUPPORTED_QUERY_PARAM` refusal and send nothing for a request's `prefetchRecordPage`, so a request's page can no longer be drawn from the data API's row through that door. A record shown in a drawer whose open asks no payload keeps warming the next record's related counts.

0 commit comments

Comments
 (0)