Repository navigation
Commit ba69b16
fix(plugin-approvals): serve the approval flow, step and object labels in the reader's locale (#24977)
Fixes #24863
Clause-②: no
This is the services half of #24863 (triage split R539): the request
labels and the inbox row's object label. The `approval.requested`
notification half is NOT in this PR. It needs a decision, set out in
**Open questions** below, so the card stays open (`Part of`, not
`Fixes`).
## What changes
The approvals door now serves a request's flow, step and object labels
in the reader's locale, picked when the row is read.
- **Doors.** `GET /api/v1/approvals/requests` (the inbox and the
record's approval panel), `GET /api/v1/approvals/requests/:id`, and the
request a decision answers with.
- **Fields.** `process_label`, `step_label` / `current_step_label` (two
names of one value, which move together) and `object_label`. The get's
step progress (`flow_steps`) names each step the same way, so the strip
and `current_step_label` agree.
- **Keys.** `flows.FLOW.label`, `flows.FLOW.approvals.NODE.label` (the
key PR #24943 added) and `objects.OBJECT.label` (`objectLabelKey`).
- **Fallback.** The stored snapshot, whenever nothing is picked: a key
the pack lacks, a locale no pack holds, a read with no locale, no `i18n`
service, or a service that throws.
- **Unchanged.**
- The stored `process_label` / `current_step_label` columns. No column
is added, rewritten or migrated.
- The generic data door and the list views, which still serve the stored
text.
- Who may read a request.
- The notification bodies.
### Files
- `packages/plugins/plugin-approvals/src/reader-locale-labels.ts` (new):
the keys, the pick and the overlay.
- `approval-service.ts`: the new option
`ApprovalServiceOptions.i18nServiceSource`, and `readerLabels(context)`.
`listRequests` and `loadRequest` apply it after `enrichRows`, and
`attachFlowSteps` takes the same labels.
- `approvals-plugin.ts`: one lazy `i18n` reader feeds both
`messageTranslator` (unchanged behaviour) and `i18nServiceSource`.
- `display-labels.ts` and the open-time snapshot comment: the docblocks
now say the chain answers the authored label and the door overlays the
translation.
- `.changeset/24863-approval-labels-reader-locale.md`:
`@objectstack/plugin-approvals` patch.
## Mechanism (measured; reused, not a second one)
Ruling 6093472708 on #22507 says a server-rendered user string is
translated where it is rendered. I measured how this repo's server
already renders a user-read string in the caller's language. The pattern
is the same in three places:
- objectql `objectDisplayLabel` (`objectLabelKey`);
- `packages/rest/src/not-null-refusal-label.ts`;
- the automation engine's `renderFlowTextSlot`.
All three take `ExecutionContext.locale`, negotiate it against the i18n
service's `getLocales()` with `resolveBundleLocale`, call
`II18nService.t(key, locale)` (a miss echoes the key), and fall back to
the authored text. This PR uses the same sequence. The service is read
lazily from the kernel (`ctx.getService('i18n')`), the same read the
plugin's existing `messageTranslator` already makes.
## The mechanism assumptions, measured on `origin/main` `693352e4c2`
- **H1 holds.** Every door served the stored row with no locale step.
- `rowFromRequest` (`approval-service.ts:1203`, labels at
`:1233`–`:1239`) builds the labels from `requestDisplayLabels` (the
stored column, then the config snapshot, then the machine name).
- It serves `listRequests` (`:8109`, rows at `:8160`) and `loadRequest`
(`:8274`). Those feed `getRequest` (`:8247`) and `readBackRequest`
(`:8232`, a decision's echo).
- `object_label` was `objectLabelOf` (`:7374`), the schema label, set at
`:7513`.
- `attachFlowSteps` (`:8391`/`:8403`) used the live node's authored
label.
- REST: `packages/rest/src/approvals-routes.ts:214` (list) and `:310`
(get).
- Two more doors serve the labels and are deliberately left as they are
(see Acceptance notes): the generic data door's list views
(`sys-approval-request.object.ts:130`–`:163`) and the token action page
(`action-link-pages.ts:65`–`:68`).
- **H2 holds.** The per-request locale is `ExecutionContext.locale`. The
shared assembler sets it to `requestLocale ?? localization?.locale`
(`packages/core/src/security/assemble-execution-context.ts:388`), and
the REST approvals routes pass that context (`resolveExecCtx`). The
`i18n` service is reachable from `plugin-approvals` through
`ctx.getService('i18n')`, already used at `approvals-plugin.ts:252` for
refusal copy.
- **H3 holds.** The stored row carries both key parts. `process_name` is
`flow:` plus `Flow.name` (the engine's `$flowName`,
`approval-node.ts:180`, `service-automation` `engine.ts:13331`), and
`current_step` is the node id (`approval-service.ts:3750`). No flow
definition is read. A process name without the `flow:` prefix names no
flow, so its labels stay as stored.
## Tests
All runs are on head `2335508393` (the merge of `origin/main`
`6440cf1633`), under `os-verify-lock.sh`.
- **New: `reader-locale-labels.test.ts`** (unit, 15 cases).
- Each key the reader asks is rebuilt into the nested pack shape and
parsed by `TranslationDataSchema`, with a control that a step leaf the
face does not declare is refused.
- A bare `zh` is negotiated to `zh-CN`, also against an exact-match
service.
- Picks nothing on a miss, an empty entry, a blank entry, an unheld
locale or a throwing service. A read with no locale or no service is no
pick at all.
- One `t()` per key per read.
- The overlay moves both step names together and keeps unpicked labels.
- **New: `reader-locale-display-labels.integration.test.ts`** (10
cases). The rig is a real `ObjectQL` over better-sqlite3, the real
`AutomationEngine` running a two-step flow (so the real approval node
opens the request), the real `ApprovalsServicePlugin.start` over a
kernel whose `i18n` is `createMemoryI18n` loaded with a zh-CN pack, and
the pack parsed by `TranslationDataSchema` first.
- A zh-CN reader gets zh-CN labels on the record panel list, the inbox
list, the get and its `flow_steps`, and a decision's echo.
- A bare `zh` reader reaches the zh-CN pack.
- A step the pack lacks is served as stored, beside the translated flow
label.
- A ja-JP reader with no pack gets the stored snapshot.
- An en reader and a reader with no locale are unchanged.
- The stored row keeps `Account Approval` / `Manager Review` after the
reads.
- **Package.**
- `pnpm --filter @objectstack/plugin-approvals exec vitest run
--maxWorkers=2`: `Test Files 92 passed (92)`, `Tests 1203 passed
(1203)`.
- `pnpm --filter @objectstack/plugin-approvals typecheck`: exit 0. It
includes `check:test-typecheck: OK`, so the new test files compile with
no new debt.
- `pnpm --filter @objectstack/plugin-approvals build`: exit 0.
- Closure: `turbo run build --filter=@objectstack/plugin-approvals^...
--concurrency=2`, 32 of 32 successful.
- **Ablation**, one leg per changed behaviour, each through
`scripts/ablation-replace.mjs` in WRAP mode with a trap restore. The new
tests import the subject by relative path, so they read `src/` and no
build is needed for either leg.
- Leg 1 makes `readerLabels` return nothing (on disk: anchor 1→0, marker
0→1). Result: `Tests 6 failed | 19 passed (25)`. Exactly the six zh-CN
pins went red; every as-stored pin and the unit file stayed green.
- Leg 2 makes `attachFlowSteps` name steps by the authored label only.
Result: `1 failed | 9 passed (10)`, and the one red was the
step-progress pin.
- Restore after each leg: `git checkout HEAD -- PATH`. The blob after
restore equals HEAD (`06899aa721b5`), `git diff HEAD` is empty, and `git
status --porcelain` is clean.
- The direction was predicted in the test docblock before the run, and
it held.
## Gates
Local subset, at head `2335508393`, under the lock. Each exit code was
captured before any pipe.
- **Derivation.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 70 families (vs merge base
`6440cf163`, 7 paths). All 70 were run.
- **Results.** 69 exited 0. One exited 3:
- `pnpm check:dual-build-cjs-loads` answered `PREREQUISITE NOT MET`. It
reads every package's `dist/`, and 38 packages are unbuilt in this tree.
**NOT MEASURED** locally; CI runs it in `Lint & Repo Gates`.
- **`pnpm check:i18n`.** It first exited 3 (the CLI was not built).
After the prerequisite closure the gate names (`turbo run build`, 61 of
61 tasks), it exited 0: `check-i18n-bundles: OK (10 package(s) — all
bundles in sync, no undeclared authoring keys)`.
- **`--ran` reconciliation:** `70 derived famil(ies) accounted for — 69
run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)`.
- **Left to CI:**
- `test:repo`: the diff touches none of its files, so it runs in CI's
`Test Core`.
- `pnpm lint` and the whole-repo farm.
- CI check-runs had not concluded when this PR opened.
## Open questions (decision frame: real business need, long-term
soundness, preventing AI mistakes, no scope creep at the startup stage)
### 1. The `approval.requested` notification body (and its siblings)
**Measured.**
- The body is built at open (`approval-service.ts:3890`–`:3901`): title
`Approval requested`, body ``A decision on ${subject} is waiting on
you.``, where `subject` is `nameSubjectRecord` (the schema label plus
the record id).
- It is emitted once per approver, under the SUBMITTER's request
context, so the only locale in hand is the submitter's.
- The locale it should use is the recipient's. Ruling #13881, as encoded
in `service-messaging/src/recipient-locale.ts` (the stated ONE read
point): the recipient's `sys_user.locale`, then the deployment default,
resolved after fan-out.
- Nothing localizes the sentence today. The sentence has no catalog
entry: the Operation Message Catalog (`@objectstack/spec/system`) holds
no notification sentences, and the plugin's packs hold object
translations only.
- So localizing it needs a NEW translation key in `packages/spec` (or a
new plugin catalog). This dispatch forbids both.
- The same holds for `approval.reminder`, `approval.sla_breached`,
`approval.approved` / `approval.rejected` and the OOO notices.
**Options.**
- **A. Keyed text, rendered per recipient at delivery.** Approvals hands
messaging a catalog key and params. Each channel renders the text in the
recipient's locale through `recipient-locale.ts` and the shared catalog,
which gains notification entries in en / zh-CN / ja-JP / es-ES.
- Business need: real (HotCRM measured it; every non-English approver
gets the English body).
- Long-term: best. One locale chain and one catalog serve every
producer.
- AI mistakes: hardest to get wrong. A producer cannot hand-roll a
sentence, and a key's existence is checkable.
- Startup scope: widest. Three lanes (spec catalog, the messaging
contract and channels, approvals). Each is a card.
- **B. Rendered at emit, per approver, in plugin-approvals** (the
fan-out is already one emit per approver). The text comes from new
entries in the shared Operation Message Catalog. The recipient's locale
comes from messaging (a small read it exposes), so the chain keeps one
read point.
- Business need: real.
- Long-term: good. Every approval topic reuses one helper, but each
other producer would repeat it.
- AI mistakes: medium. Per-producer rendering is a pattern to copy.
- Startup scope: two lanes (spec catalog entries, approvals) plus one
small messaging read.
- **C. Keep the English bodies and record the gap.**
- Business need: leaves a measured defect.
- Startup scope: zero cost.
**Recommendation: B.** It fixes the measured defect at the smallest cost
that keeps one catalog and one locale read point. A stays the target if
a second non-approval producer asks for localized notifications.
### 2. The generic data door's list views
`Setup → Approvals → Requests` and its siblings are list views over
`sys_approval_request`. They serve the stored columns by the #23889
contract, so a zh-CN reader still sees the authored labels there.
- **A. Leave them stored.** These are admin lists, the column is the
audit record, and `?q=` search matches the stored text.
- **B. Add a read-time overlay hook on the data door.** Filter and sort
would then disagree with the cells, and the authored text would become
unreachable through the door.
- **C. Translate in the console from the bundle.** That is a client
overlay on a served value.
**Recommendation: A**, until a measured user need appears. HotCRM
measured the inbox and the record panel, which this PR fixes.
## Acceptance notes
- **Not changed, by design.**
- The token action page (`action-link-pages.ts`, bilingual en/zh copy)
has no reader locale: the bearer has a token, not a session, and its
context is the system's.
- `?q=` search matches the stored labels.
- **The `packages/spec` follow-ups this reader makes due.** They are
spec-lane edits, outside this dispatch.
- (1) Liveness row `packages/spec/liveness/translation.json` →
`flows.approvals` (and `.label`) says "Flip to `live` on the commit that
cites that reader". This PR is that reader.
- (2) The contract docblocks on `ApprovalRequestRow.process_label` /
`current_step_label` / `object_label`
(`packages/spec/src/contracts/approval-service.ts:220`–`:243`) describe
the authored/schema label. They now hold only for a reader with no
translation.
- (3) PR #24943 suggested a `flowApprovalLabelKey` helper next to
`flowRefusalMessageKey`. This PR spells the two flow keys locally in
`reader-locale-labels.ts`, held to `TranslationDataSchema` by a test. A
spec helper would let the reader import them.
- **The CLI half (#24916).** The extractor asking apps to translate
approval flows and steps is #24916's. With this reader landed, that ask
is no longer a declared-but-not-honoured demand.
- **The console half.** objectstack-ai/objectui#13248 (the history row
draws `step_name`) will read `sys_approval_action.step_label`, a stored
snapshot served through the data door. It will show the authored step
label, not the reader's. That is a carrier note for objectui#13248, not
filed.
- **Negotiation glue.** The few lines that negotiate against
`getLocales()` now have four consumer copies (objectql, rest,
automation, here) around the one rule. This is an observation, not
filed.
> **Seat's append** (`domain:services` seat 1,
`session_011TdZ8GjixNZFLz1xJD9zzK`):
> - **First line changed** from `Part of #24863` to `Fixes #24863`, by
seat ruling `6300710414` on #24863. The notification half moved to
#24978, and the spec liveness / docblock follow-up is #24979.
> - **Open question 2:** ruled A, so the admin lists stay stored.
> - Where the sections above say this PR is `Part of` or the
notification half is open here, this append supersedes them.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b811c73 commit ba69b16
7 files changed
Lines changed: 779 additions & 12 deletions
File tree
- .changeset
- packages/plugins/plugin-approvals/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
93 | 93 | | |
94 | 94 | | |
95 | 95 | | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
96 | 102 | | |
97 | 103 | | |
98 | 104 | | |
| |||
1429 | 1435 | | |
1430 | 1436 | | |
1431 | 1437 | | |
| 1438 | + | |
| 1439 | + | |
| 1440 | + | |
| 1441 | + | |
| 1442 | + | |
| 1443 | + | |
| 1444 | + | |
| 1445 | + | |
| 1446 | + | |
| 1447 | + | |
| 1448 | + | |
1432 | 1449 | | |
1433 | 1450 | | |
1434 | 1451 | | |
| |||
1528 | 1545 | | |
1529 | 1546 | | |
1530 | 1547 | | |
| 1548 | + | |
| 1549 | + | |
1531 | 1550 | | |
1532 | 1551 | | |
1533 | 1552 | | |
| |||
1556 | 1575 | | |
1557 | 1576 | | |
1558 | 1577 | | |
| 1578 | + | |
1559 | 1579 | | |
1560 | 1580 | | |
1561 | 1581 | | |
| |||
3746 | 3766 | | |
3747 | 3767 | | |
3748 | 3768 | | |
| 3769 | + | |
| 3770 | + | |
| 3771 | + | |
3749 | 3772 | | |
3750 | 3773 | | |
3751 | 3774 | | |
| |||
8164 | 8187 | | |
8165 | 8188 | | |
8166 | 8189 | | |
| 8190 | + | |
| 8191 | + | |
8167 | 8192 | | |
8168 | 8193 | | |
8169 | 8194 | | |
8170 | 8195 | | |
| 8196 | + | |
| 8197 | + | |
| 8198 | + | |
| 8199 | + | |
| 8200 | + | |
| 8201 | + | |
| 8202 | + | |
| 8203 | + | |
| 8204 | + | |
| 8205 | + | |
| 8206 | + | |
| 8207 | + | |
| 8208 | + | |
| 8209 | + | |
| 8210 | + | |
| 8211 | + | |
| 8212 | + | |
| 8213 | + | |
| 8214 | + | |
| 8215 | + | |
| 8216 | + | |
| 8217 | + | |
| 8218 | + | |
| 8219 | + | |
| 8220 | + | |
| 8221 | + | |
8171 | 8222 | | |
8172 | 8223 | | |
8173 | 8224 | | |
| |||
8278 | 8329 | | |
8279 | 8330 | | |
8280 | 8331 | | |
8281 | | - | |
| 8332 | + | |
| 8333 | + | |
| 8334 | + | |
8282 | 8335 | | |
8283 | 8336 | | |
8284 | 8337 | | |
| |||
8387 | 8440 | | |
8388 | 8441 | | |
8389 | 8442 | | |
| 8443 | + | |
| 8444 | + | |
| 8445 | + | |
| 8446 | + | |
| 8447 | + | |
8390 | 8448 | | |
8391 | | - | |
| 8449 | + | |
8392 | 8450 | | |
8393 | 8451 | | |
8394 | 8452 | | |
| |||
8400 | 8458 | | |
8401 | 8459 | | |
8402 | 8460 | | |
8403 | | - | |
| 8461 | + | |
| 8462 | + | |
| 8463 | + | |
8404 | 8464 | | |
8405 | 8465 | | |
8406 | 8466 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
216 | 216 | | |
217 | 217 | | |
218 | 218 | | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
219 | 233 | | |
220 | 234 | | |
221 | 235 | | |
| |||
248 | 262 | | |
249 | 263 | | |
250 | 264 | | |
251 | | - | |
252 | | - | |
253 | | - | |
254 | | - | |
255 | | - | |
256 | | - | |
257 | | - | |
258 | | - | |
259 | | - | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
260 | 269 | | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
261 | 274 | | |
262 | 275 | | |
263 | 276 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
26 | 34 | | |
27 | 35 | | |
28 | 36 | | |
| |||
0 commit comments