Skip to content

Commit 5da2e88

Browse files
fix(plugin-approvals): the approval request page's highlights strip names the current step by its label (#24374)
Fixes #24281 Clause-②: no The served `sys_approval_request_detail` record page's highlights strip now names the current step by its label. It lists `current_step_label` (the "Approval Step" field) where it listed `current_step`, the Approval node's machine id, so a request waiting on the `lead_review` node reads `Lead Review` there. The approvals get's row builder now serves `current_step_label`, the member `ApprovalRequestRow` declares for this (`370833a303`, #24309), with the value `step_label` carries. The object's `highlightFields` makes the same swap. Responsibility: `plugin-approvals`' own row builder and page declaration | the platform path is the declared column `current_step_label` and its contract member, both on `main`, which the row builder did not serve under that name | reached by every user who opens a pending request's served page, whose Current Step chip showed the node's machine id. Readings are against objectstack `c4526a705` (this branch's base) and objectui at the console pin `.objectui-sha` = `0707b4ae3`, by `git show` and `git grep`, plus one wire reading on a booted showcase (below). The page was not driven in a browser. ## Which of the card's two routes, and why The card asks the fix to say which: **the get now serves the label under a field name the page can list.** The page names its object's declared fields, and `step_label` is not one. `current_step_label` is a declared field of `sys_approval_request` and a declared member of `ApprovalRequestRow`, so `rowFromRequest` serves it from the same `requestDisplayLabels` value as `step_label`. No new column, field, formula or contract member, and no `packages/spec` edit. ## Which doors gain the member `rowFromRequest` is the only builder of a served `ApprovalRequestRow`. It has three call sites, and no other site builds a row: - `openNodeRequest`'s return; - `listRequests`, behind `GET /api/v1/approvals/requests`; - `loadRequest`, behind `getRequest` (`GET /api/v1/approvals/requests/ID`) and `readBackRequest`. `readBackRequest` builds the `request` that `decideNode`, `recall`, `sendBack`, `resubmit`, `reassign`, `remind`, `requestInfo` and `comment` echo back. The action-token chain behind the emailed links also reads through `getRequest`. The two REST routes pass the rows through unchanged (`res.json`, `packages/rest/src/approvals-routes.ts`). So every one of these doors now serves `current_step_label` beside `step_label`. ## Readers of `step_label`, measured `step_label` stays served, with the same value. No reader moves. - This repo: `action-link-pages.ts:66` (`req.step_label || req.current_step`), the emailed confirm and result pages. Unchanged. - objectui at the pin: the inbox (`apps/console/src/pages/system/ApprovalsInboxPage.tsx:314`, `:1525`; the type in `approvalsApi.ts:68`) and the record page's approvals panel (`packages/app-shell/src/views/RecordApprovalsPanel.tsx:372`; the type in `useRecordApprovals.ts:97`). Unchanged. - No objectui source reads `current_step_label` at the pin, so the added key changes nothing for an existing reader. ## The strip and the object's list change together The rule in the page header still holds at the pin. The Details tab is a bare `record:details` node. objectui's `record-details.tsx` draws the object's `highlightFields` minus the mounted strip when a node authors neither `sections` nor `fields` (`bodyFieldsOmitted`, objectui#12433). A strip-only swap would have put a "Current Step: lead_review" row into that tab. So both lists swap `current_step` for `current_step_label`, and the object's list minus the strip is still exactly `record_id`. Other readers of the object's `highlightFields`, re-read at the pin (the same census PR #24279 took): - `ObjectView` and `ObjectGrid` default columns: only for a list view that declares no `columns`. All four `sys_approval_request` list views declare columns, and three already list `current_step_label`. Not reached. - `InterfaceListPage` default columns, `ObjectKanban` card fields, `RelatedList` default columns: no interface page, kanban view or related list of requests is declared in this repository. Not reached. - `deriveLookupColumns` (lookup picker preview): the two lookups to a request, `sys_approval_action.request_id` and `sys_approval_approver.request_id`, sit on objects whose `enable.apiMethods` is `['get', 'list']`, so no create or edit form opens that picker. Not reached. - `RecordDetailView`'s synthesized default page: not used, since this page is assigned (`isDefault: true`). - `RecordPreviewCard`, the card a `referenceVia` pointer pair draws: reached only where a pointer pair names a request row. Whether such rows exist is NOT MEASURED. Where they do, the card shows the step's label where it showed the node id. - Studio's object settings panel shows the list as authored. - Backend readers (`@objectstack/lint` field references and semantic roles, `metadata-core`'s field-security fold, `metadata-protocol`'s build probes): none changes behaviour for a list of existing fields. `current_step_label` is the fifth entry, outside the first four that the field-group shadow rule reads. The page test's build-rule leg stays green. ## The null case: what the chip shows The console reads a request through the approvals get (`routeApprovalRecordReads`). The get serves `current_step_label` by the one chain in `display-labels.ts`: - **A label is stored** (every request opened since the column, and every row the boot backfill filled): the stored label. `openNodeRequest` stores the authored node label, else the humanized node id. - **No label stored, the snapshot carries one** (a row opened before the column that the backfill has not reached): the snapshot's `__nodeLabel`. - **No label stored and none in the snapshot**: the humanized node id (`approve_step` becomes `Approve Step`), never the raw id. - **No step at all** (`current_step` null and no label anywhere): neither field is served. The strip draws no step chip, as it drew no `current_step` chip before. - **The same record read through the generic data API** (a host without the console's routing): the chip reads the stored column. It is null only on a row the boot fill has not labelled, and the chip is then absent where it used to show the raw id. ## The `object_name` chip: measured, the same family, not fixed here - **What it draws.** The get serves `object_name` as stored, the target object's API name. On the booted showcase below it answers `showcase_announcement`. At the pin, `HeaderHighlight` draws a `text` field through the text cell renderer, which prints the stored string. The pointer-pair handling (`resolveRecordPointer`) applies only to the `record_id` id half, and only in the details grid and the list grid. So the Object chip reads `showcase_announcement`. That is a static reading of the renderer plus a wire reading of the value, not a screen reading. - **Why it is not fixed here.** The get also serves `object_label` (`Announcement` on the same request), but `sys_approval_request` declares no field that holds the target object's label (its 23 declared fields were read). A page bound to the object names declared fields only: this page's own pin, "every field the page names exists on its object", holds that. So a label chip needs a new member, which this card's claim excludes. It is reported to the seat with its reach and is not built. ## Tests (at `2ff7f0350`) - `pnpm --filter '@objectstack/plugin-approvals^...' build`: VERDICT command-exit 0. `pnpm --filter @objectstack/plugin-approvals build`: exit 0, at `b62923f9d`. The one commit after it changes only a test file. - `pnpm --filter @objectstack/plugin-approvals test`: `Test Files 81 passed (81)`, `Tests 1111 passed (1111)` (1108 before, plus the 3 below). - `pnpm --filter @objectstack/plugin-approvals typecheck`: exit 0, `check:test-typecheck: OK — … 8 file(s) / 324 error(s) / 27 pinned signature(s) held`. The first spelling of the new test read `.id` off the open result, a union with the auto-outcome, and grew `approval-service.test.ts`'s debt by 7 (`189 type error(s), ledger records 182`). The second commit narrows it with a typed id helper, as the file's other helpers do. - Three new cases, named and green (`--reporter=verbose -t current_step_label`): - `approval-service.test.ts` › the get serves the step label as current_step_label, beside step_label › a labelled row serves its stored label. It also asserts that the list door serves the same three fields as the get. - › a row opened before the column serves the snapshot label, else the humanized machine id. - `sys-approval-request-page.test.ts` › names the step by its label, current_step_label, in the strip and in the object's list alike. - Control in each: `current_step` is still served raw (`approve_step`) and is still a declared field. **Ablation (once, then restored; repeated on the final head).** `node scripts/ablation-replace.mjs --delete` dropped the served line `current_step_label: labels.current_step_label,` from `rowFromRequest`, inside a script with its own `trap` restore. The test imports the service by relative path, so `src/` is what runs and no `dist/` is involved. On-disk evidence: anchor x1 → x0, blob `b137eedb4067` → `2739baafe590`. Expected direction: red. Observed: `Tests 2 failed | 1 passed`. Both get cases failed with `expected { current_step_label: undefined, …(2) } to deeply equal { …(3) }`, and the page pin stayed green (it does not read the get). Restore leg: blob after restore == HEAD (`b137eedb4067`), `git diff HEAD` empty, `git status` clean. No test file is left behind. **Wire reading (booted showcase, this branch's build).** `pnpm dev -- --fresh -p 38241`, signed in as the seeded admin. Retitling a `showcase_announcement` (the `showcase_dynamic_approval` flow's trigger) opened a pending request at the `lead_review` node. `GET /api/v1/approvals/requests/ID` answered `current_step_label: "Lead Review"`, `step_label: "Lead Review"`, `current_step: "lead_review"`, `object_name: "showcase_announcement"`, `object_label: "Announcement"`. `GET /api/v1/approvals/requests` answered the same step fields. The served metadata lists the strip `["process_label","object_name","status","current_step_label","submitter_id","updated_at"]` and the object's `highlightFields` `["process_label","object_name","record_id","status","current_step_label","submitter_id","updated_at"]`. The server was stopped after the reading. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `2ff7f0350` derived 69 commands for this diff, the same 69 the seat derived at `83dfbc046`. All 69 ran locally at `2ff7f0350`, each exit code captured before any pipe. All 69 exited 0, and none printed a PREREQUISITE NOT MET refusal of its own (the CLI and the showcase closure had been built for the wire reading). `node scripts/pm/dispatch-gates.mjs --ran` with the exit-coded record: `69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN` ("a DERIVED zero — all 69 recorded an exit code and none of them is 3"). The longest local runs were `check:query-options-erasure` (235 s), `check:slot-lookup` (123 s) and `check-comment-mask-corpus` (107 s). Not run locally, declared to CI: `test:repo` (the vitest `repo` project; the diff touches neither its tests nor a list naming it), and the repo-wide scans (`pnpm lint` and the rest of the battery). The PR's check-runs are the reading. `check:dev-prereqs` and `check:skill-frame-freshness` are not derived and not owed (no `packages/spec` build input and no frame file changed). ## Acceptance notes - **The Object chip still shows the target object's API name** (section above). It is the same family as this card, and a label needs a declared field the claim excludes. It is reported to the seat, not built. - **objectui's served-page pin copies** (`apps/console/src/services/approvalRequestsDataSource.servedPage*.test.tsx`, 8 files at the pin) copy the strip and the object's list with `current_step`. Their field maps declare no `current_step_label`, and their row fixtures are shaped as the get served a request before this PR, without it. objectstack-ai/objectui#12484 carries them. They are not touched here. - `ApprovalRequestRow.current_step_label`'s TSDoc says the member was "declared ahead of its producer", which this PR is. The sentence names this card as the producer's, so it stays true. It is a `packages/spec` file and is not touched here. - `origin/main` moved one commit past this branch's base (`a2a3777bf`, docs-only, none of the touched paths). The branch is not merged with it. CI's merge ref and the queue read the merged tree. --- _Generated by [Claude Code](https://claude.ai/code/session_01Nk17Z26X5DB59iQztDBADH)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8d3dd6f commit 5da2e88

