From ebcd41acb76c935467ffe8298dc2753b3efde23a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:10:36 +0000 Subject: [PATCH 1/7] fix(plugin-approvals,plugin-security): the drawer's Approve names the approved request, and explain names the principal as a person The approve action's success copy interpolates the decide route's read-back request title, in the inbox's inline Approve wording, with the four locale values updated in step. The explain engine's principal layer names the principal (and an on-behalf-of delegator) by name and email with the id secondary, read through the asker's own door so nothing the asker cannot read is disclosed. Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- .../src/sys-approval-request.object.ts | 8 ++- .../src/translations/en.objects.generated.ts | 2 +- .../translations/es-ES.objects.generated.ts | 2 +- .../translations/ja-JP.objects.generated.ts | 2 +- .../translations/zh-CN.objects.generated.ts | 2 +- .../plugin-security/src/explain-engine.ts | 49 +++++++++++++++++-- 6 files changed, 56 insertions(+), 9 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 6d5c4695784..423d561dfaf 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -471,7 +471,13 @@ export const SysApprovalRequest = ObjectSchema.create({ 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true' + ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true', locations: ['record_section', 'list_item'], - successMessage: 'Approved.', + // Names what was approved, in the inbox's inline Approve wording. The + // decide route answers `ApprovalDecisionResult`, whose `request` is the + // read-back row with its display enrichment, so `record_title` is the + // same title the inbox row shows (the target record's display field, + // else its payload snapshot). `${result.*}` is the success copy's + // declared scope on a `type: 'api'` action: the route's own response. + successMessage: 'Approved "${result.request.record_title}"', refreshAfter: true, }, { diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index ba6c21856ee..5bdb49aa25a 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -125,7 +125,7 @@ export const enObjects: NonNullable = { approval_approve: { label: "Approve", description: "Approve this request? Your approval is recorded, and the request moves on once this step has the approvals it requires.", - successMessage: "Approved.", + successMessage: "Approved \"${result.request.record_title}\"", params: { comment: { label: "Comment" diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 15a40853b7d..0d8018071d5 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -125,7 +125,7 @@ export const esESObjects: NonNullable = { approval_approve: { label: "Aprobar", description: "¿Aprobar esta solicitud? Su aprobación queda registrada y la solicitud avanza cuando este paso reúne las aprobaciones que requiere.", - successMessage: "Aprobada.", + successMessage: "\"${result.request.record_title}\" aprobada", params: { comment: { label: "Comentario" diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index 7dbef464625..1aa2e08b530 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -125,7 +125,7 @@ export const jaJPObjects: NonNullable = { approval_approve: { label: "承認", description: "このリクエストを承認しますか？あなたの承認が記録され、このステップに必要な承認がそろうとリクエストは次に進みます。", - successMessage: "承認しました。", + successMessage: "「${result.request.record_title}」を承認しました", params: { comment: { label: "コメント" diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index 32a85bc9d2e..4458a309d6f 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -125,7 +125,7 @@ export const zhCNObjects: NonNullable = { approval_approve: { label: "通过", description: "通过该请求？你的审批将被记录，此步骤获得所需的审批后，请求即继续流转。", - successMessage: "已通过。", + successMessage: "已通过“${result.request.record_title}”", params: { comment: { label: "审批意见" diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index c22a124bc6d..5499d10fec9 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -1990,6 +1990,37 @@ function attributeThePrincipalsRead(layers: ExplainLayer[], read: ExplainDecisio } } +/** + * [#23157] The person a user id names, for the principal layer's prose: the + * display name and email first and the id secondary — `Ada Lovelace + * (ada@example.com; id usr_1)` — the way the explain panel's own Principal + * line shows that person. + * + * The user row is read through the ASKER's door: `ql.findOne` under the + * asker's context, every data middleware included, so the sentence carries + * only what the asker could read of that user anyway, row-level and + * field-level security both. An asker the read returns no row to, or a row + * with neither a name nor an email, gets the bare id the sentence always + * carried, and nothing more. So does a read that cannot run or that throws: + * the label is presentation, never an input to a verdict, and the id it falls + * back to is the fact the context already states. + */ +async function personOf(ql: any, askerContext: any, userId: string): Promise { + if (!userId || typeof ql?.findOne !== 'function') return userId; + let row: any; + try { + row = await ql.findOne('sys_user', { where: { id: userId }, context: askerContext }); + } catch { + return userId; + } + const text = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); + const name = text(row?.name); + const email = text(row?.email); + if (name && email && name !== email) return `${name} (${email}; id ${userId})`; + if (name || email) return `${name || email} (id ${userId})`; + return userId; +} + async function explainOnce(deps: ExplainEngineDeps, input: ExplainInput, asker: ExplainAsker | undefined): Promise { const { object, operation, context } = input; const engineOp = EXPLAIN_TO_ENGINE_OP[operation]; @@ -2054,18 +2085,28 @@ async function explainOnce(deps: ExplainEngineDeps, input: ExplainInput, asker: Array.isArray(context?.delegatedPositions) ? context.delegatedPositions : []; const delegationOf = (name: string): { from: string; until?: string } | undefined => delegatedPositions.find((d) => d.name === name); + // [#23157] The principal and its delegator are named as people, read + // through the asker's door ({@link personOf}); a self-explanation's asker is + // the explained principal. + const askerContext = asker?.context ?? context; + const principalPerson = context?.userId != null + ? await personOf(deps.ql, askerContext, String(context.userId)) + : '(anonymous)'; + const delegatorPerson = context?.onBehalfOf?.userId + ? await personOf(deps.ql, askerContext, String(context.onBehalfOf.userId)) + : ''; layers.push({ layer: 'principal', verdict: delegatorMissing || delegatorUnresolved ? 'denies' : 'neutral', detail: - `Principal ${context?.userId ?? '(anonymous)'} holds position(s) [${positions.join(', ') || 'none'}] ` + + `Principal ${principalPerson} holds position(s) [${positions.join(', ') || 'none'}] ` + `resolving to permission set(s) [${setNames.join(', ') || 'none'}] (union-merged, most-permissive).` + (context?.onBehalfOf?.userId ? delegatorMissing - ? ` Acting on behalf of ${context.onBehalfOf.userId}, who no longer exists — D10 fails CLOSED (access denied).` + ? ` Acting on behalf of ${delegatorPerson}, who no longer exists — D10 fails CLOSED (access denied).` : delegatorUnresolved - ? ` Acting on behalf of ${context.onBehalfOf.userId}, whose grants could not be resolved (the lookup threw) — D10 fails CLOSED (access denied).` - : ` Acting on behalf of ${context.onBehalfOf.userId} — effective access is the D10 INTERSECTION with the delegator's set(s) [${delegatorNames.join(', ') || 'none'}].` + ? ` Acting on behalf of ${delegatorPerson}, whose grants could not be resolved (the lookup threw) — D10 fails CLOSED (access denied).` + : ` Acting on behalf of ${delegatorPerson} — effective access is the D10 INTERSECTION with the delegator's set(s) [${delegatorNames.join(', ') || 'none'}].` : '') + (delegatedPositions.length > 0 ? ` ${delegatedPositions.length} position(s) held via delegation (ADR-0091 D3): [${delegatedPositions From 3adb7e0fab26a73b83e868de059e3432c364b83f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:20:19 +0000 Subject: [PATCH 2/7] test(plugin-approvals,plugin-security): pin the named approve toast and the principal named as far as the asker reads them Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- .../src/approval-service.test.ts | 57 ++++ .../approve-success-copy-i18n.test.ts | 54 ++++ .../src/explain-asker-gate.test.ts | 20 +- .../src/explain-principal-person.test.ts | 273 ++++++++++++++++++ 4 files changed, 401 insertions(+), 3 deletions(-) create mode 100644 packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts create mode 100644 packages/plugins/plugin-security/src/explain-principal-person.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 87528cf3113..70ecd090468 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1288,6 +1288,63 @@ describe('ApprovalService (node era)', () => { expect(rows[0].record_title).toBe('Snapshot Title'); }); + // [#23157] The approve action is `type: 'api'` against the decide route, + // which answers decide()'s result verbatim; its success copy interpolates + // `${result.*}` over that answer (the scope `ActionSchema.successMessage` + // declares). So the toast names what was approved only while decide()'s + // answer carries the title the copy reads, finalized or not. + describe('the approve action\'s success copy, filled from decide()\'s answer', () => { + const approveCopy = (): string => { + const action = (SysApprovalRequest as any).actions.find((a: any) => a.name === 'approval_approve'); + return action.successMessage; + }; + /** The copy's `${result.}` tokens, each filled from the answer; an unresolved path reads as ''. */ + const fill = (copy: string, result: unknown): { text: string; unresolved: string[] } => { + const unresolved: string[] = []; + const text = copy.replace(/\$\{result\.([\w.]+)\}/g, (_m, path: string) => { + const v = path.split('.').reduce((acc, k) => (acc == null ? undefined : acc[k]), result); + if (v == null || String(v) === '') unresolved.push(path); + return v == null ? '' : String(v); + }); + return { text, unresolved }; + }; + + it('reads the answer: the copy carries at least one `${result.*}` token', () => { + expect(approveCopy()).toMatch(/\$\{result\.[\w.]+\}/); + }); + + it('a finalizing approve: the toast names the record, the way the inbox row\'s inline Approve does', async () => { + engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; + const req = await svc.openNodeRequest(openInput(['u9']), CTX); + const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + expect(out.finalized).toBe(true); + const { text, unresolved } = fill(approveCopy(), out); + expect(unresolved).toEqual([]); + expect(text).toBe('Approved "Acme Renewal"'); + }); + + it('an approve that leaves the step waiting on others still names the record', async () => { + engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; + const req = await svc.openNodeRequest(openInput(['u1', 'u2'], {}, { behavior: 'unanimous' }), CTX); + const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u1' }, SYS); + expect(out.finalized).toBe(false); + const { text, unresolved } = fill(approveCopy(), out); + expect(unresolved).toEqual([]); + expect(text).toContain('Acme Renewal'); + expect(text).not.toBe('Approved.'); + }); + + it('a record whose row is gone is named from the payload snapshot, as the inbox names it', async () => { + const req = await svc.openNodeRequest( + openInput(['u9'], { record: { id: 'opp1', name: 'Snapshot Title', amount: 1 } }), CTX, + ); + const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + const { text, unresolved } = fill(approveCopy(), out); + expect(unresolved).toEqual([]); + expect(text).toBe('Approved "Snapshot Title"'); + }); + }); + it('enrichment resolves lookup foreign keys in the payload to record titles', async () => { (engine as any).getSchema = (name: string) => name === 'opportunity' diff --git a/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts b/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts new file mode 100644 index 00000000000..243b6c022e3 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts @@ -0,0 +1,54 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// The approve action's success toast names what was approved (#23157), and +// every shipped locale says so too. +// +// The copy is an `I18nLabel` the console resolves from these bundles BEFORE it +// fills the `${result.*}` tokens, so a locale whose value drops the token +// toasts the bare verb in that language while English names the record. No +// drift gate sees that: `check:i18n` reads a translated-locale value as +// hand-written text and never compares its tokens with the source's. + +import { describe, it, expect } from 'vitest'; +import { SysApprovalRequest } from '../sys-approval-request.object.js'; +import { enObjects } from './en.objects.generated.js'; +import { zhCNObjects } from './zh-CN.objects.generated.js'; +import { jaJPObjects } from './ja-JP.objects.generated.js'; +import { esESObjects } from './es-ES.objects.generated.js'; + +const LOCALES = [ + ['en', enObjects], + ['zh-CN', zhCNObjects], + ['ja-JP', jaJPObjects], + ['es-ES', esESObjects], +] as const; + +const tokensOf = (copy: string): string[] => [...copy.matchAll(/\$\{result\.[\w.]+\}/g)].map((m) => m[0]).sort(); + +const sourceCopy = (): string => + ((SysApprovalRequest as any).actions as any[]).find((a) => a.name === 'approval_approve').successMessage; + +const bundleCopy = (bundle: unknown): string | undefined => + (bundle as any)?.sys_approval_request?._actions?.approval_approve?.successMessage; + +describe('approval_approve success copy names the approved request in every locale (#23157)', () => { + it('the source copy reads the request title from the decide answer', () => { + expect(tokensOf(sourceCopy())).toEqual(['${result.request.record_title}']); + }); + + it('every locale carries the same tokens as the source, so no language toasts the bare verb', () => { + for (const [locale, bundle] of LOCALES) { + const copy = bundleCopy(bundle); + expect(copy, `${locale} approval_approve.successMessage`).toBeTruthy(); + expect(tokensOf(copy!), `${locale} approval_approve.successMessage tokens`).toEqual(tokensOf(sourceCopy())); + } + }); + + it('the English bundle is the source copy, and the other locales translate it', () => { + expect(bundleCopy(enObjects)).toBe(sourceCopy()); + for (const [locale, bundle] of LOCALES) { + if (locale === 'en') continue; + expect(bundleCopy(bundle), `${locale} echoes the English source`).not.toBe(sourceCopy()); + } + }); +}); diff --git a/packages/plugins/plugin-security/src/explain-asker-gate.test.ts b/packages/plugins/plugin-security/src/explain-asker-gate.test.ts index 58292fae7aa..6aec5e81174 100644 --- a/packages/plugins/plugin-security/src/explain-asker-gate.test.ts +++ b/packages/plugins/plugin-security/src/explain-asker-gate.test.ts @@ -269,6 +269,18 @@ const memberDoor = (verb: ExplainOperation, id: string): Promise => /** The decision with its record id set aside: the one field two different ids must differ in. */ const withoutRecordId = (d: ExplainDecision) => ({ ...d, record: d.record ? { ...d.record, recordId: 'ID' } : d.record }); +/** + * [#23157] The decision with the principal's person set aside. The principal + * layer names the explained user as far as the ASKER's own read returns them + * (`explain-principal-person.test.ts`), so two askers' explanations of one + * principal may differ in that name; the record story compared below may not. + */ +const apartFromPrincipalPerson = (d: ExplainDecision) => ({ + ...d, + layers: d.layers.map((l) => (l.layer === 'principal' + ? { ...l, detail: l.detail.replace(/^Principal .*? holds position\(s\) /, 'Principal PERSON holds position(s) ') } + : l)), +}); const layerRecord = (d: ExplainDecision, layer: string) => d.layers.find((l) => l.layer === layer)?.record; /** Every HTTP status, with its code, that any layer's prose names. */ const statusesNamed = (d: ExplainDecision): string[] => { @@ -325,7 +337,7 @@ describe('an asker who can read the row gets the explained principal\'s full rec const bySystem = await explain(SYSTEM, 'read', HIDDEN); expect(byAdmin.record).toEqual({ recordId: HIDDEN, visible: false, decidedBy: 'rls' }); expect(layerRecord(byAdmin, 'rls')?.outcome).toBe('excluded'); - expect(byAdmin).toEqual(bySystem); + expect(apartFromPrincipalPerson(byAdmin)).toEqual(apartFromPrincipalPerson(bySystem)); }); for (const verb of WRITE_VERBS) { @@ -333,7 +345,7 @@ describe('an asker who can read the row gets the explained principal\'s full rec const byAdmin = await explain(ADMIN, verb, HIDDEN); const bySystem = await explain(SYSTEM, verb, HIDDEN); expect(byAdmin.record).toEqual({ recordId: HIDDEN, visible: false, decidedBy: 'rls' }); - expect(byAdmin).toEqual(bySystem); + expect(apartFromPrincipalPerson(byAdmin)).toEqual(apartFromPrincipalPerson(bySystem)); }); it(`${verb}: each row-scoped layer carries the member's read attribution, and names only the door's 404`, async () => { @@ -366,7 +378,9 @@ describe('controls', () => { const bySystem = await explain(SYSTEM, verb, OPEN); expect(bySystem.record).toMatchObject({ recordId: OPEN, visible: true }); expect(layerRecord(bySystem, 'rls')).toMatchObject({ outcome: 'admitted', matchesRecord: true }); - for (const asker of [ADMIN, BLIND_ADMIN, DELEGATE]) expect(await explain(asker, verb, OPEN)).toEqual(bySystem); + for (const asker of [ADMIN, BLIND_ADMIN, DELEGATE]) { + expect(apartFromPrincipalPerson(await explain(asker, verb, OPEN))).toEqual(apartFromPrincipalPerson(bySystem)); + } const self = await explain(SELF, verb, OPEN); expect(self.record).toEqual(bySystem.record); expect(self.layers.map((l) => l.record)).toEqual(bySystem.layers.map((l) => l.record)); diff --git a/packages/plugins/plugin-security/src/explain-principal-person.test.ts b/packages/plugins/plugin-security/src/explain-principal-person.test.ts new file mode 100644 index 00000000000..9cbaa5a1268 --- /dev/null +++ b/packages/plugins/plugin-security/src/explain-principal-person.test.ts @@ -0,0 +1,273 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#23157] The explain panel's Principal resolution names the principal as a +// person: name and email first, the id secondary. And it names them only as +// far as the ASKER can read that person through the read door. +// +// ## The rule +// +// The principal layer's prose used to open `Principal holds +// position(s) …`, while the panel's own Principal line above it already named +// the person. The engine now reads the principal's `sys_user` row with the +// asker's own context, every data middleware included, and names what that +// read returns: `Principal Ada Lovelace (ada@example.test; id usr_pp_ada)`. An +// asker the read returns no row to gets the bare id the prose always carried, +// and a field the read masks stays out. An on-behalf-of delegator is named the +// same way. +// +// ## The rig +// +// The real `SecurityPlugin` over a real `ObjectQL` and SQL driver +// (better-sqlite3, in memory) with the platform's identity and grant tables. +// Every asker below is armed first: what its own read of the principal's user +// row returns is measured on the read door, and each cell's expectation is +// that read's answer, never a fixture assumption. + +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import type { ExplainDecision } from '@objectstack/spec/security'; +import { SysOrganization, SysUser, SysMember } from '@objectstack/platform-objects/identity'; + +import { SysPosition } from './objects/sys-position.object.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; +import { SysUserPosition } from './objects/sys-user-position.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const SYS = { context: { isSystem: true } } as never; +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; + +const BOARD = 'qa_pp_board'; + +/** Reads every user, whole: the directory an administrator browses. */ +const DIRECTORY_SET = PermissionSetSchema.parse({ + name: 'qa_pp_directory', + objects: { sys_user: { allowRead: true, viewAllRecords: true }, [BOARD]: { allowRead: true } }, + systemPermissions: ['manage_users'], +}); +/** The same directory with the email column withheld by field-level security. */ +const MASKED_DIRECTORY_SET = PermissionSetSchema.parse({ + name: 'qa_pp_directory_masked', + objects: { sys_user: { allowRead: true, viewAllRecords: true }, [BOARD]: { allowRead: true } }, + fields: { 'sys_user.email': { readable: false, editable: false } }, + systemPermissions: ['manage_users'], +}); +/** `manage_users` and nothing else: it may explain a user, and its read door returns it no user row. */ +const USERS_ONLY_SET = PermissionSetSchema.parse({ name: 'qa_pp_users_only', objects: {}, systemPermissions: ['manage_users'] }); + +const ADA = 'usr_pp_ada'; +const GRACE = 'usr_pp_grace'; +const PLAIN = 'usr_pp_plain'; +const AGENT = 'usr_pp_agent'; +const GONE = 'usr_pp_gone'; + +const DIRECTORY_ADMIN = { userId: 'usr_pp_directory_admin', positions: [] as string[], permissions: [DIRECTORY_SET.name] }; +const MASKED_ADMIN = { userId: 'usr_pp_masked_admin', positions: [] as string[], permissions: [MASKED_DIRECTORY_SET.name] }; +const BLIND_ADMIN = { userId: 'usr_pp_blind_admin', positions: [] as string[], permissions: [USERS_ONLY_SET.name] }; +const SYSTEM = { isSystem: true }; +/** Ada explaining herself, holding the directory. */ +const ADA_SELF = { userId: ADA, positions: [] as string[], permissions: [DIRECTORY_SET.name] }; +/** An agent holding the directory, acting on behalf of `delegator` (ADR-0090 D10). */ +const agentFor = (delegator: string) => ({ + userId: AGENT, positions: [] as string[], permissions: [DIRECTORY_SET.name], + principalKind: 'agent', onBehalfOf: { userId: delegator }, +}); +/** The same agent without the directory: its own leg holds the baseline alone. */ +const baselineAgentFor = (delegator: string) => ({ ...agentFor(delegator), permissions: [] as string[] }); + +let engine: ObjectQL; +let plugin: SecurityPlugin; + +beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }) as never, + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.explain-principal-person', + name: 'Explain: the principal is named as a person, as far as the asker reads them', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + SysPosition, SysPermissionSet, SysPositionPermissionSet, SysUserPosition, SysUserPermissionSet, + SysOrganization, SysUser, SysMember, + { + name: BOARD, + label: 'Board', + sharingModel: 'public_read_write', + tenancy: { enabled: false }, + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + + const e = engine as unknown as { insert: (o: string, row: unknown, opts: never) => Promise }; + await e.insert('sys_user', [ + { id: ADA, name: 'Ada Lovelace', email: 'ada@example.test' }, + { id: GRACE, name: 'Grace Hopper', email: 'grace@example.test' }, + { id: PLAIN, name: 'Plain Person', email: 'plain@example.test' }, + { id: AGENT, name: 'Release Bot', email: 'bot@example.test' }, + { id: DIRECTORY_ADMIN.userId, name: 'Directory Admin', email: 'dir@example.test' }, + { id: MASKED_ADMIN.userId, name: 'Masked Admin', email: 'masked@example.test' }, + { id: BLIND_ADMIN.userId, name: 'Blind Admin', email: 'blind@example.test' }, + ], SYS); + // Grace holds the directory too, so an agent acting for her keeps it under + // the D10 intersection; the plain user holds nothing beyond the baseline. + await e.insert('sys_permission_set', { + id: 'ps_pp_directory', name: DIRECTORY_SET.name, label: DIRECTORY_SET.name, organization_id: null, managed_by: 'admin', + active: true, object_permissions: JSON.stringify({}), system_permissions: JSON.stringify([]), + }, SYS); + await e.insert('sys_user_permission_set', { + id: 'ups_pp_grace', user_id: GRACE, permission_set_id: 'ps_pp_directory', permission_set: DIRECTORY_SET.name, + }, SYS); + await e.insert(BOARD, [{ id: 'b_pp_1', name: 'one' }], SYS); + + const sets = [MEMBER_DEFAULT, DIRECTORY_SET, MASKED_DIRECTORY_SET, USERS_ONLY_SET]; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (type: string, name: string) => + type === 'permission' ? sets.find((s) => s.name === name) ?? null : engine.getSchema(name) ?? null, + list: async () => sets, + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: (name: string, svc: unknown) => { services[name] = svc; }, + hook: () => undefined, + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); +}); + +afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } +}); + +/** The read door's answer for `userId`'s user row, asked with `asker`'s context: the row, or null for no row or a refusal. */ +const door = (asker: Record, userId: string): Promise | null> => + (engine.findOne('sys_user', { where: { id: userId }, context: { ...asker } } as never) as Promise).then( + (row) => row ?? null, + () => null, + ); + +/** `asker` explains `userId`, or itself when `userId` is omitted. */ +const explain = (asker: Record, userId?: string): Promise => + plugin.explainAccessForCaller({ object: BOARD, operation: 'read', ...(userId ? { userId } : {}) }, { ...asker }); + +const principalDetail = (d: ExplainDecision): string => d.layers.find((l) => l.layer === 'principal')?.detail ?? ''; + +describe('armed: what each asker\'s read door returns of the user rows', () => { + it('the directory holders read Ada whole, the masked one without her email, the blind one not at all', async () => { + expect(await door(SYSTEM, ADA)).toMatchObject({ name: 'Ada Lovelace', email: 'ada@example.test' }); + expect(await door(DIRECTORY_ADMIN, ADA)).toMatchObject({ name: 'Ada Lovelace', email: 'ada@example.test' }); + expect(await door(ADA_SELF, ADA)).toMatchObject({ name: 'Ada Lovelace', email: 'ada@example.test' }); + const masked = await door(MASKED_ADMIN, ADA); + expect(masked).toMatchObject({ name: 'Ada Lovelace' }); + expect(masked?.email ?? null).toBeNull(); + expect(await door(BLIND_ADMIN, ADA)).toBeNull(); + }); + + it('under the D10 intersection: the agent acting for Grace reads both rows; for the plain user, only the delegator\'s; without the directory, neither', async () => { + expect(await door(agentFor(GRACE), GRACE)).toMatchObject({ name: 'Grace Hopper', email: 'grace@example.test' }); + expect(await door(agentFor(GRACE), AGENT)).toMatchObject({ name: 'Release Bot', email: 'bot@example.test' }); + expect(await door(agentFor(PLAIN), PLAIN)).toMatchObject({ name: 'Plain Person', email: 'plain@example.test' }); + expect(await door(agentFor(PLAIN), AGENT)).toBeNull(); + expect(await door(baselineAgentFor(GRACE), GRACE)).toBeNull(); + expect(await door(baselineAgentFor(GRACE), AGENT)).toBeNull(); + }); +}); + +describe('the principal is named as a person, as far as the asker reads them', () => { + it('an administrator who reads the directory: name and email first, the id secondary', async () => { + const d = await explain(DIRECTORY_ADMIN, ADA); + expect(d.principal.userId).toBe(ADA); + expect(principalDetail(d)).toMatch(/^Principal Ada Lovelace \(ada@example\.test; id usr_pp_ada\) holds position\(s\) \[/); + }); + + it('a principal explaining itself is named the same way', async () => { + expect(principalDetail(await explain(ADA_SELF))).toMatch( + /^Principal Ada Lovelace \(ada@example\.test; id usr_pp_ada\) holds position\(s\) \[/, + ); + }); + + it('the system asker reads every row, and is told the same', async () => { + expect(principalDetail(await explain(SYSTEM, ADA))).toMatch( + /^Principal Ada Lovelace \(ada@example\.test; id usr_pp_ada\) holds position\(s\) \[/, + ); + }); + + it('a field the asker\'s read masks stays out: the masked administrator is told the name, never the email', async () => { + const d = await explain(MASKED_ADMIN, ADA); + expect(principalDetail(d)).toMatch(/^Principal Ada Lovelace \(id usr_pp_ada\) holds position\(s\) \[/); + expect(JSON.stringify(d)).not.toContain('ada@example.test'); + }); + + it('control: an asker whose read returns no user row is told the bare id, and nothing more', async () => { + const d = await explain(BLIND_ADMIN, ADA); + expect(d.principal.userId).toBe(ADA); + expect(principalDetail(d)).toMatch(/^Principal usr_pp_ada holds position\(s\) \[/); + const whole = JSON.stringify(d); + expect(whole).not.toContain('Ada Lovelace'); + expect(whole).not.toContain('ada@example.test'); + }); + + it('control: the blind and the directory explanations differ in the principal\'s name alone', async () => { + const strip = (d: ExplainDecision) => JSON.stringify(d).replace(/Principal [^[]*? holds position/g, 'Principal PERSON holds position'); + expect(strip(await explain(BLIND_ADMIN, ADA))).toBe(strip(await explain(DIRECTORY_ADMIN, ADA))); + }); +}); + +describe('an on-behalf-of delegator is named the same way (ADR-0090 D10)', () => { + it('an agent acting for a delegator its read returns: both named as people', async () => { + const detail = principalDetail(await explain(agentFor(GRACE))); + expect(detail).toMatch(/^Principal Release Bot \(bot@example\.test; id usr_pp_agent\) holds position\(s\) \[/); + expect(detail).toContain( + 'Acting on behalf of Grace Hopper (grace@example.test; id usr_pp_grace) — effective access is the D10 INTERSECTION', + ); + }); + + it('control: an agent whose intersected read does not return its own row is named by id, and nothing more', async () => { + const d = await explain(agentFor(PLAIN)); + const detail = principalDetail(d); + expect(detail).toMatch(/^Principal usr_pp_agent holds position\(s\) \[/); + expect(detail).toContain('Acting on behalf of Plain Person (plain@example.test; id usr_pp_plain) — effective access'); + const whole = JSON.stringify(d); + expect(whole).not.toContain('Release Bot'); + expect(whole).not.toContain('bot@example.test'); + }); + + it('control: an agent whose intersected read returns neither row names the delegator by id too, and nothing more', async () => { + const d = await explain(baselineAgentFor(GRACE)); + const detail = principalDetail(d); + expect(detail).toMatch(/^Principal usr_pp_agent holds position\(s\) \[/); + expect(detail).toContain('Acting on behalf of usr_pp_grace — effective access is the D10 INTERSECTION'); + const whole = JSON.stringify(d); + for (const disclosed of ['Grace Hopper', 'grace@example.test', 'Release Bot', 'bot@example.test']) { + expect(whole).not.toContain(disclosed); + } + }); + + it('a delegator that no longer exists is named by the id the link carries', async () => { + expect(principalDetail(await explain(agentFor(GONE)))).toContain( + 'Acting on behalf of usr_pp_gone, who no longer exists — D10 fails CLOSED (access denied).', + ); + }); +}); From 0e8b7204a0f61d89c2986ad575debb6a11977ace Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:33:24 +0000 Subject: [PATCH 3/7] test(plugin-approvals): narrow the opened request in the success-copy cells so the test-typecheck debt does not grow Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- .../plugin-approvals/src/approval-service.test.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 70ecd090468..9197d9b1aff 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1308,6 +1308,9 @@ describe('ApprovalService (node era)', () => { }); return { text, unresolved }; }; + /** The id of the request an ordinary open answers (never an auto-outcome in these cells). */ + const openRequestId = async (...args: Parameters): Promise => + ((await svc.openNodeRequest(...args)) as { id: string }).id; it('reads the answer: the copy carries at least one `${result.*}` token', () => { expect(approveCopy()).toMatch(/\$\{result\.[\w.]+\}/); @@ -1315,8 +1318,8 @@ describe('ApprovalService (node era)', () => { it('a finalizing approve: the toast names the record, the way the inbox row\'s inline Approve does', async () => { engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; - const req = await svc.openNodeRequest(openInput(['u9']), CTX); - const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.decide(id, { decision: 'approve', actorId: 'u9' }, SYS); expect(out.finalized).toBe(true); const { text, unresolved } = fill(approveCopy(), out); expect(unresolved).toEqual([]); @@ -1325,8 +1328,8 @@ describe('ApprovalService (node era)', () => { it('an approve that leaves the step waiting on others still names the record', async () => { engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; - const req = await svc.openNodeRequest(openInput(['u1', 'u2'], {}, { behavior: 'unanimous' }), CTX); - const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u1' }, SYS); + const id = await openRequestId(openInput(['u1', 'u2'], {}, { behavior: 'unanimous' }), CTX); + const out = await svc.decide(id, { decision: 'approve', actorId: 'u1' }, SYS); expect(out.finalized).toBe(false); const { text, unresolved } = fill(approveCopy(), out); expect(unresolved).toEqual([]); @@ -1335,10 +1338,10 @@ describe('ApprovalService (node era)', () => { }); it('a record whose row is gone is named from the payload snapshot, as the inbox names it', async () => { - const req = await svc.openNodeRequest( + const id = await openRequestId( openInput(['u9'], { record: { id: 'opp1', name: 'Snapshot Title', amount: 1 } }), CTX, ); - const out = await svc.decide(req.id, { decision: 'approve', actorId: 'u9' }, SYS); + const out = await svc.decide(id, { decision: 'approve', actorId: 'u9' }, SYS); const { text, unresolved } = fill(approveCopy(), out); expect(unresolved).toEqual([]); expect(text).toBe('Approved "Snapshot Title"'); From 09218ddc1cebd2b5a569d5c3f4992536028116c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:33:46 +0000 Subject: [PATCH 4/7] chore(changeset): plugin-approvals and plugin-security patch for the named approve toast and principal Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- ...57-server-authored-ui-text-names-what-and-who.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/23157-server-authored-ui-text-names-what-and-who.md diff --git a/.changeset/23157-server-authored-ui-text-names-what-and-who.md b/.changeset/23157-server-authored-ui-text-names-what-and-who.md new file mode 100644 index 00000000000..36d148a7393 --- /dev/null +++ b/.changeset/23157-server-authored-ui-text-names-what-and-who.md @@ -0,0 +1,13 @@ +--- +"@objectstack/plugin-approvals": patch +"@objectstack/plugin-security": patch +--- + +fix: the approvals drawer's Approve toast names the approved request, and Explain access names the principal as a person + +Clause-②: no + +Two server-authored strings that the console renders verbatim now name what and who they are about. + +- `@objectstack/plugin-approvals`: the `approval_approve` action's `successMessage` was the bare verb `Approved.`. It is now `Approved "${result.request.record_title}"`, the wording of the inbox row's inline Approve. The decide route already answers the read-back request with its display enrichment, so `record_title` is the title the inbox shows (the target record's display field, else its payload snapshot). The zh-CN, ja-JP and es-ES values of the same key are updated to carry the same token. No key, route or response shape changes. +- `@objectstack/plugin-security`: the explain engine's principal layer said `Principal USER_ID holds position(s) …`. It now names the person first and the id second, for example `Principal Ada Lovelace (ada@example.com; id usr_1) holds position(s) …`, and an on-behalf-of delegator is named the same way. The user row is read with the asker's own context, so row-level and field-level security apply. An asker whose read returns no row gets the bare id, as before, and a field its read masks is left out. As a result, two askers can now be shown different names for the same principal. The decision's `principal` object and every verdict are unchanged. From 4d7a1cd83c7ea10eddeab8dac18509d3e05a19cf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 08:26:02 +0000 Subject: [PATCH 5/7] fix(plugin-security): explain names the user an ADR-0091 D3 delegated position came from, in the clause and the attribution Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- .../plugin-security/src/explain-engine.ts | 11 ++++- .../src/explain-principal-person.test.ts | 47 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 5499d10fec9..c623872ba91 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -2095,6 +2095,13 @@ async function explainOnce(deps: ExplainEngineDeps, input: ExplainInput, asker: const delegatorPerson = context?.onBehalfOf?.userId ? await personOf(deps.ql, askerContext, String(context.onBehalfOf.userId)) : ''; + // ...and so is the user each ADR-0091 D3 delegated position came from, once + // per distinct user, in the clause and in the position's attribution alike. + const positionDelegators = new Map(); + for (const from of new Set(delegatedPositions.map((d) => d.from))) { + positionDelegators.set(from, await personOf(deps.ql, askerContext, from)); + } + const delegatorOf = (from: string): string => positionDelegators.get(from) ?? from; layers.push({ layer: 'principal', verdict: delegatorMissing || delegatorUnresolved ? 'denies' : 'neutral', @@ -2110,7 +2117,7 @@ async function explainOnce(deps: ExplainEngineDeps, input: ExplainInput, asker: : '') + (delegatedPositions.length > 0 ? ` ${delegatedPositions.length} position(s) held via delegation (ADR-0091 D3): [${delegatedPositions - .map((d) => `${d.name} from ${d.from}${d.until ? ` until ${d.until}` : ''}`) + .map((d) => `${d.name} from ${delegatorOf(d.from)}${d.until ? ` until ${d.until}` : ''}`) .join(', ')}].` : '') + (expiredGrants.length > 0 @@ -2127,7 +2134,7 @@ async function explainOnce(deps: ExplainEngineDeps, input: ExplainInput, asker: ...positions.map((p) => { const d = delegationOf(p); return d - ? { kind: 'position' as const, name: p, via: `delegation from ${d.from}${d.until ? ` until ${d.until}` : ''}` } + ? { kind: 'position' as const, name: p, via: `delegation from ${delegatorOf(d.from)}${d.until ? ` until ${d.until}` : ''}` } : { kind: 'position' as const, name: p }; }), ...setNames.map((n) => ({ kind: 'permission_set' as const, name: n, via: viaOf(n) })), diff --git a/packages/plugins/plugin-security/src/explain-principal-person.test.ts b/packages/plugins/plugin-security/src/explain-principal-person.test.ts index 9cbaa5a1268..2a5cac45496 100644 --- a/packages/plugins/plugin-security/src/explain-principal-person.test.ts +++ b/packages/plugins/plugin-security/src/explain-principal-person.test.ts @@ -64,6 +64,10 @@ const GRACE = 'usr_pp_grace'; const PLAIN = 'usr_pp_plain'; const AGENT = 'usr_pp_agent'; const GONE = 'usr_pp_gone'; +/** Holds a position Grace delegated to her (ADR-0091 D3). */ +const LIN = 'usr_pp_lin'; +const DELEGATED_POSITION = 'qa_pp_approver'; +const DELEGATED_UNTIL = '2099-01-01T00:00:00.000Z'; const DIRECTORY_ADMIN = { userId: 'usr_pp_directory_admin', positions: [] as string[], permissions: [DIRECTORY_SET.name] }; const MASKED_ADMIN = { userId: 'usr_pp_masked_admin', positions: [] as string[], permissions: [MASKED_DIRECTORY_SET.name] }; @@ -117,6 +121,7 @@ beforeAll(async () => { { id: ADA, name: 'Ada Lovelace', email: 'ada@example.test' }, { id: GRACE, name: 'Grace Hopper', email: 'grace@example.test' }, { id: PLAIN, name: 'Plain Person', email: 'plain@example.test' }, + { id: LIN, name: 'Lin Delegate', email: 'lin@example.test' }, { id: AGENT, name: 'Release Bot', email: 'bot@example.test' }, { id: DIRECTORY_ADMIN.userId, name: 'Directory Admin', email: 'dir@example.test' }, { id: MASKED_ADMIN.userId, name: 'Masked Admin', email: 'masked@example.test' }, @@ -131,6 +136,11 @@ beforeAll(async () => { await e.insert('sys_user_permission_set', { id: 'ups_pp_grace', user_id: GRACE, permission_set_id: 'ps_pp_directory', permission_set: DIRECTORY_SET.name, }, SYS); + // A position Lin holds because Grace delegated it, until a date far ahead. + await e.insert('sys_user_position', { + id: 'up_pp_lin_delegated', user_id: LIN, position: DELEGATED_POSITION, + delegated_from: GRACE, valid_until: DELEGATED_UNTIL, + }, SYS); await e.insert(BOARD, [{ id: 'b_pp_1', name: 'one' }], SYS); const sets = [MEMBER_DEFAULT, DIRECTORY_SET, MASKED_DIRECTORY_SET, USERS_ONLY_SET]; @@ -271,3 +281,40 @@ describe('an on-behalf-of delegator is named the same way (ADR-0090 D10)', () => ); }); }); + +describe('the user an ADR-0091 D3 delegated position came from is named the same way', () => { + const contributorVia = (d: ExplainDecision): string | undefined => + d.layers.find((l) => l.layer === 'principal')?.contributors?.find((c) => c.kind === 'position' && c.name === DELEGATED_POSITION)?.via; + + it('armed: Lin holds the delegated position, and the askers read Grace as they read Ada', async () => { + expect((await explain(SYSTEM, LIN)).principal.positions).toContain(DELEGATED_POSITION); + expect(await door(DIRECTORY_ADMIN, GRACE)).toMatchObject({ name: 'Grace Hopper', email: 'grace@example.test' }); + const masked = await door(MASKED_ADMIN, GRACE); + expect(masked).toMatchObject({ name: 'Grace Hopper' }); + expect(masked?.email ?? null).toBeNull(); + expect(await door(BLIND_ADMIN, GRACE)).toBeNull(); + }); + + it('an administrator who reads the directory: the clause and the attribution name Grace, the id secondary', async () => { + const d = await explain(DIRECTORY_ADMIN, LIN); + const grace = 'Grace Hopper (grace@example.test; id usr_pp_grace)'; + expect(principalDetail(d)).toContain(`1 position(s) held via delegation (ADR-0091 D3): [${DELEGATED_POSITION} from ${grace} until `); + expect(contributorVia(d)).toMatch(/^delegation from Grace Hopper \(grace@example\.test; id usr_pp_grace\) until /); + }); + + it('a field the asker\'s read masks stays out: the masked administrator is told Grace\'s name, never her email', async () => { + const d = await explain(MASKED_ADMIN, LIN); + expect(principalDetail(d)).toContain(`${DELEGATED_POSITION} from Grace Hopper (id usr_pp_grace) until `); + expect(contributorVia(d)).toMatch(/^delegation from Grace Hopper \(id usr_pp_grace\) until /); + expect(JSON.stringify(d)).not.toContain('grace@example.test'); + }); + + it('control: an asker whose read returns no row for Grace is told her bare id, and nothing more', async () => { + const d = await explain(BLIND_ADMIN, LIN); + expect(principalDetail(d)).toContain(`${DELEGATED_POSITION} from usr_pp_grace until `); + expect(contributorVia(d)).toMatch(/^delegation from usr_pp_grace until /); + const whole = JSON.stringify(d); + expect(whole).not.toContain('Grace Hopper'); + expect(whole).not.toContain('grace@example.test'); + }); +}); From 73046d21d9216a5283a66731e044b44ce020f662 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 08:32:12 +0000 Subject: [PATCH 6/7] fix(plugin-approvals): every request action's success copy names the request, in all four locales Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- .../src/approval-service.test.ts | 126 +++++++++++++----- .../src/sys-approval-request.object.ts | 18 +-- .../approve-success-copy-i18n.test.ts | 54 -------- .../src/translations/en.objects.generated.ts | 16 +-- .../translations/es-ES.objects.generated.ts | 16 +-- .../translations/ja-JP.objects.generated.ts | 16 +-- .../success-copy-names-request-i18n.test.ts | 68 ++++++++++ .../translations/zh-CN.objects.generated.ts | 16 +-- 8 files changed, 206 insertions(+), 124 deletions(-) delete mode 100644 packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts create mode 100644 packages/plugins/plugin-approvals/src/translations/success-copy-names-request-i18n.test.ts diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 9197d9b1aff..7ad4f3348a5 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -1288,16 +1288,15 @@ describe('ApprovalService (node era)', () => { expect(rows[0].record_title).toBe('Snapshot Title'); }); - // [#23157] The approve action is `type: 'api'` against the decide route, - // which answers decide()'s result verbatim; its success copy interpolates - // `${result.*}` over that answer (the scope `ActionSchema.successMessage` - // declares). So the toast names what was approved only while decide()'s - // answer carries the title the copy reads, finalized or not. - describe('the approve action\'s success copy, filled from decide()\'s answer', () => { - const approveCopy = (): string => { - const action = (SysApprovalRequest as any).actions.find((a: any) => a.name === 'approval_approve'); - return action.successMessage; - }; + // [#23157] Every request action is `type: 'api'` against a route that + // answers its service method's result verbatim, and each of those results + // carries `request`, the read-back row with its display enrichment. Each + // action's success copy interpolates `${result.*}` over that answer (the + // scope `ActionSchema.successMessage` declares), so the toast names the + // request only while the answer carries the title the copy reads. + describe('each request action\'s success copy, filled from its route\'s answer', () => { + const copyOf = (name: string): string => + ((SysApprovalRequest as any).actions as any[]).find((a) => a.name === name).successMessage; /** The copy's `${result.}` tokens, each filled from the answer; an unresolved path reads as ''. */ const fill = (copy: string, result: unknown): { text: string; unresolved: string[] } => { const unresolved: string[] = []; @@ -1308,43 +1307,110 @@ describe('ApprovalService (node era)', () => { }); return { text, unresolved }; }; + const named = (action: string, answer: unknown): string => { + const { text, unresolved } = fill(copyOf(action), answer); + expect(unresolved, `${action}: every token resolves on the answer`).toEqual([]); + return text; + }; /** The id of the request an ordinary open answers (never an auto-outcome in these cells). */ const openRequestId = async (...args: Parameters): Promise => ((await svc.openNodeRequest(...args)) as { id: string }).id; + /** A flow whose approval node declares the `revise` out-edge send-back needs (#3823). */ + const REVISE_FLOW = { + name: 'deal_approval', + nodes: [ + { id: 'approve_step', type: 'approval' }, + { id: 'wait_revision', type: APPROVAL_REVISE_NODE_TYPE }, + ], + edges: [{ id: 'e_rev', source: 'approve_step', target: 'wait_revision', label: 'revise' }], + }; - it('reads the answer: the copy carries at least one `${result.*}` token', () => { - expect(approveCopy()).toMatch(/\$\{result\.[\w.]+\}/); + beforeEach(() => { + engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; + svc.attachAutomation({ + async resume() { /* resumed */ }, + async cancelRun() { /* handed back */ }, + async getFlow() { return REVISE_FLOW; }, + } as any); }); - it('a finalizing approve: the toast names the record, the way the inbox row\'s inline Approve does', async () => { - engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; + it('reads the answer: every action\'s copy carries a `${result.*}` token', () => { + for (const a of (SysApprovalRequest as any).actions as any[]) { + expect(a.successMessage, `${a.name}.successMessage`).toMatch(/\$\{result\.[\w.]+\}/); + } + }); + + it('approve, finalizing: the toast names the record, the way the inbox row\'s inline Approve does', async () => { const id = await openRequestId(openInput(['u9']), CTX); const out = await svc.decide(id, { decision: 'approve', actorId: 'u9' }, SYS); expect(out.finalized).toBe(true); - const { text, unresolved } = fill(approveCopy(), out); - expect(unresolved).toEqual([]); - expect(text).toBe('Approved "Acme Renewal"'); + expect(named('approval_approve', out)).toBe('Approved "Acme Renewal"'); }); - it('an approve that leaves the step waiting on others still names the record', async () => { - engine._tables['opportunity'] = [{ id: 'opp1', name: 'Acme Renewal', amount: 100 }]; + it('approve, leaving the step waiting on others: still names the record', async () => { const id = await openRequestId(openInput(['u1', 'u2'], {}, { behavior: 'unanimous' }), CTX); const out = await svc.decide(id, { decision: 'approve', actorId: 'u1' }, SYS); expect(out.finalized).toBe(false); - const { text, unresolved } = fill(approveCopy(), out); - expect(unresolved).toEqual([]); - expect(text).toContain('Acme Renewal'); - expect(text).not.toBe('Approved.'); + expect(named('approval_approve', out)).toBe('Approved "Acme Renewal"'); }); - it('a record whose row is gone is named from the payload snapshot, as the inbox names it', async () => { - const id = await openRequestId( - openInput(['u9'], { record: { id: 'opp1', name: 'Snapshot Title', amount: 1 } }), CTX, - ); + it('approve, on a record whose row is gone: named from the payload snapshot, as the inbox names it', async () => { + engine._tables['opportunity'] = []; + const id = await openRequestId(openInput(['u9'], { record: { id: 'opp1', name: 'Snapshot Title', amount: 1 } }), CTX); const out = await svc.decide(id, { decision: 'approve', actorId: 'u9' }, SYS); - const { text, unresolved } = fill(approveCopy(), out); - expect(unresolved).toEqual([]); - expect(text).toBe('Approved "Snapshot Title"'); + expect(named('approval_approve', out)).toBe('Approved "Snapshot Title"'); + }); + + it('reject: names the record, the way the inbox row\'s inline Reject does', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.decide(id, { decision: 'reject', actorId: 'u9' }, SYS); + expect(out.request.status).toBe('rejected'); + expect(named('approval_reject', out)).toBe('Rejected "Acme Renewal"'); + }); + + it('reassign: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.reassign(id, { actorId: 'u9', to: 'u7' }, asUser('u9')); + expect(named('approval_reassign', out)).toBe('Reassigned "Acme Renewal"'); + }); + + it('send back for revision: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.sendBack(id, { actorId: 'u9', comment: 'split the lines' }, asUser('u9')); + expect(out.request.status).toBe('returned'); + expect(named('approval_send_back', out)).toBe('Sent "Acme Renewal" back for revision'); + }); + + it('request information: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.requestInfo(id, { actorId: 'u9', comment: 'which cost centre?' }, asUser('u9')); + expect(named('approval_request_info', out)).toBe('Information requested on "Acme Renewal"'); + }); + + it('remind: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.remind(id, { actorId: 'u1' }, CTX); + expect(named('approval_remind', out)).toBe('Reminder sent for "Acme Renewal"'); + }); + + it('recall: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.recall(id, { actorId: 'u1' }, CTX); + expect(out.request.status).toBe('recalled'); + expect(named('approval_recall', out)).toBe('Recalled "Acme Renewal"'); + }); + + it('resubmit: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + await svc.sendBack(id, { actorId: 'u9', comment: 'split the lines' }, asUser('u9')); + const out = await svc.resubmit(id, { actorId: 'u1', comment: 'split' }, CTX); + expect(named('approval_resubmit', out)).toBe('Resubmitted "Acme Renewal"'); + }); + + it('reply: names the record', async () => { + const id = await openRequestId(openInput(['u9']), CTX); + const out = await svc.comment(id, { actorId: 'u1', comment: 'attached the receipt' }, CTX); + expect(named('approval_comment', out)).toBe('Reply posted on "Acme Renewal"'); }); }); diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 423d561dfaf..9fa7608ae58 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -477,6 +477,8 @@ export const SysApprovalRequest = ObjectSchema.create({ // same title the inbox row shows (the target record's display field, // else its payload snapshot). `${result.*}` is the success copy's // declared scope on a `type: 'api'` action: the route's own response. + // Every action below names the request the same way: each of their + // routes answers `{ request, … }` with that same read-back row. successMessage: 'Approved "${result.request.record_title}"', refreshAfter: true, }, @@ -507,7 +509,7 @@ export const SysApprovalRequest = ObjectSchema.create({ 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true' + ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true', locations: ['record_section', 'list_item'], - successMessage: 'Rejected.', + successMessage: 'Rejected "${result.request.record_title}"', refreshAfter: true, }, { @@ -530,7 +532,7 @@ export const SysApprovalRequest = ObjectSchema.create({ 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true' + ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true', locations: ['record_section'], - successMessage: 'Reassigned.', + successMessage: 'Reassigned "${result.request.record_title}"', refreshAfter: true, }, @@ -550,7 +552,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ], visible: 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true', locations: ['record_section'], - successMessage: 'Sent back for revision.', + successMessage: 'Sent "${result.request.record_title}" back for revision', refreshAfter: true, }, { @@ -565,7 +567,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ], visible: 'has(record.viewer) && has(record.viewer.can_act) && record.viewer.can_act == true', locations: ['record_section'], - successMessage: 'Information requested.', + successMessage: 'Information requested on "${result.request.record_title}"', refreshAfter: true, }, @@ -592,7 +594,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ], visible: 'has(record.status) && record.status == "pending" && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true', locations: ['record_section'], - successMessage: 'Reminder sent.', + successMessage: 'Reminder sent for "${result.request.record_title}"', refreshAfter: true, }, { @@ -634,7 +636,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true' + ' || has(record.viewer) && has(record.viewer.can_override) && record.viewer.can_override == true', locations: ['record_section'], - successMessage: 'Recalled.', + successMessage: 'Recalled "${result.request.record_title}"', refreshAfter: true, }, { @@ -649,7 +651,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ], visible: 'has(record.status) && record.status == "returned" && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true', locations: ['record_section'], - successMessage: 'Resubmitted.', + successMessage: 'Resubmitted "${result.request.record_title}"', refreshAfter: true, }, @@ -697,7 +699,7 @@ export const SysApprovalRequest = ObjectSchema.create({ ' || has(record.status) && record.status == "pending"' + ' && has(record.viewer) && has(record.viewer.is_submitter) && record.viewer.is_submitter == true', locations: ['record_section'], - successMessage: 'Reply posted.', + successMessage: 'Reply posted on "${result.request.record_title}"', refreshAfter: true, }, ], diff --git a/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts b/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts deleted file mode 100644 index 243b6c022e3..00000000000 --- a/packages/plugins/plugin-approvals/src/translations/approve-success-copy-i18n.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -// -// The approve action's success toast names what was approved (#23157), and -// every shipped locale says so too. -// -// The copy is an `I18nLabel` the console resolves from these bundles BEFORE it -// fills the `${result.*}` tokens, so a locale whose value drops the token -// toasts the bare verb in that language while English names the record. No -// drift gate sees that: `check:i18n` reads a translated-locale value as -// hand-written text and never compares its tokens with the source's. - -import { describe, it, expect } from 'vitest'; -import { SysApprovalRequest } from '../sys-approval-request.object.js'; -import { enObjects } from './en.objects.generated.js'; -import { zhCNObjects } from './zh-CN.objects.generated.js'; -import { jaJPObjects } from './ja-JP.objects.generated.js'; -import { esESObjects } from './es-ES.objects.generated.js'; - -const LOCALES = [ - ['en', enObjects], - ['zh-CN', zhCNObjects], - ['ja-JP', jaJPObjects], - ['es-ES', esESObjects], -] as const; - -const tokensOf = (copy: string): string[] => [...copy.matchAll(/\$\{result\.[\w.]+\}/g)].map((m) => m[0]).sort(); - -const sourceCopy = (): string => - ((SysApprovalRequest as any).actions as any[]).find((a) => a.name === 'approval_approve').successMessage; - -const bundleCopy = (bundle: unknown): string | undefined => - (bundle as any)?.sys_approval_request?._actions?.approval_approve?.successMessage; - -describe('approval_approve success copy names the approved request in every locale (#23157)', () => { - it('the source copy reads the request title from the decide answer', () => { - expect(tokensOf(sourceCopy())).toEqual(['${result.request.record_title}']); - }); - - it('every locale carries the same tokens as the source, so no language toasts the bare verb', () => { - for (const [locale, bundle] of LOCALES) { - const copy = bundleCopy(bundle); - expect(copy, `${locale} approval_approve.successMessage`).toBeTruthy(); - expect(tokensOf(copy!), `${locale} approval_approve.successMessage tokens`).toEqual(tokensOf(sourceCopy())); - } - }); - - it('the English bundle is the source copy, and the other locales translate it', () => { - expect(bundleCopy(enObjects)).toBe(sourceCopy()); - for (const [locale, bundle] of LOCALES) { - if (locale === 'en') continue; - expect(bundleCopy(bundle), `${locale} echoes the English source`).not.toBe(sourceCopy()); - } - }); -}); diff --git a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts index 5bdb49aa25a..fbe91fbaceb 100644 --- a/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/en.objects.generated.ts @@ -138,7 +138,7 @@ export const enObjects: NonNullable = { approval_reject: { label: "Reject", description: "Reject this request? A rejection is final for every approver.", - successMessage: "Rejected.", + successMessage: "Rejected \"${result.request.record_title}\"", params: { comment: { label: "Comment" @@ -150,7 +150,7 @@ export const enObjects: NonNullable = { }, approval_reassign: { label: "Reassign", - successMessage: "Reassigned.", + successMessage: "Reassigned \"${result.request.record_title}\"", params: { to: { label: "New approver", @@ -163,7 +163,7 @@ export const enObjects: NonNullable = { }, approval_send_back: { label: "Send back", - successMessage: "Sent back for revision.", + successMessage: "Sent \"${result.request.record_title}\" back for revision", params: { comment: { label: "Reason" @@ -172,7 +172,7 @@ export const enObjects: NonNullable = { }, approval_request_info: { label: "Request info", - successMessage: "Information requested.", + successMessage: "Information requested on \"${result.request.record_title}\"", params: { comment: { label: "What do you need?" @@ -181,7 +181,7 @@ export const enObjects: NonNullable = { }, approval_remind: { label: "Send reminder", - successMessage: "Reminder sent.", + successMessage: "Reminder sent for \"${result.request.record_title}\"", params: { comment: { label: "Note" @@ -191,7 +191,7 @@ export const enObjects: NonNullable = { approval_recall: { label: "Recall", description: "Recall this request? Approvers can no longer act on it and the record is unlocked.", - successMessage: "Recalled.", + successMessage: "Recalled \"${result.request.record_title}\"", params: { comment: { label: "Comment" @@ -200,7 +200,7 @@ export const enObjects: NonNullable = { }, approval_resubmit: { label: "Resubmit", - successMessage: "Resubmitted.", + successMessage: "Resubmitted \"${result.request.record_title}\"", params: { comment: { label: "What changed?" @@ -209,7 +209,7 @@ export const enObjects: NonNullable = { }, approval_comment: { label: "Reply", - successMessage: "Reply posted.", + successMessage: "Reply posted on \"${result.request.record_title}\"", params: { comment: { label: "Comment" diff --git a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts index 0d8018071d5..3d9d71979ba 100644 --- a/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/es-ES.objects.generated.ts @@ -138,7 +138,7 @@ export const esESObjects: NonNullable = { approval_reject: { label: "Rechazar", description: "¿Rechazar esta solicitud? Un rechazo es definitivo para todos los aprobadores.", - successMessage: "Rechazada.", + successMessage: "\"${result.request.record_title}\" rechazada", params: { comment: { label: "Comentario" @@ -150,7 +150,7 @@ export const esESObjects: NonNullable = { }, approval_reassign: { label: "Reasignar", - successMessage: "Reasignada.", + successMessage: "\"${result.request.record_title}\" reasignada", params: { to: { label: "Nuevo aprobador", @@ -163,7 +163,7 @@ export const esESObjects: NonNullable = { }, approval_send_back: { label: "Devolver", - successMessage: "Devuelta para revisión.", + successMessage: "\"${result.request.record_title}\" devuelta para revisión", params: { comment: { label: "Motivo" @@ -172,7 +172,7 @@ export const esESObjects: NonNullable = { }, approval_request_info: { label: "Pedir información", - successMessage: "Información solicitada.", + successMessage: "Información solicitada sobre \"${result.request.record_title}\"", params: { comment: { label: "¿Qué necesitas?" @@ -181,7 +181,7 @@ export const esESObjects: NonNullable = { }, approval_remind: { label: "Enviar recordatorio", - successMessage: "Recordatorio enviado.", + successMessage: "Recordatorio enviado para \"${result.request.record_title}\"", params: { comment: { label: "Nota" @@ -191,7 +191,7 @@ export const esESObjects: NonNullable = { approval_recall: { label: "Retirar", description: "¿Retirar esta solicitud? Los aprobadores ya no podrán actuar sobre ella y el registro se desbloqueará.", - successMessage: "Retirada.", + successMessage: "\"${result.request.record_title}\" retirada", params: { comment: { label: "Comentario" @@ -200,7 +200,7 @@ export const esESObjects: NonNullable = { }, approval_resubmit: { label: "Reenviar", - successMessage: "Reenviada.", + successMessage: "\"${result.request.record_title}\" reenviada", params: { comment: { label: "¿Qué ha cambiado?" @@ -209,7 +209,7 @@ export const esESObjects: NonNullable = { }, approval_comment: { label: "Responder", - successMessage: "Respuesta publicada.", + successMessage: "Respuesta publicada en \"${result.request.record_title}\"", params: { comment: { label: "Comentario" diff --git a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts index 1aa2e08b530..f87e1019fcc 100644 --- a/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/ja-JP.objects.generated.ts @@ -138,7 +138,7 @@ export const jaJPObjects: NonNullable = { approval_reject: { label: "却下", description: "このリクエストを却下しますか？却下はすべての承認者に対して最終決定になります。", - successMessage: "却下しました。", + successMessage: "「${result.request.record_title}」を却下しました", params: { comment: { label: "コメント" @@ -150,7 +150,7 @@ export const jaJPObjects: NonNullable = { }, approval_reassign: { label: "再割り当て", - successMessage: "再割り当てしました。", + successMessage: "「${result.request.record_title}」を再割り当てしました", params: { to: { label: "新しい承認者", @@ -163,7 +163,7 @@ export const jaJPObjects: NonNullable = { }, approval_send_back: { label: "差し戻し", - successMessage: "修正のため差し戻しました。", + successMessage: "「${result.request.record_title}」を修正のため差し戻しました", params: { comment: { label: "理由" @@ -172,7 +172,7 @@ export const jaJPObjects: NonNullable = { }, approval_request_info: { label: "追加情報を依頼", - successMessage: "追加情報を依頼しました。", + successMessage: "「${result.request.record_title}」について追加情報を依頼しました", params: { comment: { label: "何が必要ですか？" @@ -181,7 +181,7 @@ export const jaJPObjects: NonNullable = { }, approval_remind: { label: "リマインダー送信", - successMessage: "リマインダーを送信しました。", + successMessage: "「${result.request.record_title}」のリマインダーを送信しました", params: { comment: { label: "メモ" @@ -191,7 +191,7 @@ export const jaJPObjects: NonNullable = { approval_recall: { label: "取り下げ", description: "このリクエストを取り下げますか？承認者は操作できなくなり、レコードのロックが解除されます。", - successMessage: "取り下げました。", + successMessage: "「${result.request.record_title}」を取り下げました", params: { comment: { label: "コメント" @@ -200,7 +200,7 @@ export const jaJPObjects: NonNullable = { }, approval_resubmit: { label: "再提出", - successMessage: "再提出しました。", + successMessage: "「${result.request.record_title}」を再提出しました", params: { comment: { label: "何を変更しましたか？" @@ -209,7 +209,7 @@ export const jaJPObjects: NonNullable = { }, approval_comment: { label: "返信", - successMessage: "返信を投稿しました。", + successMessage: "「${result.request.record_title}」に返信を投稿しました", params: { comment: { label: "コメント" diff --git a/packages/plugins/plugin-approvals/src/translations/success-copy-names-request-i18n.test.ts b/packages/plugins/plugin-approvals/src/translations/success-copy-names-request-i18n.test.ts new file mode 100644 index 00000000000..cdf069ce5f2 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/translations/success-copy-names-request-i18n.test.ts @@ -0,0 +1,68 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Every request action's success toast names the request it acted on +// (#23157), and every shipped locale says so too. +// +// The copy is an `I18nLabel` the console resolves from these bundles BEFORE it +// fills the `${result.*}` tokens, so a locale whose value drops the token +// toasts the bare verb in that language while English names the record. No +// drift gate sees that: `check:i18n` reads a translated-locale value as +// hand-written text and never compares its tokens with the source's. + +import { describe, it, expect } from 'vitest'; +import { SysApprovalRequest } from '../sys-approval-request.object.js'; +import { enObjects } from './en.objects.generated.js'; +import { zhCNObjects } from './zh-CN.objects.generated.js'; +import { jaJPObjects } from './ja-JP.objects.generated.js'; +import { esESObjects } from './es-ES.objects.generated.js'; + +const LOCALES = [ + ['en', enObjects], + ['zh-CN', zhCNObjects], + ['ja-JP', jaJPObjects], + ['es-ES', esESObjects], +] as const; + +/** Every action declared on the request object, with its source copy. */ +const ACTIONS = ((SysApprovalRequest as any).actions as Array<{ name: string; successMessage?: string }>) + .map((a) => [a.name, a.successMessage ?? ''] as const); + +const tokensOf = (copy: string): string[] => [...copy.matchAll(/\$\{result\.[\w.]+\}/g)].map((m) => m[0]).sort(); + +const bundleCopy = (bundle: unknown, action: string): string | undefined => + (bundle as any)?.sys_approval_request?._actions?.[action]?.successMessage; + +describe('each sys_approval_request action\'s success copy names the request, in every locale (#23157)', () => { + it('armed: the object declares all nine request actions', () => { + expect(ACTIONS.map(([name]) => name).sort()).toEqual([ + 'approval_approve', 'approval_comment', 'approval_reassign', 'approval_recall', 'approval_reject', + 'approval_remind', 'approval_request_info', 'approval_resubmit', 'approval_send_back', + ]); + }); + + it('every source copy reads the request title from its route\'s answer', () => { + for (const [name, copy] of ACTIONS) { + expect(tokensOf(copy), `${name}.successMessage`).toEqual(['${result.request.record_title}']); + } + }); + + it('every locale carries the same tokens as the source, so no language toasts the bare verb', () => { + for (const [locale, bundle] of LOCALES) { + for (const [name, copy] of ACTIONS) { + const translated = bundleCopy(bundle, name); + expect(translated, `${locale} ${name}.successMessage`).toBeTruthy(); + expect(tokensOf(translated!), `${locale} ${name}.successMessage tokens`).toEqual(tokensOf(copy)); + } + } + }); + + it('the English bundle is the source copy, and the other locales translate it', () => { + for (const [name, copy] of ACTIONS) { + expect(bundleCopy(enObjects, name), `en ${name}`).toBe(copy); + for (const [locale, bundle] of LOCALES) { + if (locale === 'en') continue; + expect(bundleCopy(bundle, name), `${locale} ${name} echoes the English source`).not.toBe(copy); + } + } + }); +}); diff --git a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts index 4458a309d6f..8c347046464 100644 --- a/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-approvals/src/translations/zh-CN.objects.generated.ts @@ -138,7 +138,7 @@ export const zhCNObjects: NonNullable = { approval_reject: { label: "拒绝", description: "拒绝该请求？拒绝对所有审批人立即生效。", - successMessage: "已拒绝。", + successMessage: "已拒绝“${result.request.record_title}”", params: { comment: { label: "审批意见" @@ -150,7 +150,7 @@ export const zhCNObjects: NonNullable = { }, approval_reassign: { label: "转签", - successMessage: "已转签。", + successMessage: "已转签“${result.request.record_title}”", params: { to: { label: "新审批人", @@ -163,7 +163,7 @@ export const zhCNObjects: NonNullable = { }, approval_send_back: { label: "退回修改", - successMessage: "已退回修改。", + successMessage: "已将“${result.request.record_title}”退回修改", params: { comment: { label: "原因" @@ -172,7 +172,7 @@ export const zhCNObjects: NonNullable = { }, approval_request_info: { label: "退回补充", - successMessage: "已退回给申请人补充材料。", + successMessage: "已就“${result.request.record_title}”退回给申请人补充材料", params: { comment: { label: "需要申请人补充什么？" @@ -181,7 +181,7 @@ export const zhCNObjects: NonNullable = { }, approval_remind: { label: "催办", - successMessage: "已发送催办。", + successMessage: "已就“${result.request.record_title}”发送催办", params: { comment: { label: "备注" @@ -191,7 +191,7 @@ export const zhCNObjects: NonNullable = { approval_recall: { label: "撤回", description: "撤回该请求？撤回后审批人将无法继续处理，记录随即解锁。", - successMessage: "已撤回。", + successMessage: "已撤回“${result.request.record_title}”", params: { comment: { label: "备注" @@ -200,7 +200,7 @@ export const zhCNObjects: NonNullable = { }, approval_resubmit: { label: "重新提交", - successMessage: "已重新提交。", + successMessage: "已重新提交“${result.request.record_title}”", params: { comment: { label: "你修改了什么？" @@ -209,7 +209,7 @@ export const zhCNObjects: NonNullable = { }, approval_comment: { label: "回复", - successMessage: "已发送回复。", + successMessage: "已在“${result.request.record_title}”上发送回复", params: { comment: { label: "回复内容" From dd9bb74c5938a4f35cc6f33bdd6cfb84017f52ee Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 08:33:41 +0000 Subject: [PATCH 7/7] chore(changeset): name every request action's copy and the delegated-position user Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude --- ...erver-authored-ui-text-names-what-and-who.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.changeset/23157-server-authored-ui-text-names-what-and-who.md b/.changeset/23157-server-authored-ui-text-names-what-and-who.md index 36d148a7393..94a9e905762 100644 --- a/.changeset/23157-server-authored-ui-text-names-what-and-who.md +++ b/.changeset/23157-server-authored-ui-text-names-what-and-who.md @@ -3,11 +3,20 @@ "@objectstack/plugin-security": patch --- -fix: the approvals drawer's Approve toast names the approved request, and Explain access names the principal as a person +fix: the approval request actions' success toasts name the request, and Explain access names the principal as a person Clause-②: no -Two server-authored strings that the console renders verbatim now name what and who they are about. +Server-authored strings that the console renders verbatim now name what and who they are about. -- `@objectstack/plugin-approvals`: the `approval_approve` action's `successMessage` was the bare verb `Approved.`. It is now `Approved "${result.request.record_title}"`, the wording of the inbox row's inline Approve. The decide route already answers the read-back request with its display enrichment, so `record_title` is the title the inbox shows (the target record's display field, else its payload snapshot). The zh-CN, ja-JP and es-ES values of the same key are updated to carry the same token. No key, route or response shape changes. -- `@objectstack/plugin-security`: the explain engine's principal layer said `Principal USER_ID holds position(s) …`. It now names the person first and the id second, for example `Principal Ada Lovelace (ada@example.com; id usr_1) holds position(s) …`, and an on-behalf-of delegator is named the same way. The user row is read with the asker's own context, so row-level and field-level security apply. An asker whose read returns no row gets the bare id, as before, and a field its read masks is left out. As a result, two askers can now be shown different names for the same principal. The decision's `principal` object and every verdict are unchanged. +- `@objectstack/plugin-approvals`: every action on `sys_approval_request` had a bare-verb `successMessage` (`Approved.`, `Rejected.`, `Reassigned.` and so on). Each now names the request through `${result.request.record_title}`. Each action's route answers `{ request, … }` with the read-back request and its display enrichment, so `record_title` is the title the inbox shows: the target record's display field, else its payload snapshot. The new copies: + - Approve and Reject use the inbox row's inline wording: `Approved "TITLE"` and `Rejected "TITLE"`. + - The others read `Reassigned "TITLE"`, `Sent "TITLE" back for revision`, `Information requested on "TITLE"`, `Reminder sent for "TITLE"`, `Recalled "TITLE"`, `Resubmitted "TITLE"` and `Reply posted on "TITLE"`. + + The zh-CN, ja-JP and es-ES values of the same keys carry the same token. No key, route or response shape changes. +- `@objectstack/plugin-security`: the explain engine's principal layer said `Principal USER_ID holds position(s) …`. It now names the person first and the id second, for example `Principal Ada Lovelace (ada@example.com; id usr_1) holds position(s) …`. Three other users in the same layer are named the same way: + - an on-behalf-of delegator; + - the user an ADR-0091 D3 delegated position came from, in the clause; + - that same user in the position's contributor attribution (`delegation from …`). + + Each user row is read with the asker's own context, so row-level and field-level security apply. An asker whose read returns no row gets the bare id, as before, and a field its read masks is left out. As a result, two askers can now be shown different names for the same user. The decision's `principal` object and every verdict are unchanged.