Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/26181-approval-notification-record-title.md
Original file line number Diff line number Diff line change
@@ -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
10 changes: 7 additions & 3 deletions content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions packages/plugins/plugin-approvals/src/action-link-pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]> = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<object_name>/<record_id>`, 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)
*
Expand Down Expand Up @@ -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<string, string> = {
'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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
Expand Down Expand Up @@ -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.' },
]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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.<object>.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.<object>.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.
Expand Down Expand Up @@ -75,18 +78,19 @@ export function approvalMessageKey(id: ApprovalMessageId): string {
}

/**
* The record `objectName`/`recordId` names: the object's label — keyed by
* `objects.<object>.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.<object>.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,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
Expand All @@ -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);
Expand All @@ -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);
});

Expand Down
Loading
Loading