Skip to content

Commit 14208f2

Browse files
fix(core): a row menu's url action reads the row's ctx.recordId and ctx.record (#29127)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbN1EehYQ4h5a3GavPuVMmrb)_ Fixes #28995 Clause-②: no Metadata-protocol: none Declarations as set by the claim (card comment 6571768622) and its amendment (card comment 6571831284), which fixes the file surface: `ActionRunner.ts`, the new test beside it and a root changeset. ## 维护者速读 - **改了什么**:列表视图行菜单(`list_item`)里的 `type: 'url'` 动作,目标地址中的 `${ctx.recordId}` 与 `${ctx.record.*}` 现在取被点开的那一行;此前在行菜单里插值为空,在记录头部菜单里正常。同一规则也作用于行菜单里 `type: 'api'` 动作的 target 和 `onSuccess.navigate` 跳转。`${ctx.selection.ids}` 不变,仍是列表勾选集。 - **为什么改**:duly 在 `sys_user` 上声明的「打开成员页」url 动作,从行菜单点击落到 `/apps/…/sys_user/record/page/duly_member`(缺记录 id),只能退回仅在记录头部提供。规范把 `ctx.recordId` 写成 url 动作可用的上下文,一行就是一条记录的上下文。 - **风险与回滚**:低。只改动作运行器拼装 `${ctx.*}` 插值作用域的一处:分发携带行(`params._rowRecord`)时,行绑定为 `ctx.record`,其 `id` 为 `ctx.recordId`,且压过页面级记录(相关列表行在记录页上读子行)。不带行的分发与此前逐字节相同。回滚即还原本 PR。 - **先看**:`ui/packages/core/src/actions/ActionRunner.ts` 的 `buildInterpolationContext`(末尾新增的行绑定);测试 `ui/packages/core/src/actions/__tests__/ActionRunner.rowTargetContext.test.ts`。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/core/src/actions/__tests__/ActionRunner.rowTargetContext.test.ts`,预期 1 个文件、5 个测试全绿。 - **需要你做的**:无,席位落地。 ## What changes - **One rule for the row, now on targets too.** A list view's row menu (`ObjectGrid`'s row actions) dispatches a `list_item` action as `{ ...def, params: { _rowRecord: row } }` through the page's action runner, whose context names the page, not the row. `ActionRunner.buildInterpolationContext`, the `${ctx.*}` scope behind `interpolateTarget`, read that context only, so on a list page `${ctx.recordId}` and `${ctx.record.*}` printed empty. It now takes the action and, when the dispatch carries a row, binds `ctx.record` to the row and `ctx.recordId` to its `id`, after the `ctx` mirror `ActionProvider` publishes, so the row wins over a page-level record. This is the rule `templateEvaluatorFor` already applies to a row action's copy templates (objectui#12967), via the same `dispatchRow` helper. - **Reach.** `interpolateTarget` serves `type: 'url'` targets (and the `navigation` alias), `type: 'api'` targets before a registered api handler receives them, and the `onSuccess.navigate` hop, so all three read the row from a row menu. - **Unchanged.** `ctx.selection.ids` / `ctx.selection.count` stay the grid's checked selection: opening a row's menu does not select the row. A dispatch with no row reads exactly what it read before. No other row surface or host was touched; `ListView.tsx` and `plugin-grid` are not in the diff. ## Tests - New `ActionRunner.rowTargetContext.test.ts` drives the same `type: 'url'` action through two doors: a runner carrying `record` (flat and in the `ctx` mirror, as `ActionProvider` publishes it) and a list-page runner with the row stashed as `ObjectGrid` stashes it. It pins `${ctx.recordId}` and `${ctx.record.FIELD}` filled identically on both doors, the row winning over a page-level record, `${ctx.selection.*}` staying the checked selection under a row dispatch, and a CONTROL with no row on a list page still reading no record. ## Acceptance notes - **Live browser not measured.** The pin runs the real runner in vitest with a recording navigation handler; the console's own url path is that same runner (`useConsoleActionRuntime` registers no url handler), so the click on a live row menu goes through the changed function. - **Selection by design.** The card names `${ctx.selection.ids}` among the empty holes. In a row menu with nothing checked, empty is the documented answer (`view.zod` documents it as the toolbar selection, empty when nothing is selected), so it is left as is. ## Verification <details><summary>New pin · 5/5 here, 3 red against main's runner</summary> `pnpm --dir ui exec vitest run --maxWorkers=2 packages/core/src/actions/__tests__/ActionRunner.rowTargetContext.test.ts` under `os-verify-lock` at `45dbb5d8`: Test Files 1 passed (1), Tests 5 passed (5). Reverse check at `45dbb5d8`: `ActionRunner.ts` restored from `origin/main` (`5d3667e2`; the row binding's count on disk 0), same command: Tests 3 failed, 2 passed. The `${ctx.recordId}`, `${ctx.record.FIELD}` and row-over-page cases failed; the selection case and the CONTROL passed, as designed. Restored with `git checkout HEAD --`; blob hash back to `76bfe046`, `git diff HEAD` empty. </details> <details><summary>Core package, type check and lint</summary> - `pnpm --dir ui exec vitest run --maxWorkers=2 packages/core/` (locked): Test Files 230 passed (230), Tests 4236 passed, 27 skipped. - `turbo run type-check --filter=@object-ui/core` (`tsc --noEmit && tsc -p tsconfig.test.json`): exit 0. - `eslint` on the two changed `.ts` files: 0 errors, 15 warnings, all on lines outside this diff's hunks. - `pnpm --dir ui run check:new-line-citations`: 0 new citations. - No other ui test file interpolates `${ctx.recordId}` or `${ctx.record.*}` through a row dispatch (`git grep` over ui test files: four app-shell files name the holes, none with a row stash); a dispatch without a row reads exactly as before. CI's Test UI runs the rest. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands` derived 34 commands against merge base `5d3667e29` at head `45dbb5d8`. All 34 ran and exited 0, including `check-sdui-manifest.mjs` and its `--self-test`, `check:type-check-debt`, `check:nul-bytes`, `check:cross-package-test-inputs`, `check:test-source-alias` and `pnpm --dir ui exec vitest run scripts/` (Test Files 169 passed, 2 skipped; Tests 4900 passed, 2 skipped). `dispatch-gates --ran` reconciled 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. `origin/main` moved 5 commits since the base, none touching `ActionRunner.ts`. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jV25i2Lc1VE2P98L8u)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent af8448f commit 14208f2

3 files changed

Lines changed: 119 additions & 4 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/core': patch
3+
---
4+
5+
fix(core): a row action's `${ctx.recordId}` and `${ctx.record.*}` target holes read the row it was opened on
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A `type: 'url'` action shown in a list view's row menu (`locations: ['list_item']`) interpolated `${ctx.recordId}` and `${ctx.record.*}` to empty, so a target such as `/apps/APP/OBJECT/record/${ctx.recordId}/page/PAGE` opened `/apps/APP/OBJECT/record/page/PAGE`. The same action in the record header worked. The row menu dispatches the clicked row under `params._rowRecord` through the page's action runner, and the runner built the `ctx` scope from its own context, which on a list page names no record.
11+
12+
Now a dispatch that carries a row binds `ctx.record` to that row and `ctx.recordId` to its `id` for target interpolation: `type: 'url'` targets, `type: 'api'` targets and the `onSuccess.navigate` hop. The row wins over a page-level record, so a related-list row on a record page reads the child row. This is the rule the runner already applied to a row action's copy templates. `${ctx.selection.ids}` and `${ctx.selection.count}` are unchanged: they stay the grid's checked selection, which opening a row menu does not change.

‎ui/packages/core/src/actions/ActionRunner.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2997,7 +2997,9 @@ export class ActionRunner {
29972997
* fragment.
29982998
*
29992999
* `ctx.origin`, `ctx.user.*`, `ctx.org.*`, `ctx.recordId` are surfaced
3000-
* implicitly from the runner's ActionContext + window, and
3000+
* implicitly from the runner's ActionContext + window; a dispatch that
3001+
* carries a row binds `ctx.record` / `ctx.recordId` to that row (see
3002+
* `buildInterpolationContext`), and
30013003
* `ctx.selection.ids` / `ctx.selection.count` reflect the grid's current
30023004
* checkbox selection (`context.selectedRecords`) — `${ctx.selection.ids}`
30033005
* comma-joins the ids via the standard `String(array)` behaviour, so a
@@ -3059,7 +3061,7 @@ export class ActionRunner {
30593061
// (`composeSuccessMessage`, until objectstack#27948's step 2 retires it):
30603062
// `result` only, because that is the one scope the contract declares for the
30613063
// copy, and no encoding, because a toast is text.
3062-
const scopes: Record<string, unknown> = { param: params, ctx: this.buildInterpolationContext() };
3064+
const scopes: Record<string, unknown> = { param: params, ctx: this.buildInterpolationContext(action) };
30633065
if (resultScope !== undefined) scopes.result = resultScope;
30643066
return substituteScopeTokens(target, scopes, encodeURIComponent);
30653067
}
@@ -3068,8 +3070,19 @@ export class ActionRunner {
30683070
* Assemble the `ctx` namespace exposed to target interpolation.
30693071
* Built-in keys: origin, user, org, recordId. Anything the consumer
30703072
* placed under context.ctx is merged on top.
3071-
*/
3072-
private buildInterpolationContext(): Record<string, unknown> {
3073+
*
3074+
* The record is the one the dispatch runs against, the rule
3075+
* `templateEvaluatorFor` applies to copy templates (objectui#12967). A row
3076+
* surface (a list view's row menu, a related list's rows) stashes the clicked
3077+
* row under `params._rowRecord` and fires through a runner whose context
3078+
* names the page, not the row. So when the dispatch carries a row,
3079+
* `ctx.record` is that row and `ctx.recordId` its `id`, over both the
3080+
* runner's own record and the `ctx` mirror: a related-list row on a record
3081+
* page is the child, not the page's record. `ctx.selection` is untouched:
3082+
* it stays the grid's checkbox selection, which a row dispatch does not
3083+
* change.
3084+
*/
3085+
private buildInterpolationContext(action: ActionDef): Record<string, unknown> {
30733086
const ctx: Record<string, unknown> = {
30743087
origin: typeof window !== 'undefined' ? window.location.origin : '',
30753088
// Backend API origin (typically `import.meta.env.VITE_SERVER_URL`
@@ -3102,6 +3115,11 @@ export class ActionRunner {
31023115
if (this.context.ctx && typeof this.context.ctx === 'object') {
31033116
Object.assign(ctx, this.context.ctx);
31043117
}
3118+
const row = dispatchRow(action);
3119+
if (row) {
3120+
ctx.record = row;
3121+
ctx.recordId = row.id;
3122+
}
31053123
return ctx;
31063124
}
31073125

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* A row action's `${ctx.*}` target holes read the row, the way the record
11+
* header's do.
12+
*
13+
* The record header binds `record` in the runner's context (`RecordDetailView`'s
14+
* `ActionProvider`, which also mirrors it under `ctx`). A list row binds nothing
15+
* there: `ObjectGrid` dispatches a row action as
16+
* `{ ...def, params: { _rowRecord: row } }` through the page's runner. The
17+
* target interpolation read the runner's context only, so a `list_item` url
18+
* action declared as `/apps/APP/OBJECT/record/${ctx.recordId}/page/P` landed
19+
* on `/apps/APP/OBJECT/record/page/P` from a row menu.
20+
*
21+
* Each pin drives the SAME action through both doors:
22+
* - header — a runner whose context carries `record` (flat and in the `ctx`
23+
* mirror, as `ActionProvider` publishes it), dispatch without a row;
24+
* - row — a runner whose context carries no `record` (a list page), the
25+
* dispatch carrying the row exactly as `ObjectGrid` stashes it.
26+
*/
27+
import { describe, it, expect, vi, type Mock } from 'vitest';
28+
import { ActionRunner, type ActionDef, type NavigationHandler } from '../ActionRunner';
29+
30+
const ROW = { id: 'usr_1', name: 'Ada' };
31+
32+
const OPEN_MEMBER: ActionDef = {
33+
name: 'member_open',
34+
objectName: 'sys_user',
35+
type: 'url',
36+
target: '/apps/app_x/sys_user/record/${ctx.recordId}/page/member',
37+
};
38+
39+
/** `ActionProvider`'s context shape: the flat keys plus their `ctx` mirror. */
40+
const provided = (context: Record<string, unknown>) => new ActionRunner({ ...context, ctx: { ...context } });
41+
/** The record header. */
42+
const headerDoor = () => provided({ objectName: 'sys_user', record: { ...ROW } });
43+
/** A list page: the context names the object, not a record. */
44+
const listDoor = () => provided({ objectName: 'sys_user' });
45+
/** `ObjectGrid`'s row dispatch shape: the row under `params._rowRecord`. */
46+
const asRow = (def: ActionDef, row: Record<string, unknown> = ROW): ActionDef => ({ ...def, params: { _rowRecord: row } });
47+
48+
async function navigatedTo(runner: ActionRunner, def: ActionDef): Promise<string> {
49+
const nav: Mock<NavigationHandler> = vi.fn();
50+
runner.setNavigationHandler(nav);
51+
const result = await runner.execute(def);
52+
expect(result.success).toBe(true);
53+
expect(nav).toHaveBeenCalledTimes(1);
54+
return nav.mock.calls[0][0];
55+
}
56+
57+
describe('ActionRunner — a row action interpolates its target from the row', () => {
58+
it('`${ctx.recordId}` reads the same id from the record header and from a list row', async () => {
59+
expect(await navigatedTo(headerDoor(), OPEN_MEMBER)).toBe('/apps/app_x/sys_user/record/usr_1/page/member');
60+
expect(await navigatedTo(listDoor(), asRow(OPEN_MEMBER))).toBe('/apps/app_x/sys_user/record/usr_1/page/member');
61+
});
62+
63+
it('`${ctx.record.FIELD}` reads the row on a list page', async () => {
64+
const def: ActionDef = { ...OPEN_MEMBER, target: '/people?id=${ctx.record.id}&name=${ctx.record.name}' };
65+
expect(await navigatedTo(headerDoor(), def)).toBe('/people?id=usr_1&name=Ada');
66+
expect(await navigatedTo(listDoor(), asRow(def))).toBe('/people?id=usr_1&name=Ada');
67+
});
68+
69+
it('the row wins over a page-level record (a related-list row on a record page)', async () => {
70+
const onParentPage = provided({ objectName: 'sys_org', record: { id: 'org_1', name: 'Parent' } });
71+
const def: ActionDef = { ...OPEN_MEMBER, target: '/people/${ctx.recordId}?name=${ctx.record.name}' };
72+
expect(await navigatedTo(onParentPage, asRow(def))).toBe('/people/usr_1?name=Ada');
73+
});
74+
75+
it('`${ctx.selection.ids}` stays the checked selection, which a row dispatch does not fill', async () => {
76+
const def: ActionDef = { ...OPEN_MEMBER, target: '/people?ids=${ctx.selection.ids}&count=${ctx.selection.count}' };
77+
expect(await navigatedTo(listDoor(), asRow(def))).toBe('/people?ids=&count=0');
78+
const checked = new ActionRunner({ objectName: 'sys_user', selectedRecords: [{ id: 'usr_2' }, { id: 'usr_3' }] });
79+
expect(await navigatedTo(checked, asRow(def))).toBe('/people?ids=usr_2%2Cusr_3&count=2');
80+
});
81+
82+
it('CONTROL: a dispatch with no row on a list page still reads no record', async () => {
83+
expect(await navigatedTo(listDoor(), OPEN_MEMBER)).toBe('/apps/app_x/sys_user/record//page/member');
84+
});
85+
});

0 commit comments

Comments
 (0)