fix(plugin-approvals): the out-of-office notifications name the absent approver and the delegate by display name

.changeset/24065-ooo-notification-person-names.md

@@ -0,0 +1,9 @@
+---
+'@objectstack/plugin-approvals': patch
+---
+
+Clause-②: no
+
+The two out-of-office approval notifications name people by their display name instead of their raw user id. The delegate's `approval.ooo_substituted` body used to read `You are covering an approval on Deal D_123 while u_ooo is out of office.`, and now names the absent approver: `… while Olive Away is out of office.` The absent approver's `approval.ooo_skipped` body used to read `… was routed to u_bob while you are out of office.`, and now names the delegate: `… was routed to Bob …`.
+
+The names come from the same user-name lookup the approvals inbox uses for pending approvers and the timeline, read once per opening for every person the out-of-office routing touched. An account row with no name is named by its email, as the inbox names it. A person with no readable name (no account row for the id, or a failed read) is still named by the user id, as before. The `ooo_substitute` entry on the request's audit trail is unchanged and still records the two user ids (`u_ooo → u_bob — reason`).

packages/plugins/plugin-approvals/src/approval-notification-body.integration.test.ts

@@ -49,6 +49,16 @@
  * 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.
  *
+ * ## And whom the out-of-office bodies name (#24065)
+ *
+ * The two out-of-office bodies also name a person: the delegate is told whom
+ * they cover, and the absent approver is told who covers for them. Both used to
+ * interpolate the raw `sys_user` id (`… while u_ooo is out of office.`). They
+ * now name each person through `resolveUserNames`, the chain the approvals door
+ * names people with. The battery pins each person's name in each body, and a
+ * control: an approver id no account row answers to is named by that id, the
+ * way every reader of the chain renders an unresolved address.
+ *
  * ## And whose notification it is (#23284)
  *
  * The same battery pins the organization every call site hands messaging: the
@@ -80,6 +90,13 @@ const USERS = [
   { id: 'u_boss', name: 'Boss', email: 'boss@example.com' },
 ];
 
+/**
+ * [#24065] An approver id no `sys_user` row answers to, such as an account
+ * removed after its out-of-office delegation was written. Deliberately not in
+ * {@link USERS}.
+ */
+const GONE = 'u_gone';
+
 /** The text each person writes — distinct, so no one can pass for another. */
 const COMMENT = 'The signed contract is attached to the record now.';
 const QUESTION = 'Which cost centre should this be booked against?';
@@ -238,6 +255,15 @@ describe('every approval notification carries its text in body', () => {
     }, { context: SYSTEM } as any);
     const ooo = await open('ooo', [{ type: 'user', value: 'u_ooo' }]);
     expect(ooo.pending_approvers).toEqual(['u_bob']);