7 files changed

Lines changed: 99 additions & 9 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/plugin-approvals': patch
3+
---
4+
5+
fix(plugin-approvals): the approval request page's highlights strip names the current step by its label, not the node id
6+
7+
Clause-②: no
8+
9+
- **The approvals get serves `current_step_label`.** `ApprovalRequestRow` declares it as the step's label under the `sys_approval_request` column's own name. Every door that serves a request now serves it with exactly `step_label`'s value: the get, the list, and the request that a decision, recall, send-back, resubmit, reassign, remind, info request or comment echoes back. `step_label` stays, with the same value, for the inbox and the emailed action-link pages. `current_step` stays the Approval node's machine id.
10+
- **The strip's step chip is the step's label.** `sys_approval_request_detail` lists `current_step_label` (the "Approval Step" field) where it listed `current_step`, so a request waiting on the `approve_step` node reads `Manager Review` there instead of `approve_step`. The request lists already showed this label.
11+
- **The object's `highlightFields` makes the same swap.** The page's Details tab is a bare `record:details` node, which draws the object's `highlightFields` minus the strip. A change to the strip alone would have moved `current_step` into that tab.
12+
- **When no label is stored.** The get serves the stored label, else the node label in the request's node-config snapshot, else the humanized node id (`approve_step` → `Approve Step`). A request the boot backfill has not labelled still shows a step chip. A request with no step serves neither field, and the strip draws no step chip.
13+
14+
A patch: one served key that the contract already declares, and one entry in one platform object's field list and in its page. No export, type, accepted value or stored column changes.

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,47 @@ describe('ApprovalService (node era)', () => {
12711271
expect(req.step_label).toBe('Approve Step'); // from `approve_step`
12721272
});
12731273

