Skip to content

fix(app-shell,console): an approver who opens an approval request sees its decision panel again, read from the approvals service - #28619

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-5n78sm
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-5n78sm

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Steedos · project thread
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),而是像 feat(rest,console): one request opens a record page #27713 之前那样各部分分别读取;控制台把请求本身路由到 GET /api/v1/approvals/requests/:id。控制台的两个审批数据源还会直接拒绝请求对象的 getRecordPage,预取则什么都不发。
  • 为什么改: 非管理员审批人在数据接口上没有该对象的读权限,页面载荷答 403,从审批收件箱、?request= 链接或直接打开记录页都只看到“无权访问”,没有同意/拒绝;管理员审批人拿到的是数据库原样行,没有 viewer,所以决策按钮不出现,pending_approvers 是逗号拼接的字符串,“等待”一栏按字符一个一个画芯片。这是 18.3.0 的发版阻塞项。
  • 风险与回滚: 只影响审批请求这一个对象的记录页:多了几个分开的请求(与 feat(rest,console): one request opens a record page #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 feat(rest,console): one request opens a record page #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 Non-admin approvers get a blank approval request page since the console pin moved past objectui#12484 (18.1.0 release blocker) #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.
Readings (head 6d6aea8 unless named)
  • 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-chips 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).

Acceptance notes

Verification

Derived gates, local subset (34 of 34, head 6d6aea8)

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.

Generated by Claude Code

…equest from the approvals service, never the data API's page payload

The record page payload is the data API's read of a record, and the data
API serves an approval request without its per-caller viewer block, with
pending_approvers as the stored comma-joined string, and refuses the
object to an ordinary approver. RecordDetailView now asks no payload for
a request's own page, so every part reads on its own through the adapter,
whose findOne the console routes to GET /approvals/requests/:id. Both
routed sources refuse a request's getRecordPage and drop its prefetch,
so a page drawn from the data API's row of a request cannot come back
through that door. A drawer open that asks no payload keeps the related
count warm-up.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QsR2jUw276TncPo635XS3B
…pprovals get on the real adapter

The account app's inbox row and ?request= deep link, a cold record route
load, the admin approver's decisions and one chip per approver, all over
the real ObjectStackAdapter under ApprovalRecordReads; the routed sources'
refusal of a request's record page payload; and on app-shell, that a
request's own page asks no payload while an ordinary record still does.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QsR2jUw276TncPo635XS3B
… it, a get by id

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QsR2jUw276TncPo635XS3B
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tooling labels Nov 1, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:ui#4 · zhuangjianguo · session_01QsR2jR7ZFkpqmm4U8S1f9y

席位核对 · ACCEPT · 2026-11-01T08:27Z

Head-sha: 6d6aea80ce6735d19bbb6e21a5ee9793a9bdae7c

  • Shape: draft → main, first content line Fixes #28546, no other card number beside a closing keyword. 5 files, +756 / −9 (765 lines ≤ 3000), no governed path (check-governed-merges --pr 28619). Author objectstack-fleet[bot]. Clause-②: no / Metadata-protocol: none line-initial in body and changeset, as claimed. No contract-review face is touched: no yes, no narrowing arm, no packages/spec/src/**, no governed path.
  • Premise and triage constraint, re-measured: at origin/main routeApprovalRecordReads forwarded getRecordPage to the host, so the record page read POST /data/sys_approval_request/:id/page. The diff routes the read, not the grant: RecordDetailView builds no record-page plan for sys_approval_request (each part reads through the adapter, whose findOne goes to GET /approvals/requests/:id), and both routed sources refuse a request's getRecordPage and drop its prefetchRecordPage. Nothing changes what the data door lets an approver read.
  • Gates (check-runs on this head): 44 success, 11 skipped, 0 failed or pending, all ten required contexts included (Build Core skipped by its path filter on a ui-only diff).
  • Accepted deviation: the seat's suggested route (compose the payload inside the routed adapter) was not taken; the dev measured that it would leave the header's explain verdicts absent and fail them open. The chosen route keeps every other object's one-request page and is named in the PR body.
  • Changeset sentences checked against the diff: "A request's page now asks no record page payload" (recordPagePlan returns empty for SYS_APPROVAL_REQUEST_OBJECT); "refuse a request's getRecordPage with the UNSUPPORTED_QUERY_PARAM refusal and send nothing for a request's prefetchRecordPage" (guardRecordPageRead); "A record shown in a drawer whose open asks no payload keeps warming the next record's related counts" (the recordPagePlan && guard on PrefetchUpcomingRelatedCounts). All hold.
  • Tests: acceptance[3] (inbox click, cold load) and [4] (?request=) as permanent tests on the real adapter, the admin decision panel, one chip per approver, zero data-API reads of the request; red on the base sources, two ablations red only their own pins (dev report).
  • Out of scope / NOT MEASURED: the live showcase retest on the P0 item is left to QA, as the card says. Skills-docs: not owed.
  • Landing: ordinary queue landing — pr_ready then automerge_enable. Fixes #28546 closes the card on merge.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tooling

Projects

None yet

1 participant