+    // [#24065] The control: the absent approver names no account, so no name
+    // can be read for them; the delegate the same hop routes to does.
+    await engine.insert('sys_approval_delegation', {
+      id: 'dlg_2', delegator_id: GONE, delegate_id: 'u_alice', reason: 'leave',
+      valid_from: '2026-09-01T00:00:00.000Z', valid_until: '2026-12-31T00:00:00.000Z',
+      created_at: '2026-09-01T00:00:00.000Z', updated_at: '2026-09-01T00:00:00.000Z',
+    }, { context: SYSTEM } as any);
+    const oooGone = await open('ooo_gone', [{ type: 'user', value: GONE }]);
+    expect(oooGone.pending_approvers).toEqual(['u_alice']);
 
     // ── The SLA sweep: a notify (submitter told) and a reassign (submitter not) ──
     await open('sla_notify', [{ type: 'user', value: 'u_alice' }],
@@ -255,8 +281,8 @@ describe('every approval notification carries its text in body', () => {
   it('the battery reaches every call site that can notify anyone — twelve of the thirteen', () => {
     expect(new Set(emits.map(siteOf))).toEqual(new Set([
       // The opening tells each concrete approver on every request the battery
-      // opens — all but `ooo`, whose one approver is the delegate, told once
-      // by `approval.ooo_substituted` instead.
+      // opens — all but `ooo` and `ooo_gone`, whose one approver is the
+      // delegate, told once by `approval.ooo_substituted` instead.
       'approval.requested@thread',
       'approval.requested@revise',
       'approval.requested@budget',
@@ -266,6 +292,8 @@ describe('every approval notification carries its text in body', () => {
       'approval.requested@bare',
       'approval.ooo_substituted@ooo',
       'approval.ooo_skipped@ooo',
+      'approval.ooo_substituted@ooo_gone',
+      'approval.ooo_skipped@ooo_gone',
       'approval.returned@budget',
       'approval.returned@revise',
       'approval.returned@rework',
@@ -327,6 +355,44 @@ describe('every approval notification carries its text in body', () => {
     expect(slashed).toEqual([]);
   });
 
+  /** [#24065] Every body one call site sent. */
+  const bodiesAt = (site: string): string[] =>
+    emits.filter((e) => siteOf(e) === site).map((e) => String(e.payload?.body));
+
+  it('[#24065] both out-of-office bodies name the absent approver and the delegate by display name, never by user id', () => {
+    // The pin is each person's name in each body, not the sentence around it.
+    const NAMED: Record<string, { name: string; id: string }> = {
+      // The delegate is told whom they cover …
+      'approval.ooo_substituted@ooo': { name: 'Olive Away', id: 'u_ooo' },
+      // … and the absent approver who covers for them.
+      'approval.ooo_skipped@ooo': { name: 'Bob', id: 'u_bob' },
+      // The control's resolvable half: the delegate on the same hop as an
+      // approver no account answers to.
+      'approval.ooo_skipped@ooo_gone': { name: 'Alice', id: 'u_alice' },
+    };
+    const offenders = Object.entries(NAMED).flatMap(([site, { name, id }]) => {
+      const bodies = bodiesAt(site);
+      if (!bodies.length) return [`${site}: sent nothing`];
+      return bodies.filter((body) => !body.includes(name) || body.includes(id)).map((body) => `${site}: ${body}`);
+    });
+    expect(offenders).toEqual([]);
+
+    // And no out-of-office body anywhere in the battery names one of its
+    // people by their user id.
+    const byId = emits
+      .filter((e) => e.topic === 'approval.ooo_substituted' || e.topic === 'approval.ooo_skipped')
+      .flatMap((e) => USERS.filter((u) => String(e.payload?.body).includes(u.id))
+        .map((u) => `${siteOf(e)}: names ${u.id} in ${String(e.payload?.body)}`));
+    expect(byId).toEqual([]);
+  });
+
+  it('[#24065] a person no name can be read for is named by their stored id, as every reader of the chain renders an unresolved address', () => {
+    // `u_gone` names no `sys_user` row, so the chain answers no name for it.
+    // The body still names the person, by the id the delegation stores: never
+    // an empty slot, never `undefined`.
+    expect(bodiesAt('approval.ooo_substituted@ooo_gone')).toEqual([expect.stringContaining(GONE)]);
+  });
+
   it('every notification carries a non-empty body, and no field the payload does not declare', () => {
     const offenders: string[] = [];
     for (const e of emits) {

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

@@ -3605,6 +3605,22 @@ export class ApprovalService implements IApprovalService {
     // audit trail (a system action, no human actor) and notified to both the
     // delegate — who now owns the slot — and the skipped approver. [#23978]
     // Both bodies name the record the way every approval notification does.
+    //
+    // [#24065] And each body names the person by display name, through the
+    // chain the approvals door names people with (`resolveUserNames`):
+    // both people of every hop in ONE read for the whole fan-out, and no read
+    // at all when nothing was substituted. A person the chain answers no name
+    // for (no account row, or the read failed) is named by the stored id, the
+    // way every reader of that chain renders an unresolved address. A body is
+    // a STORED copy of the name (messaging stores it, the inbox serves it),
+    // like the stored labels the `resolveUserNames` docblock lists.
+    //
+    // The audit `comment` keeps the ids by design: it is the request's audit
+    // trail of identities, served as written by the timeline and the action
+    // lists. Display names belong in read-time `*_name` companions (the
+    // `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;
     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,
@@ -3621,7 +3637,7 @@ export class ApprovalService implements IApprovalService {
         dedupKey: `approval-ooo-${id}-${sub.to}`,
         payload: {
           title: 'Approval routed to you (out-of-office cover)',
-          body: `You are covering an approval on ${this.nameSubjectRecord(input.object, input.recordId)} while ${sub.from} is out of office.`,
+          body: `You are covering an approval on ${this.nameSubjectRecord(input.object, input.recordId)} while ${oooPerson(sub.from)} is out of office.`,
           actionUrl: '/system/approvals',
         },
       });
@@ -3633,7 +3649,7 @@ export class ApprovalService implements IApprovalService {
         dedupKey: `approval-ooo-skip-${id}-${sub.from}`,
         payload: {
           title: 'Approval routed to your delegate',
-          body: `An approval on ${this.nameSubjectRecord(input.object, input.recordId)} was routed to ${sub.to} while you are out of office.`,
+          body: `An approval on ${this.nameSubjectRecord(input.object, input.recordId)} was routed to ${oooPerson(sub.to)} while you are out of office.`,
           actionUrl: '/system/approvals',
         },
       });