1274+
// [#24281] The record page's step chip lists `current_step_label`, a declared
1275+
// field of `sys_approval_request`, and the console reads the page's request
1276+
// through this get. So the get serves the step label under that name too,
1277+
// with exactly `step_label`'s value, while `current_step` stays the raw id.
1278+
describe('the get serves the step label as current_step_label, beside step_label', () => {
1279+
/** The id of the request an ordinary open answers (never an auto-outcome here). */
1280+
const openId = async (input: ReturnType<typeof openInput>): Promise<string> =>
1281+
((await svc.openNodeRequest(input, CTX)) as { id: string }).id;
1282+
const stored = (id: string) => engine._tables['sys_approval_request'].find((r) => r.id === id)!;
1283+
const stepFields = (row: any) => ({
1284+
current_step_label: row?.current_step_label,
1285+
step_label: row?.step_label,
1286+
current_step: row?.current_step,
1287+
});
1288+
1289+
it('a labelled row serves its stored label', async () => {
1290+
const id = await openId(openInput(['u9'], { nodeLabel: 'Manager Review' }));
1291+
expect(stored(id).current_step_label).toBe('Manager Review');
1292+
expect(stepFields(await svc.getRequest(id, SYS))).toEqual({
1293+
current_step_label: 'Manager Review', step_label: 'Manager Review', current_step: 'approve_step',
1294+
});
1295+
// The list door builds its rows the same way.
1296+
const [listed] = await svc.listRequests({ status: 'pending' }, SYS);
1297+
expect(stepFields(listed)).toEqual(stepFields(await svc.getRequest(id, SYS)));
1298+
});
1299+
1300+
it('a row opened before the column serves the snapshot label, else the humanized machine id', async () => {
1301+
const snap = await openId(openInput(['u9'], { nodeLabel: 'Manager Review' }));
1302+
const bare = await openId(openInput(['u9'], { recordId: 'opp2' }));
1303+
// No stored label: the row the boot backfill has not reached.
1304+
stored(snap).current_step_label = null;
1305+
stored(bare).current_step_label = null;
1306+
expect(stepFields(await svc.getRequest(snap, SYS))).toEqual({
1307+
current_step_label: 'Manager Review', step_label: 'Manager Review', current_step: 'approve_step',
1308+
});
1309+
expect(stepFields(await svc.getRequest(bare, SYS))).toEqual({
1310+
current_step_label: 'Approve Step', step_label: 'Approve Step', current_step: 'approve_step',
1311+
});
1312+
});
1313+
});
1314+
12741315
it('listRequests enriches record_title and submitter_name', async () => {
12751316
engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }];
12761317
engine._tables['sys_user'] = [{ id: 'u1', name: 'Ada Lovelace', email: 'ada@example.com' }];

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,11 @@ function rowFromRequest(row: any): ApprovalRequestRow {
11811181
submitted_at: row.created_at ?? undefined,
11821182
process_label: labels.process_label,
11831183
step_label: labels.current_step_label,
1184+
// [#24281] The same step label under the column's own name: a page bound
1185+
// to `sys_approval_request` names the object's declared fields, and
1186+
// `step_label` is not one, so the record page's step chip lists this.
1187+
// `step_label` stays for the inbox and the action-link pages.
1188+
current_step_label: labels.current_step_label,
11841189
sla_due_at: slaDueAt(row.created_at, cfg),
11851190
// ADR-0044 revision round (rides the config snapshot; absent ⇒ round 1).
11861191
round: typeof cfg?.__round === 'number' ? cfg.__round : undefined,
@@ -3519,7 +3524,8 @@ export class ApprovalService implements IApprovalService {
35193524
const processName = `flow:${input.flowName ?? input.nodeId}`;
35203525
// Display labels also ride the config snapshot, where rows opened before
35213526
// the `process_label` / `current_step_label` columns (#23889) carry them;
3522-
// `rowFromRequest` surfaces them as `process_label` / `step_label`.
3527+
// `rowFromRequest` surfaces them as `process_label` and as `step_label` /
3528+
// `current_step_label`.
35233529
const configSnapshot: any = { ...input.config };
35243530
if (input.flowLabel) configSnapshot.__flowLabel = input.flowLabel;
35253531
if (input.nodeLabel) configSnapshot.__nodeLabel = input.nodeLabel;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
*
1010
* - `ApprovalService.openNodeRequest` writes `process_label` /
1111
* `current_step_label` with it at open;
12-
* - `rowFromRequest` serves `process_label` / `step_label` with it on the
13-
* approvals door;
12+
* - `rowFromRequest` serves `process_label` and the step's label with it on
13+
* the approvals door, the step's under two names of one value: `step_label`
14+
* and the column's own `current_step_label`;
1415
* - the boot backfill (`display-label-backfill.ts`) fills the two columns
1516
* with it on a row written before they existed, and the action's
1617
* `step_label` from the filled request.

‎packages/plugins/plugin-approvals/src/sys-approval-request-page.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,24 @@ describe('sys_approval_request detail page — the slot map', () => {
165165
expect(declared.filter((f: string) => !strip.includes(f))).toEqual(['record_id']);
166166
});
167167

168+
it('names the step by its label, current_step_label, in the strip and in the object\'s list alike', () => {
169+
// `current_step` is the Approval node's machine id (`approve_step`);
170+
// `current_step_label` is the step's label, a declared field the approvals
171+
// get serves under that name (`approval-service.test.ts` pins the get). The
172+
// object's list moves with the strip for the reason the flow chip's does.
173+
const strip = SLOTS.highlights.properties.fields as string[];
174+
const declared = SysApprovalRequest.highlightFields ?? [];
175+
const fields = SysApprovalRequest.fields as AnyRec;
176+
expect({ strip: strip.includes('current_step_label'), declared: declared.includes('current_step_label') })
177+
.toEqual({ strip: true, declared: true });
178+
expect({ strip: strip.includes('current_step'), declared: declared.includes('current_step') })
179+
.toEqual({ strip: false, declared: false });
180+
expect(fields.current_step_label).toMatchObject({ type: 'text', label: 'Approval Step' });
181+
// Control: the machine id stays a declared field. It is dropped from the
182+
// two lists, not from the object.
183+
expect(fields.current_step).toMatchObject({ type: 'text' });
184+
});
185+
168186
it('tabs: Details is record:details, Timeline is sys_approval_action as a related list on request_id', () => {
169187
expect(TAB_ITEMS.map((t) => t.label?.en)).toEqual(['Details', 'Timeline']);
170188
expect(TAB_ITEMS[0].children).toEqual([{ type: 'record:details' }]);

‎packages/plugins/plugin-approvals/src/sys-approval-request.object.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,13 @@ export const SysApprovalRequest = ObjectSchema.create({
6262
nameField: 'display_title', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField)
6363
titleFormat: '{process_name} · {record_id}',
6464
// [ADR-0085] The flow is named by its label, `process_label`, never by the
65-
// machine source `process_name` (`flow:NAME`), as the request lists name it.
66-
// The record page's highlights strip is this list minus `record_id`, and the
67-
// page's bare `record:details` node draws this list minus the strip, so the
68-
// two move together (`sys-approval-request.page.ts` says why).
69-
highlightFields: ['process_label', 'object_name', 'record_id', 'status', 'current_step', 'submitter_id', 'updated_at'],
65+
// machine source `process_name` (`flow:NAME`), and the step by its label,
66+
// `current_step_label`, never by the node id `current_step`, as the request
67+
// lists name them. The record page's highlights strip is this list minus
68+
// `record_id`, and the page's bare `record:details` node draws this list
69+
// minus the strip, so the two move together (`sys-approval-request.page.ts`
70+
// says why).
71+
highlightFields: ['process_label', 'object_name', 'record_id', 'status', 'current_step_label', 'submitter_id', 'updated_at'],
7072

7173
// Curated built-in list views — render as segmented tabs in the console.
7274
// Filters use {current_user_id} substitution wired by the console.

‎packages/plugins/plugin-approvals/src/sys-approval-request.page.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ import type { Page } from '@objectstack/spec/ui';
4444
* snapshot's, else the humanized machine name. Where the chain yields
4545
* nothing, the field has no value, the strip draws no Flow chip, and the
4646
* record title still names `process_name`.
47+
* - The step chip is `current_step_label` (the "Approval Step" field), not
48+
* `current_step` (the Approval node's machine id, such as `approve_step`).
49+
* The approvals get serves it by the same chain: the stored label, else
50+
* the snapshot's `__nodeLabel`, else the humanized node id. A request with
51+
* no step at all serves neither field, and the strip draws no step chip.
52+
* - The Object chip is `object_name`, the target object's API name as
53+
* stored. The object declares no field that holds that object's label, so
54+
* the strip has none to list in its place.
4755
* - `tabs` is authored: Details (`record:details`, a bare node: see the
4856
* strip above) and Timeline — `sys_approval_action` as a
4957
* related list keyed on `request_id`, newest first. The `tabs` slot is
@@ -80,7 +88,7 @@ export const SysApprovalRequestDetailPage: Page = {
8088
highlights: {
8189
type: 'record:highlights',
8290
properties: {
83-
fields: ['process_label', 'object_name', 'status', 'current_step', 'submitter_id', 'updated_at'],
91+
fields: ['process_label', 'object_name', 'status', 'current_step_label', 'submitter_id', 'updated_at'],
8492
},
8593
},
8694

0 commit comments

Comments
 (0)