From 452d891d16cdfd4b6bc04de32af3d60b7ac33310 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 01:49:48 +0000 Subject: [PATCH 1/3] fix(plugin-approvals): approval.requested names the record by label and title; the empty-slot warning states the slot, not the request's fate The approval.requested body named the record as object/id. It now names it the way the approvals inbox row does: the object's label and the record's title, read through the same live resolution enrichRows uses (factored out as resolveSubjectDisplays). With no title, label and id; the unredacted submit snapshot is never consulted for a title. The per-slot "expanded to nobody" warning no longer says the request cannot advance: on the default admin_rescue path the request opens on the organization's administrators, which the node-level line states. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../23556-approval-request-names-record.md | 12 +++ ...min-rescue-named-slate.integration.test.ts | 14 +++ ...requested-notification.integration.test.ts | 36 ++++++- .../src/approval-service.test.ts | 7 +- .../plugin-approvals/src/approval-service.ts | 102 ++++++++++++++---- 5 files changed, 146 insertions(+), 25 deletions(-) create mode 100644 .changeset/23556-approval-request-names-record.md diff --git a/.changeset/23556-approval-request-names-record.md b/.changeset/23556-approval-request-names-record.md new file mode 100644 index 00000000000..9a307805991 --- /dev/null +++ b/.changeset/23556-approval-request-names-record.md @@ -0,0 +1,12 @@ +--- +'@objectstack/plugin-approvals': patch +--- + +fix(plugin-approvals): the `approval.requested` notification names the record by its object label and title, and the empty-slot warning no longer says the request "cannot advance" + +Clause-②: no + +- **The notification names the record.** The body of `approval.requested` used to read `A decision on crm_opportunity/JCqLLy5dSug1MldB is waiting on you.`, so a grouped bell entry could not tell an approver which record was waiting. It now names the record the way the Approvals Inbox row does, from the same resolution: the object's label, then the record's title in quotes, `A decision on Opportunity "Acme renewal" is waiting on you.` A record with no title is named by its label and id, `A decision on Opportunity JCqLLy5dSug1MldB is waiting on you.`, which is what the body carried before. The object's API name stands in only when its schema declares no label, as in the inbox. The title is never taken from the submit snapshot, which the inbox serves only after redacting it for its reader. Title, topic, action link and recipients are unchanged. +- **The empty-slot warning states the slot, not the request's fate.** An approver that expands to nobody used to log `the slot routes to no one and the request cannot advance until someone is added or the approver is re-pointed`, and on the default `onEmptyApprovers: 'admin_rescue'` path the next line opened the request on the organization's administrators, who then decided it. The line now reads `the slot names no one, so no one is asked through it. Staff it or re-point the approver; a slate left naming no one is decided by the node's onEmptyApprovers.` What the node does with an empty slate is stated once, by the node-level line that already follows it. + +A patch: both are server-authored texts. No schema, translation key, export or notification field moves. diff --git a/packages/plugins/plugin-approvals/src/admin-rescue-named-slate.integration.test.ts b/packages/plugins/plugin-approvals/src/admin-rescue-named-slate.integration.test.ts index efea3fb0d6d..b5faf949ce2 100644 --- a/packages/plugins/plugin-approvals/src/admin-rescue-named-slate.integration.test.ts +++ b/packages/plugins/plugin-approvals/src/admin-rescue-named-slate.integration.test.ts @@ -31,6 +31,8 @@ * on the slate for being one; another organization's administrator is not * on it; * - the request says why: a timeline note and `__resolvedFrom`; + * - [#23556] the log names the empty slot once and the routing once, and no + * line says the request cannot advance; * - an administrator out of office is covered by their delegate; * - CONTROLS: a staffed slate is unchanged; `fallback`, `auto_approve` and * `fail` are unchanged; an organization with no administrator still gets @@ -260,6 +262,18 @@ describe('an empty admin_rescue slate opens on the organization\'s administrator }); }); + it('[#23556] the log names the empty slot and the routing once — no line says the request cannot advance', async () => { + const before = warnings.length; + const row = await open('logged', UNSTAFFED); + expect([...row.pending_approvers].sort()).toEqual(['u_admin', 'u_owner']); + const lines = warnings.slice(before); + // The scene: the per-slot line and the node-level routing line both fired, + // so the absence below is not read off an opening that logged nothing. + expect(lines.filter((w) => w.includes("approver 'position:sales_manager' expanded to nobody"))).toHaveLength(1); + expect(lines.filter((w) => w.includes("opening on this organization's administrators"))).toHaveLength(1); + expect(lines.filter((w) => /cannot advance/.test(w))).toEqual([]); + }); + it('a fallback that resolves to no one degrades to the administrators, and says so', async () => { const row = await open('fallback_empty', UNSTAFFED, { onEmptyApprovers: 'fallback', fallbackApprovers: [{ type: 'position', value: 'treasury' }], 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 e56e5a55bdd..e26f7170253 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,6 +21,10 @@ * 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 the way the approvals inbox row does — + * the object's label and the record's title, from the same resolution — and + * a record with no title is named by label and id, never by the unredacted + * submit snapshot; * - 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, @@ -185,18 +189,46 @@ 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. + // it, the submitter as the actor. [#23556] 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 crm_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 }); expect(e.actorId).toBe('u_sub'); } expect(new Set(requested(row.id).map((e) => e.dedupKey)).size).toBe(3); + + // [#23556] ...and by the very name the recipient's own inbox row shows. + const [listed] = await svc.listRequests({ approverId: 'u_alice', recordId: 'D_ladder' }, asUser('u_alice')); + expect([listed?.object_label, listed?.record_title]).toEqual(['Deal', 'ladder']); + expect(requested(row.id)[0]?.payload?.body) + .toBe(`A decision on ${listed!.object_label} "${listed!.record_title}" is waiting on you.`); + }); + + it('[#23556] a record with no title is named by its object label and id — never by the unredacted submit snapshot', async () => { + // The live record has no title; the trigger snapshot the flow handed over + // still carries one. The inbox reads that snapshot only after redacting it + // for its reader's field-level security, and stored notification text has + // no reader to redact for — so the body names what it named before: the id. + const SNAPSHOT_TITLE = 'Snapshot-only title'; + await engine.insert('crm_deal', { id: 'D_untitled', title: null }, { context: SYSTEM } as any); + const row: any = await svc.openNodeRequest({ + object: 'crm_deal', recordId: 'D_untitled', runId: 'run_untitled', nodeId: 'review', flowName: 'deal_review', + config: { approvers: [{ type: 'user', value: 'u_alice' }], behavior: 'first_response' } as any, + submitterId: 'u_sub', record: { id: 'D_untitled', title: SNAPSHOT_TITLE }, + }, asUser('u_sub')); + // The scene: the snapshot really does carry the title the body must not use. + 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_untitled is waiting on you.'); + expect(String(message?.payload?.body)).not.toContain(SNAPSHOT_TITLE); }); it('a person the slate lists under two groups is told once', async () => { diff --git a/packages/plugins/plugin-approvals/src/approval-service.test.ts b/packages/plugins/plugin-approvals/src/approval-service.test.ts index 7ad4f3348a5..a444ebac994 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.test.ts @@ -3508,7 +3508,12 @@ describe('ApprovalService — a graph approver that expands to nobody warns (#38 expect(req.pending_approvers).toEqual([`${type}:${value}`]); const hit = warnings.find(([msg]) => String(msg).includes('expanded to nobody')); expect(hit, `no warning for ${type}`).toBeTruthy(); - expect(String(hit[0])).toContain('cannot advance until someone is added or the approver is re-pointed'); + // [#23556] The line names the slot and points at the node's policy; the + // request's fate is the node-level line's to state, so this one never says + // the request "cannot advance" — false on the default admin_rescue path. + expect(String(hit[0])).toContain(`'${type}:${value}'`); + expect(String(hit[0])).toContain('onEmptyApprovers'); + expect(String(hit[0])).not.toMatch(/cannot advance/); expect(hit[1]).toMatchObject({ type, value, organizationId: 't1' }); }); diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 5d330baff3d..9dd601b7469 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -2212,9 +2212,20 @@ export class ApprovalService implements IApprovalService { // permanently stuck approval (#3424). The fallback itself stays (a // literal keeps 15.x slots and substring fixtures working); it just // stops being invisible. + // + // [#23556] The line states the SLOT, never the request's fate. This + // resolver sees one approver entry, not the slate: whether the request + // opens on the slate's other people, or the slate names no one and the + // node's `onEmptyApprovers` decides (by default `admin_rescue`, which + // opens on the organization's administrators, #22824), is only known + // to `openNodeRequest`, which logs that decision itself on the line that + // follows. The old ending ("the request cannot advance until someone is + // added") restated a fate this resolver cannot know, and was false on + // the default path. this.logger?.warn?.( - `[approvals] approver '${type}:${a.value}' expanded to nobody — the slot routes to no one ` - + `and the request cannot advance until someone is added or the approver is re-pointed`, + `[approvals] approver '${type}:${a.value}' expanded to nobody — the slot names no one, so no one is ` + + `asked through it. Staff it or re-point the approver; a slate left naming no one is decided by the ` + + `node's onEmptyApprovers.`, { type, value: a.value, organizationId: organizationId ?? null }, ); } @@ -3583,8 +3594,14 @@ export class ApprovalService implements IApprovalService { // `approval.ooo_substituted` — one message per approver, not two. // No one-tap decision links: ADR-0043 mints those on `remind()` only, and // minting bearer tokens on every opening is a decision this does not make. + // + // [#23556] The body names the record the way the approvals list does — + // `Deal "Acme renewal"`, never `crm_deal/` — so a grouped bell entry + // tells its reader which record is waiting. One name for every recipient, + // read once, and only when someone is told. const toldByOoo = new Set(substitutions.map((sub) => sub.to)); const openedOn = new Set(approvers.filter((a) => a && !a.includes(':') && !toldByOoo.has(a))); + const subject = openedOn.size ? await this.nameSubjectRecord(input.object, input.recordId) : ''; for (const approver of openedOn) { await this.notify({ topic: 'approval.requested', @@ -3595,7 +3612,7 @@ export class ApprovalService implements IApprovalService { dedupKey: `approval-requested-${id}-${approver}`, payload: { title: 'Approval requested', - body: `A decision on ${input.object}/${input.recordId} is waiting on you.`, + body: `A decision on ${subject} is waiting on you.`, actionUrl: '/system/approvals', }, }); @@ -6680,6 +6697,65 @@ export class ApprovalService implements IApprovalService { } catch { return {}; } } + /** + * The subject records' titles and their objects' labels, keyed + * `" "` and by object name: the LIVE half of how the approvals + * list names a request's record ({@link ApprovalService.enrichRows} sets + * `record_title` and `object_label` from it, then falls back to the reader's + * redacted payload snapshot for a title this read did not find). + * + * [#23556] One resolution with two readers: the list, and the + * `approval.requested` body ({@link ApprovalService.nameSubjectRecord}), so + * the notification names the record by the same live title and label its + * recipient's inbox row shows. Batched: one read per object. Best-effort — + * an unregistered object or a failed read leaves its entries unset. + */ + private async resolveSubjectDisplays( + byObject: ReadonlyMap>, + ): Promise<{ titles: Map; objectLabels: Map }> { + const titles = new Map(); + const objectLabels = new Map(); + for (const [object, idSet] of byObject) { + try { + const schema: any = (this.engine as any).getSchema?.(object); + if (schema?.label) objectLabels.set(object, String(schema.label)); + } catch { /* label optional */ } + const ids = Array.from(idSet); + const displayField = this.resolveDisplayField(object); + try { + const recs = await this.engine.find(object, { + where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX, + }); + for (const rec of (recs ?? []) as any[]) { + const title = ApprovalService.pickTitle(rec, displayField); + if (rec?.id && title) titles.set(`${object} ${rec.id}`, title); + } + } catch { /* object may be unregistered — the caller's fallback applies */ } + } + return { titles, objectLabels }; + } + + /** + * [#23556] The record an approval notification is about, named the way the + * approvals list names it: the object's label (its API name only when the + * schema has none, as the list falls back), then the record's title in + * quotes — `Deal "Acme renewal"` — from {@link ApprovalService.resolveSubjectDisplays}. + * + * With no title from that read, the record id stands in for it + * (`Deal D_123`), which is what the body carried before. ⛔ The payload + * snapshot is NOT consulted for a title here, unlike the list's fallback: + * the list reads the snapshot only after redacting it for the READER's + * field-level security (#10749), and stored notification text has no + * reader to redact for, so an unredacted snapshot title could name a field + * a recipient's inbox row would withhold. + */ + private async nameSubjectRecord(object: string, recordId: string): Promise { + const { titles, objectLabels } = await this.resolveSubjectDisplays(new Map([[object, new Set([recordId])]])); + const label = objectLabels.get(object) ?? object; + const title = titles.get(`${object} ${recordId}`); + return title ? `${label} "${title}"` : `${label} ${recordId}`; + } + /** * Attach inbox display fields to rows so clients never render a raw * identifier: `record_title`, `submitter_name`, `object_label`, @@ -6703,25 +6779,7 @@ export class ApprovalService implements IApprovalService { if (!set) { set = new Set(); byObject.set(r.object_name, set); } set.add(r.record_id); } - const titles = new Map(); - const objectLabels = new Map(); - for (const [object, idSet] of byObject) { - try { - const schema: any = (this.engine as any).getSchema?.(object); - if (schema?.label) objectLabels.set(object, String(schema.label)); - } catch { /* label optional */ } - const ids = Array.from(idSet); - const displayField = this.resolveDisplayField(object); - try { - const recs = await this.engine.find(object, { - where: { id: { $in: ids } }, limit: ids.length, context: SYSTEM_CTX, - }); - for (const rec of (recs ?? []) as any[]) { - const title = ApprovalService.pickTitle(rec, displayField); - if (rec?.id && title) titles.set(`${object} ${rec.id}`, title); - } - } catch { /* object may be unregistered — payload fallback below */ } - } + const { titles, objectLabels } = await this.resolveSubjectDisplays(byObject); // Lookup foreign keys inside payload snapshots → referenced record titles. const lookupFieldsByObject = new Map>(); From 8e5919cb46b4432f5e31f0c33867ab8d173b015b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 02:26:58 +0000 Subject: [PATCH 2/3] docs(plugin-approvals): say that the shared subject-title read is not narrowed by any reader's field-level security Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- packages/plugins/plugin-approvals/src/approval-service.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 9dd601b7469..a5fb47c28d6 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -6709,6 +6709,11 @@ export class ApprovalService implements IApprovalService { * the notification names the record by the same live title and label its * recipient's inbox row shows. Batched: one read per object. Best-effort — * an unregistered object or a failed read leaves its entries unset. + * + * The read runs under the system context, so no reader's field-level + * security narrows the title it finds — for the list exactly as for the + * notification. Narrowing it per reader is a change to THIS function, and + * the notification would then have to pass each recipient. */ private async resolveSubjectDisplays( byObject: ReadonlyMap>, From 7c8b072027989481271a78f9e2ad84217bae9e14 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 03:05:08 +0000 Subject: [PATCH 3/3] fix(plugin-approvals): approval.requested names the record by object label and id only; the title is withheld until the subject-title read is narrowed per reader The body no longer embeds the record title from any source: the inbox's subject-title read runs under the system context with no reader's field-level security, and notification text is stored and may be e-mailed. nameSubjectRecord now reads only the object label, through objectLabelOf, which resolveSubjectDisplays shares (enrichRows' behaviour unchanged). The empty-slot warning is unchanged. Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ Co-authored-by: Claude --- .../23556-approval-request-names-record.md | 6 +- ...requested-notification.integration.test.ts | 42 +++++------ .../plugin-approvals/src/approval-service.ts | 74 ++++++++++--------- 3 files changed, 62 insertions(+), 60 deletions(-) diff --git a/.changeset/23556-approval-request-names-record.md b/.changeset/23556-approval-request-names-record.md index 9a307805991..864abf806ae 100644 --- a/.changeset/23556-approval-request-names-record.md +++ b/.changeset/23556-approval-request-names-record.md @@ -2,11 +2,11 @@ '@objectstack/plugin-approvals': patch --- -fix(plugin-approvals): the `approval.requested` notification names the record by its object label and title, and the empty-slot warning no longer says the request "cannot advance" +fix(plugin-approvals): the `approval.requested` notification names the record by its object label and id, and the empty-slot warning no longer says the request "cannot advance" Clause-②: no -- **The notification names the record.** The body of `approval.requested` used to read `A decision on crm_opportunity/JCqLLy5dSug1MldB is waiting on you.`, so a grouped bell entry could not tell an approver which record was waiting. It now names the record the way the Approvals Inbox row does, from the same resolution: the object's label, then the record's title in quotes, `A decision on Opportunity "Acme renewal" is waiting on you.` A record with no title is named by its label and id, `A decision on Opportunity JCqLLy5dSug1MldB is waiting on you.`, which is what the body carried before. The object's API name stands in only when its schema declares no label, as in the inbox. The title is never taken from the submit snapshot, which the inbox serves only after redacting it for its reader. Title, topic, action link and recipients are unchanged. -- **The empty-slot warning states the slot, not the request's fate.** An approver that expands to nobody used to log `the slot routes to no one and the request cannot advance until someone is added or the approver is re-pointed`, and on the default `onEmptyApprovers: 'admin_rescue'` path the next line opened the request on the organization's administrators, who then decided it. The line now reads `the slot names no one, so no one is asked through it. Staff it or re-point the approver; a slate left naming no one is decided by the node's onEmptyApprovers.` What the node does with an empty slate is stated once, by the node-level line that already follows it. +- **The notification names the record's object by its label.** The body of `approval.requested` used to read `A decision on crm_opportunity/JCqLLy5dSug1MldB is waiting on you.`, with the object's API name. It now reads `A decision on Opportunity JCqLLy5dSug1MldB is waiting on you.`, using the label the Approvals Inbox row shows for that object. The API name is used only when the object's schema declares no label, as in the inbox. The body carries the record's id, as before, and not its title, from any source. Title, topic, action link and recipients are unchanged. +- **The empty-slot warning states the slot, not the request's fate.** An approver that expands to nobody used to log `the slot routes to no one and the request cannot advance until someone is added or the approver is re-pointed`. On the default `onEmptyApprovers: 'admin_rescue'` path the next line opened the request on the organization's administrators, who then decided it. The line now reads `the slot names no one, so no one is asked through it. Staff it or re-point the approver; a slate left naming no one is decided by the node's onEmptyApprovers.` What the node does with an empty slate is stated once, by the node-level line that already follows it. A patch: both are server-authored texts. No schema, translation key, export or notification field moves. 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 e26f7170253..f31e96613f3 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,10 +21,9 @@ * 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 the way the approvals inbox row does — - * the object's label and the record's title, from the same resolution — and - * a record with no title is named by label and id, never by the unredacted - * submit snapshot; + * - [#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; * - 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, @@ -190,44 +189,43 @@ 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 title, never `crm_deal/D_ladder`. + // its object's label and its id, 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 "ladder" is waiting on you.`, + body: `A decision on Deal D_ladder is waiting on you.`, actionUrl: `/system/approvals?request=${encodeURIComponent(row.id)}`, }); expect(e.source).toEqual({ object: 'sys_approval_request', id: row.id }); expect(e.actorId).toBe('u_sub'); } expect(new Set(requested(row.id).map((e) => e.dedupKey)).size).toBe(3); - - // [#23556] ...and by the very name the recipient's own inbox row shows. - const [listed] = await svc.listRequests({ approverId: 'u_alice', recordId: 'D_ladder' }, asUser('u_alice')); - expect([listed?.object_label, listed?.record_title]).toEqual(['Deal', 'ladder']); - expect(requested(row.id)[0]?.payload?.body) - .toBe(`A decision on ${listed!.object_label} "${listed!.record_title}" is waiting on you.`); }); - it('[#23556] a record with no title is named by its object label and id — never by the unredacted submit snapshot', async () => { - // The live record has no title; the trigger snapshot the flow handed over - // still carries one. The inbox reads that snapshot only after redacting it - // for its reader's field-level security, and stored notification text has - // no reader to redact for — so the body names what it named before: the id. + it('[#23594] a record whose title resolves is still named by label and id — no title reaches the body', async () => { + // The inbox's subject title is not yet narrowed by its reader's field-level + // security (#23594), and notification 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). + const LIVE_TITLE = 'Acme renewal'; const SNAPSHOT_TITLE = 'Snapshot-only title'; - await engine.insert('crm_deal', { id: 'D_untitled', title: null }, { context: SYSTEM } as any); + await engine.insert('crm_deal', { id: 'D_titled', title: LIVE_TITLE }, { context: SYSTEM } as any); const row: any = await svc.openNodeRequest({ - object: 'crm_deal', recordId: 'D_untitled', runId: 'run_untitled', nodeId: 'review', flowName: 'deal_review', + object: 'crm_deal', recordId: 'D_titled', runId: 'run_titled', nodeId: 'review', flowName: 'deal_review', config: { approvers: [{ type: 'user', value: 'u_alice' }], behavior: 'first_response' } as any, - submitterId: 'u_sub', record: { id: 'D_untitled', title: SNAPSHOT_TITLE }, + submitterId: 'u_sub', record: { id: 'D_titled', title: SNAPSHOT_TITLE }, }, asUser('u_sub')); - // The scene: the snapshot really does carry the title the body must not use. + // 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. + 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_untitled is waiting on you.'); + 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(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 a5fb47c28d6..fd9c6431ebe 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -3595,13 +3595,12 @@ export class ApprovalService implements IApprovalService { // No one-tap decision links: ADR-0043 mints those on `remind()` only, and // minting bearer tokens on every opening is a decision this does not make. // - // [#23556] The body names the record the way the approvals list does — - // `Deal "Acme renewal"`, never `crm_deal/` — so a grouped bell entry - // tells its reader which record is waiting. One name for every recipient, - // read once, and only when someone is told. + // [#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. const toldByOoo = new Set(substitutions.map((sub) => sub.to)); const openedOn = new Set(approvers.filter((a) => a && !a.includes(':') && !toldByOoo.has(a))); - const subject = openedOn.size ? await this.nameSubjectRecord(input.object, input.recordId) : ''; + const subject = this.nameSubjectRecord(input.object, input.recordId); for (const approver of openedOn) { await this.notify({ topic: 'approval.requested', @@ -6704,16 +6703,15 @@ export class ApprovalService implements IApprovalService { * `record_title` and `object_label` from it, then falls back to the reader's * redacted payload snapshot for a title this read did not find). * - * [#23556] One resolution with two readers: the list, and the - * `approval.requested` body ({@link ApprovalService.nameSubjectRecord}), so - * the notification names the record by the same live title and label its - * recipient's inbox row shows. Batched: one read per object. Best-effort — - * an unregistered object or a failed read leaves its entries unset. + * Batched: one read per object. Best-effort — an unregistered object or a + * failed read leaves its entries unset. * - * The read runs under the system context, so no reader's field-level - * security narrows the title it finds — for the list exactly as for the - * notification. Narrowing it per reader is a change to THIS function, and - * the notification would then have to pass each recipient. + * [#23594] The read runs under the system context, so no reader's + * field-level security narrows the title it finds. Narrowing it per reader + * is a change to THIS function — the one place the inbox's subject title is + * resolved. The `approval.requested` body ({@link ApprovalService.nameSubjectRecord}) + * withholds the title until then, because notification text is stored and + * may be e-mailed, so a title sent cannot be withdrawn by a later fix. */ private async resolveSubjectDisplays( byObject: ReadonlyMap>, @@ -6721,10 +6719,8 @@ export class ApprovalService implements IApprovalService { const titles = new Map(); const objectLabels = new Map(); for (const [object, idSet] of byObject) { - try { - const schema: any = (this.engine as any).getSchema?.(object); - if (schema?.label) objectLabels.set(object, String(schema.label)); - } catch { /* label optional */ } + const label = this.objectLabelOf(object); + if (label) objectLabels.set(object, label); const ids = Array.from(idSet); const displayField = this.resolveDisplayField(object); try { @@ -6741,24 +6737,32 @@ export class ApprovalService implements IApprovalService { } /** - * [#23556] The record an approval notification is about, named the way the - * approvals list names it: the object's label (its API name only when the - * schema has none, as the list falls back), then the record's title in - * quotes — `Deal "Acme renewal"` — from {@link ApprovalService.resolveSubjectDisplays}. - * - * With no title from that read, the record id stands in for it - * (`Deal D_123`), which is what the body carried before. ⛔ The payload - * snapshot is NOT consulted for a title here, unlike the list's fallback: - * the list reads the snapshot only after redacting it for the READER's - * field-level security (#10749), and stored notification text has no - * reader to redact for, so an unredacted snapshot title could name a field - * a recipient's inbox row would withhold. + * 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). + */ + private objectLabelOf(object: string): string | undefined { + try { + const schema: any = (this.engine as any).getSchema?.(object); + return schema?.label ? String(schema.label) : undefined; + } catch { return undefined; } + } + + /** + * [#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. */ - private async nameSubjectRecord(object: string, recordId: string): Promise { - const { titles, objectLabels } = await this.resolveSubjectDisplays(new Map([[object, new Set([recordId])]])); - const label = objectLabels.get(object) ?? object; - const title = titles.get(`${object} ${recordId}`); - return title ? `${label} "${title}"` : `${label} ${recordId}`; + private nameSubjectRecord(object: string, recordId: string): string { + // [#23594] The title would go here; withheld until resolveSubjectDisplays narrows it by each reader's FLS. + return `${this.objectLabelOf(object) ?? object} ${recordId}`; } /**