diff --git a/.changeset/26181-approval-notification-record-title.md b/.changeset/26181-approval-notification-record-title.md new file mode 100644 index 00000000000..4d117ead2f5 --- /dev/null +++ b/.changeset/26181-approval-notification-record-title.md @@ -0,0 +1,10 @@ +--- +'@objectstack/plugin-approvals': patch +--- + +fix(plugin-approvals): an approval notification names the record by its title, as the recipient may read it + +Every approval notification body named its record by the object's label and the record id (`Your Contract C8Mp7TjRYta8mOWx was approved.`), so a recipient could not tell which record it was without opening it. Each notification is delivered to one named recipient, so it now names the record by its display title (ADR-0079 `nameField`) as that recipient may read it: the inbox's own rule (the record read under the system context, the title picked from it as served to the reader, narrowed by field-level security), asked as the recipient, whose envelope is resolved through `resolveUserAuthzGrants` in the request's organization. It falls back to the record id when the title is empty, when the recipient may not read the title field or is served it masked, and when anything on the way fails. A notification that names one record to several recipients goes out once per name they read, so no recipient is handed another recipient's title. The message templates keep their `{{object}} {{record}}` shape. The token-bearing one-tap action pages keep naming the record by its id: they have no reader to narrow for. + +Clause-②: no +Metadata-protocol: none diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index b6ff0ba2bff..976c137ce33 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -565,9 +565,13 @@ or that report the decision to anyone but the submitter. **The decision tells the submitter.** When a decision ends the approval, the submitter of the request it decides gets one notification: `approval.approved` ("Request approved") or `approval.rejected` ("Request rejected"). Its body -names the record (`Deal D_123`) and the outcome, followed by the decider's -comment when they wrote one, and it links to that request in the Approvals -Inbox. There is one topic per outcome, so a delivery preference can mute +names the record and the outcome, followed by the decider's comment when they +wrote one, and it links to that request in the Approvals Inbox. Like every +approval notification, it names the record by its object's label and its +title as that recipient may read it (`Deal Acme renewal`), and by its id +(`Deal D_123`) when the title is empty or the recipient's field-level security +hides the title field. The one-tap email action pages always name the record +by its id, because they are opened with a token rather than a session. There is one topic per outcome, so a delivery preference can mute approvals and keep rejections. "Ends the approval" means the process, not the step. A co-sign vote that diff --git a/packages/plugins/plugin-approvals/src/action-link-pages.ts b/packages/plugins/plugin-approvals/src/action-link-pages.ts index 4123cfb893a..b3aebf46c71 100644 --- a/packages/plugins/plugin-approvals/src/action-link-pages.ts +++ b/packages/plugins/plugin-approvals/src/action-link-pages.ts @@ -57,8 +57,13 @@ function shell(title: string, body: string): string { * (#10749). These pages have no reader to narrow for: the bearer carries a * token, not a session, and the token chain reads the request under the system * context, which field-level security does not narrow. So no subject field is - * shown here — the record is named the way the approval notification names it, - * by the object's label and the record's id. + * shown here — the record is named by the object's label and the record's id. + * + * [#26181, 2026-10-27] The notification surfaces were re-ruled (ruling A′): + * an approval notification is rendered for one named recipient, so it names + * the record by its title as that recipient may read it, else its id + * (`ApprovalService.subjectNamesFor`). These pages keep the id: the reason + * above, no reader to narrow for, still holds here. */ function summaryRows(req: ApprovalRequestRow): string { const rows: Array<[string, string]> = [ diff --git a/packages/plugins/plugin-approvals/src/approval-decision-outcome-notification.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-decision-outcome-notification.integration.test.ts index fb365507a43..16970e3e4ea 100644 --- a/packages/plugins/plugin-approvals/src/approval-decision-outcome-notification.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-decision-outcome-notification.integration.test.ts @@ -234,7 +234,7 @@ describe('the decision that ends an approval tells its submitter (approval.appro expect(told.map((e) => [e.topic, e.audience])).toEqual([['approval.approved', ['u_sub']]]); expect(told[0].payload).toEqual({ title: 'Request approved', - body: 'Your Deal D_won was approved.', + body: 'Your Deal won was approved.', actionUrl: `/system/approvals?request=${encodeURIComponent(req.id)}`, }); expect(told[0].source).toEqual({ object: 'sys_approval_request', id: req.id }); @@ -251,7 +251,7 @@ describe('the decision that ends an approval tells its submitter (approval.appro expect(told.map((e) => [e.topic, e.audience])).toEqual([['approval.rejected', ['u_sub']]]); expect(told[0].payload).toEqual({ title: 'Request rejected', - body: `Your Deal D_lost was rejected.\n\n${REJECT_NOTE}`, + body: `Your Deal lost was rejected.\n\n${REJECT_NOTE}`, actionUrl: `/system/approvals?request=${encodeURIComponent(req.id)}`, }); expect(told[0].actorId).toBe('u_alice'); diff --git a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts index d93bd7cab4b..f6905fbef97 100644 --- a/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts @@ -39,15 +39,17 @@ * * ## And how it names the record (#23978) * - * Ruling B of #23556 / #23594 names the record an approval notification is - * about by its object's label and its id (`Deal D_123`), with no title, through - * `nameSubjectRecord`. `approval.requested` used it; every other body that + * Every body names the record an approval notification is about by its + * object's label, the way `approval.requested` did; every other body that * names the record built `/`, so an approver read the * API name and a slash in the reminder, the SLA breach, the escalation, the * hand-off, both send-back sentences and both out-of-office sentences. The same * battery pins the record's name in each of those bodies, and a control: a * record of an object whose schema has no label is named by the object's API - * name and the id, the helper's own fallback. + * name, the helper's own fallback. [#26181 ruling A′] The record itself is + * named by its title as the recipient is served it (here every recipient: no + * field-visibility source is composed), once ruling B's id + * (`notification-subject-title-per-recipient` pins the narrowing). * * ## And whom the out-of-office bodies name (#24065) * @@ -343,29 +345,30 @@ describe('every approval notification carries its text in body', () => { expect([sentBack.audience, sentBack.payload?.body]).toEqual([['u_sub'], SEND_BACK]); // [#24753] The rejection note follows the outcome sentence, which names the record. const rejected = one('approval.rejected', 'rejected'); - expect([rejected.audience, rejected.payload?.body]).toEqual([['u_sub'], `Your Deal D_rejected was rejected.\n\n${REJECTION}`]); + expect([rejected.audience, rejected.payload?.body]).toEqual([['u_sub'], `Your Deal rejected was rejected.\n\n${REJECTION}`]); }); - it('[#23978] every body that names the record names it by its object label and its id, as approval.requested does', () => { - // Ruling B of #23556 / #23594: label and id, no title — one form for every - // body, the one `approval.requested` already sends. The pin is the record's - // name in each body, not the rest of the sentence around it. + it('[#23978 / #26181] every body that names the record names it by its object label and its title, as approval.requested does', () => { + // One form for every body, the one `approval.requested` sends: the + // object's label and the record's title as its recipient is served it + // (ruling A′, which re-ruled #23556 / #23594 B's id). The pin is the + // record's name in each body, not the rest of the sentence around it. const NAMED: Record = { - 'approval.requested@thread': 'Deal D_thread', - 'approval.reminder@thread': 'Deal D_thread', - 'approval.sla_breached@sla_notify': 'Deal D_sla_notify', - 'approval.sla_breached@sla_notify>submitter': 'Deal D_sla_notify', - 'approval.escalated@sla_reassign': 'Deal D_sla_reassign', - 'approval.reassigned@thread': 'Deal D_thread', - 'approval.returned@budget': 'Deal D_budget', - 'approval.returned@rework': 'Deal D_rework', - 'approval.ooo_substituted@ooo': 'Deal D_ooo', - 'approval.ooo_skipped@ooo': 'Deal D_ooo', - 'approval.approved@approved': 'Deal D_approved', - 'approval.rejected@rejected': 'Deal D_rejected', + 'approval.requested@thread': 'Deal thread', + 'approval.reminder@thread': 'Deal thread', + 'approval.sla_breached@sla_notify': 'Deal sla_notify', + 'approval.sla_breached@sla_notify>submitter': 'Deal sla_notify', + 'approval.escalated@sla_reassign': 'Deal sla_reassign', + 'approval.reassigned@thread': 'Deal thread', + 'approval.returned@budget': 'Deal budget', + 'approval.returned@rework': 'Deal rework', + 'approval.ooo_substituted@ooo': 'Deal ooo', + 'approval.ooo_skipped@ooo': 'Deal ooo', + 'approval.approved@approved': 'Deal approved', + 'approval.rejected@rejected': 'Deal rejected', // The control: the object's schema has no label, so the record is named - // by the object's API name and the id. - 'approval.reminder@bare': 'crm_memo D_bare', + // by the object's API name and its title. + 'approval.reminder@bare': 'crm_memo bare', }; const offenders = Object.entries(NAMED).flatMap(([site, name]) => { const bodies = emits.filter((e) => siteOf(e) === site).map((e) => String(e.payload?.body)); diff --git a/packages/plugins/plugin-approvals/src/approval-notification-keyed-text.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-notification-keyed-text.integration.test.ts index 0f08bf82f5f..6d94be6a778 100644 --- a/packages/plugins/plugin-approvals/src/approval-notification-keyed-text.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-notification-keyed-text.integration.test.ts @@ -277,8 +277,10 @@ describe('#24978 — every approval notification carries its text as catalog key if (out.body === SENTINEL.body || out.body === e.payload?.body || out.body.includes('{{')) { offenders.push(`${site} ${locale} body: ${out.body}`); } - // Every body sentence names the record; in this locale, by this locale's label. - if (!out.body.includes(`${DEAL_LABELS[locale]} D_`)) offenders.push(`${site} ${locale} body names no localized record: ${out.body}`); + // Every body sentence names the record; in this locale, by this locale's + // label, and [#26181] by its title (each deal is titled by its key). + const title = site.split('@')[1]?.replace('>submitter', ''); + if (!out.body.includes(`${DEAL_LABELS[locale]} ${title}`)) offenders.push(`${site} ${locale} body names no localized record: ${out.body}`); } } expect(offenders).toEqual([]); @@ -344,18 +346,18 @@ describe('#24978 — end to end: a real messaging service renders each approver\ }, asUser('u_sub')); expect(await inboxOf('u_zh')).toEqual([ - { topic: 'approval.requested', title: '新的审批请求', body: '商机 D_1 正在等待您的审批决定。' }, + { topic: 'approval.requested', title: '新的审批请求', body: '商机 Big deal 正在等待您的审批决定。' }, ]); expect(await inboxOf('u_en')).toEqual([ - { topic: 'approval.requested', title: 'Approval requested', body: 'A decision on Deal D_1 is waiting on you.' }, + { topic: 'approval.requested', title: 'Approval requested', body: 'A decision on Deal Big deal is waiting on you.' }, ]); expect(await inboxOf('u_none')).toEqual([ - { topic: 'approval.requested', title: '承認依頼', body: '商談 D_1 について、あなたの承認判断が必要です。' }, + { topic: 'approval.requested', title: '承認依頼', body: '商談 Big deal について、あなたの承認判断が必要です。' }, ]); await svc.decide(request.id, { decision: 'approve', actorId: 'u_en', comment: 'Looks good — ship it.' }, asUser('u_en')); expect(await inboxOf('u_sub')).toEqual([ - { topic: 'approval.approved', title: '请求已批准', body: '您的商机 D_1 已获批准。\n\nLooks good — ship it.' }, + { topic: 'approval.approved', title: '请求已批准', body: '您的商机 Big deal 已获批准。\n\nLooks good — ship it.' }, ]); }); }); diff --git a/packages/plugins/plugin-approvals/src/approval-notification-text.ts b/packages/plugins/plugin-approvals/src/approval-notification-text.ts index cf2cf8411a7..bcd99e66ccf 100644 --- a/packages/plugins/plugin-approvals/src/approval-notification-text.ts +++ b/packages/plugins/plugin-approvals/src/approval-notification-text.ts @@ -27,8 +27,11 @@ * ## The record a notification names * * `object` and `record` ({@link subjectParams}): the object's label and the - * record id, the #23556 / #23594 ruling B form, never a title. The label rides - * as a keyed param — `objects..label`, the key the reader-language + * record's name. [#26181 ruling A′] The name is the record's display title as + * the one recipient the text is built for may read it, else the record id + * (`ApprovalService.notify` resolves it per recipient); the #23556 / #23594 + * ruling B form, label and id for every reader, stands only on the + * reader-less action-link pages. The label rides as a keyed param — `objects..label`, the key the reader-language * labels (`reader-locale-labels.ts`) and objectql's refusals name an object * by — so a recipient whose language pack holds it reads it in their language; * the schema label (the API name when there is none) stands otherwise. @@ -75,18 +78,19 @@ export function approvalMessageKey(id: ApprovalMessageId): string { } /** - * The record `objectName`/`recordId` names: the object's label — keyed by - * `objects..label`, falling back to `objectLabel` (the schema label), - * else the API name — and the record id. + * The record of `objectName` a notification names: the object's label — keyed + * by `objects..label`, falling back to `objectLabel` (the schema + * label), else the API name — and `record`, the name its recipient reads for + * it (its title as they may read it, else its id). */ export function subjectParams( objectName: string, objectLabel: string | undefined, - recordId: string, + record: string, ): ApprovalSubjectParams { return { object: { key: objectLabelKey(objectName), fallback: objectLabel ?? objectName }, - record: recordId, + record, }; } diff --git a/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts b/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts index 497df05668a..9652f8078b3 100644 --- a/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-requested-notification.integration.test.ts @@ -21,9 +21,11 @@ * approver on the slate it opened on — a named user and every holder of a * staffed position alike — through the same `notify()` ingress and payload * shape as `approval.reminder`; - * - [#23556] the body names the record by its object's label and its id, and - * [#23594] carries no title, even for a record whose title resolves — not - * the live record's and not the submit snapshot's; + * - [#23556] the body names the record by its object's label, and [#26181] + * by the title the inbox serves its recipient — the live record's, never + * the submit snapshot's (with no field-visibility source composed, every + * recipient is served it; `notification-subject-title-per-recipient` pins + * the narrowing); * - a person the slate lists twice is one person, and a `type:value` slot * literal (an unstaffed position) names no one; * - the control: a step whose approvers resolve to nobody notifies no one, @@ -192,13 +194,13 @@ describe('opening an approval step tells each resolved approver, once (approval. // One message per person, each addressed to that person alone, in the // reminder's shape: the request as its source, the inbox deep-linked to - // it, the submitter as the actor. [#23556] The body names the record by - // its object's label and its id, never `crm_deal/D_ladder`. + // it, the submitter as the actor. [#23556 / #26181] The body names the + // record by its object's label and its title, never `crm_deal/D_ladder`. for (const e of requested(row.id)) { expect(e.audience).toHaveLength(1); expect(e.payload).toEqual({ title: 'Approval requested', - body: `A decision on Deal D_ladder is waiting on you.`, + body: `A decision on Deal ladder is waiting on you.`, actionUrl: `/system/approvals?request=${encodeURIComponent(row.id)}`, }); expect(e.source).toEqual({ object: 'sys_approval_request', id: row.id }); @@ -207,12 +209,12 @@ describe('opening an approval step tells each resolved approver, once (approval. expect(new Set(requested(row.id).map((e) => e.dedupKey)).size).toBe(3); }); - it('[#23594] a record whose title resolves is still named by label and id — no title reaches the body', async () => { + it('[#26181] a record whose title resolves is named by the title the inbox serves its recipient, never the snapshot\'s', async () => { // The inbox narrows the subject title by its reader's field-level security - // (#23594), but a notification has no reader to narrow for, and its text is - // stored and may be e-mailed, so the body withholds the title from EVERY - // source: the live record's and the submit snapshot's (which differ here, - // so either leak is told apart). + // (#23594), and a notification is rendered for one recipient, so its body + // names the record by the title that recipient is served: the live + // record's, never the submit snapshot's (which differ here, so the source + // is told apart). const LIVE_TITLE = 'Acme renewal'; const SNAPSHOT_TITLE = 'Snapshot-only title'; await engine.insert('crm_deal', { id: 'D_titled', title: LIVE_TITLE }, { context: SYSTEM } as any); @@ -221,16 +223,15 @@ describe('opening an approval step tells each resolved approver, once (approval. config: { approvers: [{ type: 'user', value: 'u_alice' }], behavior: 'first_response' } as any, submitterId: 'u_sub', record: { id: 'D_titled', title: SNAPSHOT_TITLE }, }, asUser('u_sub')); - // The scene: both titles really are there to leak — the inbox resolves the - // live one for this very recipient, and the snapshot carries its own. + // The scene: both titles are there — the inbox resolves the live one for + // this very recipient, and the snapshot carries its own. const [listed] = await svc.listRequests({ approverId: 'u_alice', recordId: 'D_titled' }, asUser('u_alice')); expect(listed?.record_title).toBe(LIVE_TITLE); expect(row.payload).toMatchObject({ title: SNAPSHOT_TITLE }); const [message] = requested(row.id); expect(message?.audience).toEqual(['u_alice']); - expect(message?.payload?.body).toBe('A decision on Deal D_titled is waiting on you.'); - expect(String(message?.payload?.body)).not.toContain(LIVE_TITLE); + expect(message?.payload?.body).toBe(`A decision on Deal ${LIVE_TITLE} is waiting on you.`); expect(String(message?.payload?.body)).not.toContain(SNAPSHOT_TITLE); }); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index fb61622d006..fbf728e7166 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -73,8 +73,10 @@ import { omitInternalFieldsFromWriteResponse, resolveReadableParentIds, resolveUserAuthzGrants, + vetOrganizationClaim, withoutOperationPrivateKeys, } from '@objectstack/core'; +import type { TenancyPosture } from '@objectstack/spec/security'; import { filterApproversWhoCanRead, resolveApproverDirectoryOrg, @@ -411,6 +413,26 @@ function approvalNotice(text: ApprovalNotificationText): { }; } +/** + * [#26181] What an approval notification says, as `notify()` takes it: + * + * - a notice that names no record (a person's own words under a keyed + * headline) — its {@link approvalNotice} as built; + * - a notice about a record — the record (`about`) and the sentence built + * around the name each recipient reads for it (`text`). The name is the + * recipient's to read (ruling A′): the record's title as they may read it, + * else its id ({@link ApprovalService.subjectNamesFor}), so the sentence is + * built once per name, never once for everyone. + */ +type ApprovalNoticeInput = + | (ReturnType & { readonly about?: never; readonly text?: never }) + | { + readonly about: { readonly object: string; readonly recordId: string }; + readonly text: (subject: ApprovalSubjectParams) => ApprovalNotificationText; + readonly payload?: never; + readonly keyedText?: never; + }; + /** Minimum time between submitter reminders on one request. */ export const REMIND_COOLDOWN_MS = 4 * 60 * 60 * 1000; @@ -1807,7 +1829,6 @@ export class ApprovalService implements IApprovalService { private async notify(input: { topic: string; audience: string[]; - payload: ApprovalNotificationPayload; /** * [#23284] The organization of the request the notification is about — * the request row's own `organization_id` (`requestOrg` while the request @@ -1819,48 +1840,70 @@ export class ApprovalService implements IApprovalService { * approver acting from another organization does not move the request. */ organizationId: string | null | undefined; - /** - * [#24978] The text as catalog keys, from {@link approvalNotificationText} - * — the same call that builds `payload.title` / `payload.body`, so the - * literal and its keys cannot name two different sentences. REQUIRED, so - * no call site can send an approval notification that only ever reads in - * English. - */ - keyedText: ApprovalKeyedText; dedupKey?: string; source?: { object: string; id: string }; actorContext?: ExecutionContext; - }): Promise { - const { actorContext, organizationId, ...event } = input; + } & ApprovalNoticeInput): Promise { + // [#24978] The text is REQUIRED in one of its two shapes, each carrying the + // catalog keys beside the literal (`approvalNotificationText` builds both), + // so no call site can send an approval notification that only ever reads + // in English. + const { actorContext, organizationId, about, text, payload: built, keyedText, ...event } = input; const organization = organizationId != null && String(organizationId) !== '' ? String(organizationId) : undefined; const audience = event.audience.filter(a => a && !a.includes(':')); if (!this.messaging || !audience.length) return 0; const person = actorContext ? recordedActor(actorContext) : null; - // Deep-link the inbox (#2678 P1.5): a notification about one request should - // land on that request, not the bare inbox. Rewritten centrally so every - // call site — and any future one — inherits it; the query param is read by - // the console inbox to auto-open the drawer. - let payload = event.payload; - if ( - payload.actionUrl === '/system/approvals' - && event.source?.object === 'sys_approval_request' - && event.source.id - ) { - payload = { ...payload, actionUrl: `/system/approvals?request=${encodeURIComponent(event.source.id)}` }; + // [#26181] A notice about a record goes out once per NAME its recipients + // read for it: the recipients who read the same name share one emit, and a + // recipient never receives a text built around another recipient's title. + const parts: Array<{ audience: string[]; payload: ApprovalNotificationPayload; keyedText: ApprovalKeyedText }> = []; + if (about && text) { + const names = await this.subjectNamesFor(about, audience, organization); + const byName = new Map(); + for (const recipient of audience) { + const name = names.get(recipient) ?? about.recordId; + byName.set(name, [...(byName.get(name) ?? []), recipient]); + } + const label = this.objectLabelOf(about.object); + for (const [name, recipients] of byName) { + parts.push({ audience: recipients, ...approvalNotice(text(subjectParams(about.object, label, name))) }); + } + } else if (built && keyedText) { + parts.push({ audience, payload: built, keyedText }); } - try { - await this.messaging.emit({ - severity: 'info', ...event, payload, audience, ...(person ? { actorId: person } : {}), - ...(organization ? { organizationId: organization } : {}), - }); - return audience.length; - } catch (err: any) { - this.logger?.warn?.('[approvals] notification failed', { - topic: event.topic, error: err?.message ?? String(err), - }); - return 0; + let notified = 0; + for (const part of parts) { + // Deep-link the inbox (#2678 P1.5): a notification about one request should + // land on that request, not the bare inbox. Rewritten centrally so every + // call site — and any future one — inherits it; the query param is read by + // the console inbox to auto-open the drawer. + let payload = part.payload; + if ( + payload.actionUrl === '/system/approvals' + && event.source?.object === 'sys_approval_request' + && event.source.id + ) { + payload = { ...payload, actionUrl: `/system/approvals?request=${encodeURIComponent(event.source.id)}` }; + } + // A dedup key names ONE event: a notice split per name keys each part by + // its first recipient, so messaging never folds one part into another. + const dedupKey = event.dedupKey && parts.length > 1 ? `${event.dedupKey}-${part.audience[0]}` : event.dedupKey; + try { + await this.messaging.emit({ + severity: 'info', ...event, payload, keyedText: part.keyedText, audience: part.audience, + ...(dedupKey ? { dedupKey } : {}), + ...(person ? { actorId: person } : {}), + ...(organization ? { organizationId: organization } : {}), + }); + notified += part.audience.length; + } catch (err: any) { + this.logger?.warn?.('[approvals] notification failed', { + topic: event.topic, error: err?.message ?? String(err), + }); + } } + return notified; } /** Load a request row and assert it is still pending. */ @@ -4010,6 +4053,8 @@ export class ApprovalService implements IApprovalService { // `reassign` hand-off's shape), not frozen into an audit row. const oooNames = await this.resolveUserNames(substitutions.flatMap((sub) => [sub.from, sub.to])); const oooPerson = (userId: string): string => oooNames.get(userId) || userId; + // [#26181] The record each body names is the recipient's to read (`notify()`). + const subjectRecord = { object: input.object, recordId: input.recordId }; for (const sub of substitutions) { await this.engine.insert('sys_approval_action', await this.actionRow(row, { id: uid('aact'), request_id: id, organization_id: requestOrg, @@ -4018,21 +4063,17 @@ export class ApprovalService implements IApprovalService { comment: `${sub.from} → ${sub.to}${sub.reason ? ` — ${sub.reason}` : ''}`, created_at: now, }), { context: SYSTEM_CTX }); - const covering = approvalNotificationText({ - title: 'approvalOooSubstitutedTitle', body: 'approvalOooSubstitutedBody', - params: { ...this.nameSubjectRecord(input.object, input.recordId), person: oooPerson(sub.from) }, - }); await this.notify({ topic: 'approval.ooo_substituted', organizationId: requestOrg, audience: [sub.to], source: { object: 'sys_approval_request', id }, dedupKey: `approval-ooo-${id}-${sub.to}`, - ...approvalNotice(covering), - }); - const skipped = approvalNotificationText({ - title: 'approvalOooSkippedTitle', body: 'approvalOooSkippedBody', - params: { ...this.nameSubjectRecord(input.object, input.recordId), person: oooPerson(sub.to) }, + about: subjectRecord, + text: (subject) => approvalNotificationText({ + title: 'approvalOooSubstitutedTitle', body: 'approvalOooSubstitutedBody', + params: { ...subject, person: oooPerson(sub.from) }, + }), }); await this.notify({ topic: 'approval.ooo_skipped', @@ -4040,7 +4081,11 @@ export class ApprovalService implements IApprovalService { audience: [sub.from], source: { object: 'sys_approval_request', id }, dedupKey: `approval-ooo-skip-${id}-${sub.from}`, - ...approvalNotice(skipped), + about: subjectRecord, + text: (subject) => approvalNotificationText({ + title: 'approvalOooSkippedTitle', body: 'approvalOooSkippedBody', + params: { ...subject, person: oooPerson(sub.to) }, + }), }); } @@ -4088,18 +4133,15 @@ export class ApprovalService implements IApprovalService { // No one-tap decision links: a notification payload is stored, and a raw // ADR-0043 token in it is a usable link at rest (`ApprovalNotificationPayload`). // - // [#23556] The body names the record by its object's label and its id — - // `Deal D_123`, never `crm_deal/D_123` — so a grouped bell entry tells its - // reader which kind of record is waiting. One name for every recipient. + // [#23556] The body names the record by its object's label — never + // `crm_deal/...` — so a grouped bell entry tells its reader which kind of + // record is waiting; [#26181] and by its title as each approver may read + // it, else its id (`notify()`). const toldByOoo = new Set(substitutions.map((sub) => sub.to)); const openedOn = new Set(approvers.filter((a) => a && !a.includes(':') && !toldByOoo.has(a))); // // [#24978] And it rides as catalog keys beside that literal, so each // approver reads it in their own language, rendered at delivery. - const requested = approvalNotificationText({ - title: 'approvalRequestedTitle', body: 'approvalRequestedBody', - params: this.nameSubjectRecord(input.object, input.recordId), - }); for (const approver of openedOn) { await this.notify({ topic: 'approval.requested', @@ -4108,7 +4150,10 @@ export class ApprovalService implements IApprovalService { actorContext: context, source: { object: 'sys_approval_request', id }, dedupKey: `approval-requested-${id}-${approver}`, - ...approvalNotice(requested), + about: { object: input.object, recordId: input.recordId }, + text: (subject) => approvalNotificationText({ + title: 'approvalRequestedTitle', body: 'approvalRequestedBody', params: subject, + }), }); } @@ -4813,7 +4858,7 @@ export class ApprovalService implements IApprovalService { * outcome and keep the other. * * The shape is the sibling notices': addressed to the request's own - * `submitter_id`, the record named by {@link ApprovalService.nameSubjectRecord} + * `submitter_id`, the record named by {@link ApprovalService.subjectNamesFor} * (#23556), a non-empty body (#21847), deep-linked to the request by * `notify()`, the decider as the actor. The body is one English sentence, * like every approval notification — none of them reads the caller's locale; @@ -4856,24 +4901,23 @@ export class ApprovalService implements IApprovalService { if (nextStepOpened) return; } const approved = decision === 'approve'; - const subject = this.nameSubjectRecord(request.object_name, request.record_id); const note = comment?.trim(); // [#24978] The decider's comment, when there is one, rides under the // sentence as written — a hole of its own, never translated. - const outcome = approvalNotificationText({ - title: approved ? 'approvalApprovedTitle' : 'approvalRejectedTitle', - body: note - ? (approved ? 'approvalApprovedWithCommentBody' : 'approvalRejectedWithCommentBody') - : (approved ? 'approvalApprovedBody' : 'approvalRejectedBody'), - params: note ? { ...subject, comment: note } : subject, - }); await this.notify({ topic: approved ? 'approval.approved' : 'approval.rejected', organizationId: request.organization_id, audience: [submitter], actorContext: context, source: { object: 'sys_approval_request', id: request.id }, - ...approvalNotice(outcome), + about: { object: request.object_name, recordId: request.record_id }, + text: (subject) => approvalNotificationText({ + title: approved ? 'approvalApprovedTitle' : 'approvalRejectedTitle', + body: note + ? (approved ? 'approvalApprovedWithCommentBody' : 'approvalRejectedWithCommentBody') + : (approved ? 'approvalApprovedBody' : 'approvalRejectedBody'), + params: note ? { ...subject, comment: note } : subject, + }), }); } @@ -5296,7 +5340,7 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); const maxRevisions = typeof (config as any).maxRevisions === 'number' ? (config as any).maxRevisions : 3; // [#23978] Both sentences below name the record the way every approval notification does. - const subject = this.nameSubjectRecord(raw.object_name, raw.record_id); + const about = { object: String(raw.object_name), recordId: String(raw.record_id) }; let priorSendBacks = 0; if (runId && nodeId) { const siblings = await this.engine.find('sys_approval_request', { @@ -5348,17 +5392,17 @@ export class ApprovalService implements IApprovalService { resumeError = outcome.resumeError; } if (raw.submitter_id) { - const autoRejected = approvalNotificationText({ - title: 'approvalAutoRejectedTitle', body: 'approvalAutoRejectedBody', - params: { ...subject, limit: String(maxRevisions) }, - }); await this.notify({ topic: 'approval.returned', organizationId: raw.organization_id, audience: [String(raw.submitter_id)], actorContext: context, source: { object: 'sys_approval_request', id: requestId }, - ...approvalNotice(autoRejected), + about, + text: (subject) => approvalNotificationText({ + title: 'approvalAutoRejectedTitle', body: 'approvalAutoRejectedBody', + params: { ...subject, limit: String(maxRevisions) }, + }), }); } const fresh = await this.readBackRequest(requestId, context); @@ -5394,16 +5438,21 @@ export class ApprovalService implements IApprovalService { if (raw.submitter_id) { // [#24978] The approver's own words, when they wrote some, are the body // as written; only the headline is keyed then. - const sentBack = input.comment?.trim() - ? approvalNotificationText({ title: 'approvalReturnedTitle', literalBody: input.comment.trim() }) - : approvalNotificationText({ title: 'approvalReturnedTitle', body: 'approvalReturnedBody', params: subject }); + const note = input.comment?.trim(); await this.notify({ topic: 'approval.returned', organizationId: raw.organization_id, audience: [String(raw.submitter_id)], actorContext: context, source: { object: 'sys_approval_request', id: requestId }, - ...approvalNotice(sentBack), + ...(note + ? approvalNotice(approvalNotificationText({ title: 'approvalReturnedTitle', literalBody: note })) + : { + about, + text: (subject: ApprovalSubjectParams) => approvalNotificationText({ + title: 'approvalReturnedTitle', body: 'approvalReturnedBody', params: subject, + }), + }), }); } @@ -5659,10 +5708,6 @@ export class ApprovalService implements IApprovalService { }, { context: SYSTEM_CTX }); await this.syncApproverIndex(requestId, next, raw.organization_id ?? null, now); - const handedOver = approvalNotificationText({ - title: 'approvalReassignedTitle', body: 'approvalReassignedBody', - params: this.nameSubjectRecord(raw.object_name, raw.record_id), - }); await this.notify({ topic: 'approval.reassigned', organizationId: raw.organization_id, @@ -5670,7 +5715,10 @@ export class ApprovalService implements IApprovalService { actorContext: context, source: { object: 'sys_approval_request', id: requestId }, dedupKey: `approval-reassign-${requestId}-${to}`, - ...approvalNotice(handedOver), + about: { object: String(raw.object_name), recordId: String(raw.record_id) }, + text: (subject) => approvalNotificationText({ + title: 'approvalReassignedTitle', body: 'approvalReassignedBody', params: subject, + }), }); const fresh = await this.readBackRequest(requestId, context); @@ -5813,7 +5861,7 @@ export class ApprovalService implements IApprovalService { // concrete identity (a user id / an email) gets its own reminder; // `position:*`-style literals go to the shared nudge below, which // `notify()` drops — a slot address names no one. - // [#23978] One name for every recipient, the form `approval.requested` uses. + // [#23978 / #26181] The record named as `approval.requested` names it. // // ⛔ [#24564] No one-tap decision link (ADR-0043) rides a reminder. // Messaging stores the payload it is handed (`ApprovalNotificationPayload`), @@ -5832,10 +5880,12 @@ export class ApprovalService implements IApprovalService { // reminders handed over: one per distinct address. A user id and the same // account's email are two addresses here, exactly as at the opening — two // slots the person takes separately, and two reminders delivered. - const reminder = approvalNotificationText({ - title: 'approvalReminderTitle', body: 'approvalReminderBody', - params: this.nameSubjectRecord(raw.object_name, raw.record_id), - }); + const reminder = { + about: { object: String(raw.object_name), recordId: String(raw.record_id) }, + text: (subject: ApprovalSubjectParams) => approvalNotificationText({ + title: 'approvalReminderTitle', body: 'approvalReminderBody', params: subject, + }), + }; let notified = 0; const concrete = new Set(pending.filter(a => a && !a.includes(':'))); const literals = pending.filter(a => a && a.includes(':')); @@ -5847,7 +5897,7 @@ export class ApprovalService implements IApprovalService { actorContext: context, source: { object: 'sys_approval_request', id: requestId }, dedupKey: `approval-remind-${requestId}-${nowIso}-${approver}`, - ...approvalNotice(reminder), + ...reminder, }); } if (literals.length) { @@ -5858,7 +5908,7 @@ export class ApprovalService implements IApprovalService { actorContext: context, source: { object: 'sys_approval_request', id: requestId }, dedupKey: `approval-remind-${requestId}-${nowIso}`, - ...approvalNotice(reminder), + ...reminder, }); } @@ -7171,7 +7221,7 @@ export class ApprovalService implements IApprovalService { const now = this.clock.now().toISOString(); const pending = csvSplit(raw.pending_approvers); // [#23978] Every escalation sentence names the record the way every approval notification does. - const subject = this.nameSubjectRecord(raw.object_name, raw.record_id); + const about = { object: String(raw.object_name), recordId: String(raw.record_id) }; // `escalateTo` is a position machine name or a user id (same contract as // the `position` ApproverType, ADR-0090 D3). Position holders win; an @@ -7245,9 +7295,10 @@ export class ApprovalService implements IApprovalService { organizationId: raw.organization_id, audience: escalatees, source: { object: 'sys_approval_request', id: raw.id }, - ...approvalNotice(approvalNotificationText({ + about, + text: (subject) => approvalNotificationText({ title: 'approvalEscalatedTitle', body: 'approvalEscalatedBody', params: subject, - })), + }), }); } else if (action === 'auto_approve' || action === 'auto_reject') { // [#24753] The submitter hears this outcome from the submitter notice @@ -7266,9 +7317,10 @@ export class ApprovalService implements IApprovalService { organizationId: raw.organization_id, audience: [...pending, ...escalatees], source: { object: 'sys_approval_request', id: raw.id }, - ...approvalNotice(approvalNotificationText({ + about, + text: (subject) => approvalNotificationText({ title: 'approvalSlaBreachedTitle', body: 'approvalSlaBreachedBody', params: subject, - })), + }), }); } @@ -7278,10 +7330,11 @@ export class ApprovalService implements IApprovalService { organizationId: raw.organization_id, audience: [String(raw.submitter_id)], source: { object: 'sys_approval_request', id: raw.id }, - ...approvalNotice(approvalNotificationText({ + about, + text: (subject) => approvalNotificationText({ title: 'approvalSlaBreachedSubmitterTitle', body: 'approvalSlaBreachedSubmitterBody', params: { ...subject, action: taken }, - })), + }), }); } } @@ -7668,8 +7721,8 @@ export class ApprovalService implements IApprovalService { * may be served still names the record. This narrows only; nothing here * grants a reader anything. * - * The `approval.requested` body ({@link ApprovalService.nameSubjectRecord}) - * does not read this: it has no reader to narrow for. + * [#26181] A notification body names the record by the same rule, asked as + * each recipient ({@link ApprovalService.subjectNamesFor}). */ private async resolveSubjectDisplays( byObject: ReadonlyMap>, @@ -7718,17 +7771,27 @@ export class ApprovalService implements IApprovalService { where: { id: { $in: list } }, limit: list.length, context: SYSTEM_CTX, }); for (const rec of (recs ?? []) as any[]) { - const asServed = readable === undefined ? rec : redactSnapshot(rec, readable).payload; - const title = ApprovalService.pickTitle(asServed, displayField); + const title = ApprovalService.titleAsServed(rec, displayField, readable); if (rec?.id && title) into.set(`${object} ${rec.id}`, title); } } + /** + * The title of `rec` as served to a reader whose served fields are + * `readable` (`undefined`: the seam's pass-through): picked from the record + * after `redactSnapshot` keeps only those fields. Shared by + * {@link titlesAsServed} and a notification's per-recipient name + * ({@link subjectNamesFor}). + */ + private static titleAsServed(rec: any, displayField: string | undefined, readable: string[] | undefined): string | undefined { + const asServed = readable === undefined ? rec : redactSnapshot(rec, readable).payload; + return ApprovalService.pickTitle(asServed, displayField); + } + /** * An object's schema label, or `undefined` — the one spelling of "the * object's label" the inbox row ({@link ApprovalService.resolveSubjectDisplays}) - * and the `approval.requested` body ({@link ApprovalService.nameSubjectRecord}) - * share. A schema that cannot be read has no label (the label is optional). + * and every notification body (`notify()`) share. A schema that cannot be read has no label (the label is optional). */ private objectLabelOf(object: string): string | undefined { try { @@ -7738,27 +7801,100 @@ export class ApprovalService implements IApprovalService { } /** - * [#23556] The record an approval notification is about: the object's label - * (its API name only when the schema has none, as the inbox falls back), - * then the record id — `Deal D_123`. The body carried the id before; what - * changed is the API name giving way to the label. - * - * ⛔ No title, from any source — neither the live record nor the payload - * snapshot (which the inbox serves only after redacting it for its reader, - * #10749). Notification text is stored and may be e-mailed, and it has no - * reader to narrow for. - * - * [#24978] Answered as the two `{{object}} {{record}}` holes of the keyed - * sentence rather than one string: the label is keyed by - * `objects..label`, so a recipient whose language pack holds it - * reads it in their language at delivery, and the schema label (the API name - * when there is none) is the text it falls back to — the literal, which is - * exactly the `Deal D_123` this named before. + * [#26181 ruling A′] The name each of `recipients` reads for the record + * `about` names in a notification body, the `{{record}}` beside the object's + * label: its display title (ADR-0079 `nameField`, + * {@link resolveDisplayField}) as THAT recipient may read it, else the + * record id. Each notification is delivered to one named recipient — the + * bell row and the e-mail alike — so, unlike the token-bearing action-link + * pages (`action-link-pages.ts`), it has a reader to narrow for. + * + * The inbox's own rule, asked as the recipient rather than the reading + * caller ({@link resolveSubjectDisplays}): the record is read under the + * system context (#7497 ruling C), and the title is picked from it AS SERVED + * to the recipient ({@link titleAsServed} over their {@link servedFields} + * answer), so a title field the recipient may not read, or is served + * masked, gives the id. So does a record with no title, one that cannot be + * read, and a recipient whose envelope cannot be resolved (at `warn`): every + * failure falls back to the id, never to a title. + * + * Cost: one record read per notice, and one envelope resolution + * ({@link recipientContext}) plus one field-visibility ask per recipient — + * only when a field-visibility source is composed and the record has a title + * at all. A notice reaches one recipient at most call sites, and a slate at + * the others; the envelope is the one the administrators' slate already + * resolves per member ({@link organizationAdministrators}). */ - private nameSubjectRecord(object: string, recordId: string): ApprovalSubjectParams { - // [#23594 / #23556 ruling B] The inbox serves the title narrowed per reader (resolveSubjectDisplays); this body is - // written once at send time, with no per-recipient reader to narrow for, so it stays label-and-id. - return subjectParams(object, this.objectLabelOf(object), recordId); + private async subjectNamesFor( + about: { readonly object: string; readonly recordId: string }, + recipients: readonly string[], + organizationId: string | undefined, + ): Promise> { + const names = new Map(); + const displayField = this.resolveDisplayField(about.object); + let record: any; + try { + const rows = await this.engine.find(about.object, { + where: { id: about.recordId }, limit: 1, context: SYSTEM_CTX, + }); + record = Array.isArray(rows) ? rows[0] : undefined; + } catch (err: any) { + this.logger?.warn?.('[approvals] the record a notification names could not be read — it is named by its id', { + object: about.object, error: err?.message ?? String(err), + }); + return names; + } + if (!record || !ApprovalService.titleAsServed(record, displayField, undefined)) return names; + // No field-visibility source: the seam's pass-through (`servedFields` + // answers `undefined` for every caller), so every recipient reads the title + // the inbox serves them, and no envelope needs resolving to say so. + const authority = this.fieldVisibility; + const narrows = !!authority && typeof authority.getReadableFields === 'function'; + for (const recipient of recipients) { + try { + const context = narrows ? await this.recipientContext(recipient, organizationId) : undefined; + if (narrows && !context) continue; + const readable = context ? (await this.servedFields([about.object], context)).get(about.object) : undefined; + const title = ApprovalService.titleAsServed(record, displayField, readable); + if (title) names.set(recipient, title); + } catch (err: any) { + this.logger?.warn?.('[approvals] what a notification recipient may read could not be resolved — the record is named by its id', { + object: about.object, recipient, error: err?.message ?? String(err), + }); + } + } + return names; + } + + /** + * [#26181] The envelope `recipient` reads with in the request's + * organization, built as the `@mention` recipient gate (`plugin-audit`) + * builds a mentioned user's: their own grants by the resolver every request + * envelope comes from (`resolveUserAuthzGrants`), with the organization kept + * only while `vetOrganizationClaim` — the session arm of + * `resolveAuthzContext` — backs it under the posture in force. An e-mail + * address names the account that carries it; one that no account carries + * has no envelope (`undefined`). + */ + private async recipientContext( + recipient: string, + organizationId: string | undefined, + ): Promise { + let userId = recipient; + if (recipient.includes('@')) { + const rows = await this.engine.find('sys_user', { + where: { email: recipient }, fields: ['id'], limit: 1, context: SYSTEM_CTX, + }); + const id = Array.isArray(rows) ? (rows[0] as any)?.id : undefined; + if (id == null || String(id) === '') return undefined; + userId = String(id); + } + let grants = await resolveUserAuthzGrants(this.engine, userId, organizationId ? { tenantId: organizationId } : {}); + const tenantId = vetOrganizationClaim( + organizationId, grants.accessible_org_ids, this.tenancyPosture?.() as TenancyPosture | undefined, + ); + if (organizationId && !tenantId) grants = await resolveUserAuthzGrants(this.engine, userId, {}); + return { ...grants, userId, ...(tenantId ? { tenantId } : {}) } as unknown as ExecutionContext; } /** diff --git a/packages/plugins/plugin-approvals/src/notification-subject-title-per-recipient.integration.test.ts b/packages/plugins/plugin-approvals/src/notification-subject-title-per-recipient.integration.test.ts new file mode 100644 index 00000000000..5e76750f32b --- /dev/null +++ b/packages/plugins/plugin-approvals/src/notification-subject-title-per-recipient.integration.test.ts @@ -0,0 +1,252 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#26181] An approval notification names the record by its title, as the + * recipient may read it. + * + * ## The defect + * + * Every approval notification body named its record by the object's label and + * the record id (`Your Contract C8Mp7TjRYta8mOWx was approved.`), the + * #23556 / #23594 ruling B form. A recipient could not tell which record it + * was without opening it, while the app's own notifications in the same bell + * named it by title. + * + * ## The ruling (A′, maintainer 「同意」 2026-10-26) + * + * Each notification is rendered for one named recipient, so the record is + * named by its display title (ADR-0079 `nameField`) resolved AS THAT + * RECIPIENT, narrowed by their field-level security, and by the record id + * when the title is empty or unreadable to them. The token-bearing action-link + * pages keep the id: they have no reader to narrow for + * (`subject-title-reader-fls.integration.test.ts` (d) pins them). + * + * ## What this pins + * + * Driven through the real `ApprovalService` on a real `ObjectQL` over + * `SqlDriver` (better-sqlite3, in-memory), with a field-visibility source in + * place of `plugin-security`'s (its readable / queryable answers are its + * contract) and the messaging surface recording what reaches it: + * + * (a) a recipient who may read the title field reads the title, in the + * literal body and in the keyed `record` param alike; + * (b) a record whose title is empty is named by its id; + * (c) a recipient who may not read the title field, or is served it masked, + * is named the record by its id; + * (d) two recipients with different field-level security on one request read + * two different bodies, whether each has an emit of its own (the + * opening) or one emit names them both (the SLA breach): that emit is + * split, one per name, so no recipient is handed another's title. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ApprovalService } from './approval-service.js'; +import { ApprovalsServicePlugin } from './approvals-plugin.js'; +import type { FieldVisibilitySource } from './payload-redaction.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const SYSTEM = { isSystem: true, positions: [], permissions: [] } as any; +const asUser = (userId: string) => ({ userId, positions: [], permissions: [] }) as any; + +/** May read every field of the contract. */ +const READER = 'u_reader'; +/** May not read the contract's title field. */ +const RESTRICTED = 'u_restricted'; +/** Served the title field MASKED: readable, not queryable (#20964). */ +const MASKED = 'u_masked'; +/** The submitter, who may read every field. */ +const SUBMITTER = 'u_sub'; + +const TITLE = 'Acme MSA 2026'; + +const contract = { + name: 'crm_contract', + label: 'Contract', + nameField: 'name', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + amount: { name: 'amount', label: 'Amount', type: 'number' as const }, + }, +}; +const sysUser = { + name: 'sys_user', + label: 'User', + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', label: 'Name', type: 'text' as const }, + email: { name: 'email', label: 'Email', type: 'text' as const }, + }, +}; + +/** + * The authorization store a recipient's envelope is resolved from + * (`resolveUserAuthzGrants`): read, and empty — the field-visibility source + * below answers by user. + */ +const authzStore = ['sys_member', 'sys_user_position', 'sys_user_permission_set'].map((name) => ({ + name, + label: name, + fields: { + id: { name: 'id', label: 'Id', type: 'text' as const, primaryKey: true }, + user_id: { name: 'user_id', label: 'User', type: 'text' as const }, + organization_id: { name: 'organization_id', label: 'Organization', type: 'text' as const }, + }, +})); + +const ALL = ['id', 'name', 'amount']; +/** recipient → [readable, queryable] on `crm_contract`. */ +const ANSWERS: Record = { + [READER]: [ALL, ALL], + [SUBMITTER]: [ALL, ALL], + [RESTRICTED]: [['id', 'amount'], ['id', 'amount']], + [MASKED]: [ALL, ['id', 'amount']], +}; + +/** Stands in for `plugin-security`, answering by the context's `userId`. */ +function makeVisibility(): FieldVisibilitySource { + const answer = (object: string, context: any, which: 0 | 1) => { + if (context?.isSystem) return ALL; + if (object !== contract.name) return []; + return ANSWERS[String(context?.userId ?? '')]?.[which] ?? []; + }; + return { + async getReadableFields(object: string, context?: any) { return answer(object, context, 0); }, + async getQueryableFields(object: string, context?: any) { return answer(object, context, 1); }, + }; +} + +/** The plugin's own object definitions, as its manifest registers them. */ +async function packageObjects(): Promise { + let manifest: any; + const ctx: any = { + getService: (name: string) => { + if (name === 'manifest') return { register: (m: any) => { manifest = m; } }; + throw new Error(`no service '${name}'`); + }, + logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }, + }; + await new ApprovalsServicePlugin().init(ctx); + return manifest.objects; +} + +interface Emitted { + topic: string; + audience: string[]; + source?: { object: string; id: string }; + payload?: { title?: string; body?: string }; + keyedText?: { params?: Record }; +} + +describe('[#26181] an approval notification names the record by the title its recipient may read', () => { + let engine: ObjectQL; + let svc: ApprovalService; + let now = Date.parse('2026-10-01T09:00:00.000Z'); + const emits: Emitted[] = []; + let n = 0; + + /** One pending request on a fresh contract named `name`. */ + const open = async (name: string, approvers: string[], extra: Record = {}) => { + n += 1; + const recordId = `C${n}`; + await engine.insert('crm_contract', { id: recordId, name, amount: 100 * n }, { context: SYSTEM } as any); + const row: any = await svc.openNodeRequest({ + object: 'crm_contract', recordId, runId: `run_${n}`, nodeId: 'review', flowName: 'contract_review', + config: { + approvers: approvers.map((value) => ({ type: 'user', value })), + behavior: 'first_response', + ...extra, + } as any, + submitterId: SUBMITTER, record: { id: recordId, name, amount: 100 * n }, + }, asUser(SUBMITTER)); + return { requestId: String(row.id), recordId }; + }; + + /** The emits of `topic` about `requestId` that reached `recipient`. */ + const toldTo = (topic: string, requestId: string, recipient: string) => + emits.filter((e) => e.topic === topic && e.source?.id === requestId && e.audience.includes(recipient)); + + beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true, + }), true); + await engine.init(); + for (const def of [sysUser, ...authzStore, contract, ...(await packageObjects())]) { + engine.registry.registerObject(def as any, 'approvals-test', 'approvals-test'); + } + await engine.syncSchemas(); + await installDefaultOrganization(engine); + for (const id of [READER, RESTRICTED, MASKED, SUBMITTER]) { + await engine.insert('sys_user', { id, name: id, email: `${id}@example.com` }, { context: SYSTEM } as any); + } + svc = new ApprovalService({ + engine: engine as any, + clock: { now: () => new Date(now += 1000) }, + automation: { + resume: async () => ({ status: 'completed' }), + cancelRun: async () => undefined, + getRun: async (runId: string) => ({ id: runId, status: 'paused' }), + } as any, + logger: { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} } as any, + }); + svc.attachFieldVisibility(makeVisibility()); + svc.attachMessaging({ async emit(input) { emits.push(input as Emitted); } }); + }); + + afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + }); + + it('(a) a recipient who may read the title reads it, literal and keyed alike', async () => { + const { requestId } = await open(TITLE, [READER]); + const [requested] = toldTo('approval.requested', requestId, READER); + expect(requested?.payload?.body).toBe(`A decision on Contract ${TITLE} is waiting on you.`); + expect(requested?.keyedText?.params?.record).toBe(TITLE); + + await svc.decide(requestId, { decision: 'approve', actorId: READER }, asUser(READER)); + const [approved] = toldTo('approval.approved', requestId, SUBMITTER); + expect(approved?.payload?.body).toBe(`Your Contract ${TITLE} was approved.`); + expect(approved?.keyedText?.params?.record).toBe(TITLE); + }); + + it('(b) a record whose title is empty is named by its id', async () => { + const { requestId, recordId } = await open('', [READER]); + const [requested] = toldTo('approval.requested', requestId, READER); + expect(requested?.payload?.body).toBe(`A decision on Contract ${recordId} is waiting on you.`); + expect(requested?.keyedText?.params?.record).toBe(recordId); + }); + + it('(c) a recipient who may not read the title field, or is served it masked, is named the id', async () => { + const { requestId, recordId } = await open(TITLE, [RESTRICTED, MASKED]); + for (const recipient of [RESTRICTED, MASKED]) { + const told = toldTo('approval.requested', requestId, recipient); + expect(told).toHaveLength(1); + expect(told[0].payload?.body).toBe(`A decision on Contract ${recordId} is waiting on you.`); + expect(told[0].keyedText?.params?.record).toBe(recordId); + expect(JSON.stringify(told[0])).not.toContain(TITLE); + } + }); + + it('(d) two recipients with different field-level security on one request read different bodies', async () => { + // Each approver has an emit of their own: the opening. + const opened = await open(TITLE, [READER, RESTRICTED], + { escalation: { timeoutHours: 1, action: 'notify', notifySubmitter: false } }); + const [toReader] = toldTo('approval.requested', opened.requestId, READER); + const [toRestricted] = toldTo('approval.requested', opened.requestId, RESTRICTED); + expect(toReader?.payload?.body).toBe(`A decision on Contract ${TITLE} is waiting on you.`); + expect(toRestricted?.payload?.body).toBe(`A decision on Contract ${opened.recordId} is waiting on you.`); + + // One emit names both: the SLA breach tells the whole slate. It is split + // per name, so neither is handed the other's text. + now += 2 * 3600_000; + await svc.runEscalations(); + const breached = emits.filter((e) => e.topic === 'approval.sla_breached' && e.source?.id === opened.requestId); + expect(breached.map((e) => e.audience)).toEqual([[READER], [RESTRICTED]]); + expect(breached[0].payload?.body).toBe(`A decision on Contract ${TITLE} is overdue.`); + expect(breached[1].payload?.body).toBe(`A decision on Contract ${opened.recordId} is overdue.`); + expect(JSON.stringify(breached[1])).not.toContain(TITLE); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/translations/messages.ts b/packages/plugins/plugin-approvals/src/translations/messages.ts index f1f550ee8a9..6d58dc1f443 100644 --- a/packages/plugins/plugin-approvals/src/translations/messages.ts +++ b/packages/plugins/plugin-approvals/src/translations/messages.ts @@ -26,8 +26,9 @@ * ## The holes * * - `{{object}} {{record}}` — the record the notification is about, named by - * its object's label and its id (`Deal D_123`, the #23556 / #23594 ruling B - * form). `object` is itself rendered in the recipient's language from + * its object's label and its display title as the recipient may read it, + * else its id (`Deal Acme renewal`, `Deal D_123`; #26181 ruling A′). + * `object` is itself rendered in the recipient's language from * `objects..label` when the app's language pack holds it, else the * schema label; * - `{{person}}` — a display name; `{{action}}` — the escalation action's