Skip to content

Commit c1ac40b

Browse files
committed
wip(service-messaging): stamp recipient_user_id at enqueue; tests; changeset
Claude-Session: https://claude.ai/code/session_01WNppskqdULWrMoXfU9sgo1 Co-authored-by: Claude <noreply@anthropic.com>
1 parent db8d2ef commit c1ac40b

7 files changed

Lines changed: 309 additions & 16 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/service-messaging": patch
3+
---
4+
5+
fix(service-messaging): a notification delivery's title names the person it is for, not their raw user id (#25194)
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
The record title of a `sys_notification_delivery` row (Setup → Notification Deliveries) read `channel → recipient_id`, so a delivery to a user showed their raw user id. It now reads `channel → Person`: the user's name, or their email where the name is blank. A delivery to an address no user holds — an email kept as given, a phone number — is titled by that address, as before.
11+
12+
The delivery object gains one optional field, `recipient_user_id` (Recipient User), a `sys_user` reference the title reads through. `emit()` sets it when the recipient is a user's id, from one `sys_user` read per emit, and leaves it empty for an address. Deleting the user empties it; the delivery stays, titled by the raw recipient. `recipient_id` keeps its type and meaning (the address: a user id, an email or a phone number), and is now labelled **Recipient**.
13+
14+
Rows written before this change carry no `recipient_user_id` and keep their old title until the 90-day window ages them out. Nothing is migrated, and no delivery is routed by the new field.
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25194] A notification delivery's record title names the PERSON it is for,
5+
* and the address itself where the recipient is not a user.
6+
*
7+
* `recipient_id` stays a text column: a recipient is a `sys_user` id, or an
8+
* address kept verbatim — an email no user holds (`RecipientResolver`), a
9+
* phone number the SMS channel texts directly — and the outbox writes past the
10+
* engine's reference check, so a lookup there would store each address as a
11+
* dangling reference. The person rides in `recipient_user_id`, a `sys_user`
12+
* reference the enqueue stamps from one `sys_user` read per emit; the title
13+
* reads it one hop, the shape `sys_notification_preference` takes (PR #25178).
14+
*
15+
* Pinned on a real `ObjectQL` over `SqlDriver` (better-sqlite3) with the real
16+
* `sys_user`, through `MessagingService.emit` and `SqlNotificationOutbox`:
17+
*
18+
* 1. the declaration: `recipient_id` text, `recipient_user_id` an optional
19+
* `sys_user` reference that nulls when the user goes;
20+
* 2. the enqueue stamps `recipient_user_id` for a recipient a user holds, and
21+
* only for one: an unmatched email, a phone number and an id nobody holds
22+
* stay empty;
23+
* 3. the title: the name; the email where the name is blank; the address for
24+
* an address recipient; the raw recipient once the user is deleted;
25+
* 4. a failed `sys_user` read stamps nothing and fails no emit — the rows
26+
* land titled by the raw recipient, and the failure is said once;
27+
* 5. the title still never has a NULL part, and adds no stored column.
28+
*/
29+
30+
import { describe, it, expect, afterEach, vi } from 'vitest';
31+
import { ObjectQL } from '@objectstack/objectql';
32+
import { SqlDriver } from '@objectstack/driver-sql';
33+
import { SysNotification, SysUser } from '@objectstack/platform-objects';
34+
import { MessagingService } from './messaging-service.js';
35+
import { SqlNotificationOutbox, DELIVERY_OBJECT } from './sql-outbox.js';
36+
import type { MessagingChannel } from './channel.js';
37+
import { NotificationDelivery, NotificationPreference } from './objects/index.js';
38+
import { DEFAULT_ORGANIZATION_ID, installDefaultOrganization } from './default-organization.testkit.js';
39+
40+
const SYS = { context: { isSystem: true, tenantId: DEFAULT_ORGANIZATION_ID } } as any;
41+
42+
const silent: any = { debug() {}, info() {}, warn() {}, error() {}, fatal() {}, child() { return silent; } };
43+
44+
let engine: ObjectQL | undefined;
45+
46+
afterEach(async () => {
47+
try { await engine?.destroy(); } catch { /* noop */ }
48+
engine = undefined;
49+
});
50+
51+
async function boot(): Promise<{ ql: ObjectQL; driver: SqlDriver }> {
52+
const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
53+
const ql = new ObjectQL({ logger: silent });
54+
ql.registerDriver(driver, true);
55+
await ql.init();
56+
for (const def of [SysUser, SysNotification, NotificationDelivery, NotificationPreference]) {
57+
ql.registry.registerObject(def as any, '@objectstack/service-messaging');
58+
}
59+
await ql.syncSchemas();
60+
await installDefaultOrganization(ql as any);
61+
engine = ql;
62+
return { ql, driver };
63+
}
64+
65+
async function user(ql: ObjectQL, name: string, email: string): Promise<string> {
66+
return String((await ql.insert('sys_user', { name, email }, SYS)).id);
67+
}
68+
69+
/** A mounted channel that is never reached: the outbox path only enqueues. */
70+
function channel(id: string): MessagingChannel {
71+
return { id, send: async () => { throw new Error(`${id}: the enqueue path never sends`); } } as MessagingChannel;
72+
}
73+
74+
function messaging(data: unknown, logger: any = silent): MessagingService {
75+
const outbox = new SqlNotificationOutbox(data as any, { partitionCount: 1 });
76+
const service = new MessagingService({ logger, getData: () => data as any, outbox });
77+
service.registerChannel(channel('email'));
78+
service.registerChannel(channel('sms'));
79+
return service;
80+
}
81+
82+
interface Delivery { recipient_id: string; recipient_user_id: string | null; channel: string; display_title: string | null }
83+
84+
async function deliveries(ql: ObjectQL): Promise<Delivery[]> {
85+
const rows = (await ql.find(DELIVERY_OBJECT, { fields: ['recipient_id', 'recipient_user_id', 'channel', 'display_title'] }, SYS)) ?? [];
86+
return rows
87+
.map((r: any) => ({
88+
recipient_id: r.recipient_id,
89+
recipient_user_id: r.recipient_user_id ?? null,
90+
channel: r.channel,
91+
display_title: r.display_title ?? null,
92+
}))
93+
.sort((a: Delivery, b: Delivery) => `${a.channel}|${a.recipient_id}`.localeCompare(`${b.channel}|${b.recipient_id}`));
94+
}
95+
96+
describe('the declaration', () => {
97+
it('recipient_id stays text; recipient_user_id is an optional sys_user reference that nulls when the user goes', () => {
98+
const fields = (NotificationDelivery as any).fields;
99+
expect(fields.recipient_id.type).toBe('text');
100+
expect(fields.recipient_id.required).toBe(true);
101+
const ref = fields.recipient_user_id;
102+
expect(ref.type).toBe('lookup');
103+
expect(ref.reference).toBe('sys_user');
104+
expect(ref.required).toBe(false);
105+
// The lookup default: the delivery is history and outlives the user.
106+
expect(ref.deleteBehavior ?? 'set_null').toBe('set_null');
107+
expect(ref.maxLength).toBeUndefined();
108+
});
109+
});
110+
111+
describe('the enqueue stamps recipient_user_id for a recipient a user holds, and only for one', () => {
112+
it('a user (bare id and email-resolved) is stamped; an unmatched email, a phone number and an unheld id are not', async () => {
113+
const { ql } = await boot();
114+
const bea = await user(ql, 'Bea Person', 'bea@example.com');
115+
const cal = await user(ql, 'Cal Person', 'cal@example.com');
116+
117+
const result = await messaging(ql).emit({
118+
topic: 'deal.won',
119+
audience: [bea, 'cal@example.com', 'outside@example.org', '+15550100', 'usr_nobody'],
120+
channels: ['email'],
121+
payload: { title: 'Won', body: 'A deal was won.' },
122+
});
123+
expect(result.enqueued).toBe(5);
124+
125+
expect((await deliveries(ql)).map((d) => [d.recipient_id, d.recipient_user_id])).toEqual([
126+
['+15550100', null],
127+
[bea, bea],
128+
[cal, cal],
129+
['outside@example.org', null],
130+
['usr_nobody', null],
131+
].sort((a, b) => `email|${a[0]}`.localeCompare(`email|${b[0]}`)));
132+
});
133+
});
134+
135+
describe('the record title names the person, and the address where there is none', () => {
136+
it('name; email where the name is blank; the address for an address recipient', async () => {
137+
const { ql, driver } = await boot();
138+
const bea = await user(ql, 'Bea Person', 'bea@example.com');
139+
const blank = await user(ql, 'Placeholder', 'no-name@example.com');
140+
await driver.getKnex()('sys_user').where({ id: blank }).update({ name: ' ' });
141+
142+
await messaging(ql).emit({ topic: 'deal.won', audience: [bea, blank, 'outside@example.org'], channels: ['email'], title: 't', body: 'b' } as any);
143+
await messaging(ql).emit({ topic: 'deal.won', audience: ['+15550100'], channels: ['sms'], title: 't', body: 'b' } as any);
144+
145+
const titles = Object.fromEntries((await deliveries(ql)).map((d) => [d.recipient_id, d.display_title]));
146+
expect(titles).toEqual({
147+
[bea]: 'email → Bea Person',
148+
[blank]: 'email → no-name@example.com',
149+
'outside@example.org': 'email → outside@example.org',
150+
'+15550100': 'sms → +15550100',
151+
});
152+
// Reversed: the raw user id was the title before.
153+
expect(titles[bea]).not.toContain(bea);
154+
});
155+
156+
it('deleting the user empties the reference, and the row is titled by the raw recipient again', async () => {
157+
const { ql } = await boot();
158+
const eve = await user(ql, 'Eve Person', 'eve@example.com');
159+
await messaging(ql).emit({ topic: 'deal.won', audience: [eve], channels: ['email'], title: 't', body: 'b' } as any);
160+
expect((await deliveries(ql))[0]).toMatchObject({ recipient_user_id: eve, display_title: 'email → Eve Person' });
161+
162+
await ql.delete('sys_user', { where: { id: eve }, context: { isSystem: true } });
163+
164+
// The delivery history stays; only the person leaves the title.
165+
expect(await deliveries(ql)).toEqual([
166+
{ recipient_id: eve, recipient_user_id: null, channel: 'email', display_title: `email → ${eve}` },
167+
]);
168+
});
169+
});
170+
171+
describe('a failed sys_user read stamps nothing and fails no emit', () => {
172+
it('the rows land titled by the raw recipient, and the failure is said once at warn', async () => {
173+
const { ql } = await boot();
174+
const bea = await user(ql, 'Bea Person', 'bea@example.com');
175+
// The real engine, with its sys_user list read failing.
176+
const failing = new Proxy(ql, {
177+
get(target, prop, receiver) {
178+
if (prop === 'find') {
179+
return (object: string, ...rest: unknown[]) => object === 'sys_user'
180+
? Promise.reject(new Error('directory unavailable'))
181+
: (target.find as any)(object, ...rest);
182+
}
183+
const value = Reflect.get(target, prop, receiver);
184+
return typeof value === 'function' ? value.bind(target) : value;
185+
},
186+
});
187+
const logger = { ...silent, warn: vi.fn() };
188+
189+
const result = await messaging(failing, logger).emit({ topic: 'deal.won', audience: [bea, 'outside@example.org'], channels: ['email'], title: 't', body: 'b' } as any);
190+
191+
expect(result.enqueued).toBe(2);
192+
expect((await deliveries(ql)).map((d) => [d.recipient_user_id, d.display_title])).toEqual([
193+
[null, `email → ${bea}`],
194+
[null, 'email → outside@example.org'],
195+
].sort((a, b) => String(a[1]).localeCompare(String(b[1]))));
196+
const said = logger.warn.mock.calls.map((c: unknown[]) => String(c[0])).filter((m: string) => m.includes("'sys_user' read"));
197+
expect(said).toHaveLength(1);
198+
expect(said[0]).toContain('directory unavailable');
199+
});
200+
});
201+
202+
describe('the title keeps the guarantees it had', () => {
203+
it('a row missing channel or recipient_id is refused, so the title never has a NULL part', async () => {
204+
const { ql } = await boot();
205+
const NOW = new Date('2026-10-22T00:00:00Z');
206+
for (const omit of ['channel', 'recipient_id']) {
207+
const row: Record<string, unknown> = {
208+
notification_id: `ntf_${omit}`, recipient_id: 'usr_alice', channel: 'email',
209+
status: 'pending', created_at: NOW, updated_at: NOW,
210+
};
211+
delete row[omit];
212+
await expect(ql.insert(DELIVERY_OBJECT, row, SYS)).rejects.toMatchObject({
213+
code: 'VALIDATION_FAILED',
214+
fields: expect.arrayContaining([expect.objectContaining({ field: omit, code: 'required' })]),
215+
});
216+
}
217+
});
218+
219+
it('the registered body points at display_title, which adds no stored column', async () => {
220+
const { ql, driver } = await boot();
221+
const registered = ql.registry.getObject(DELIVERY_OBJECT) as any;
222+
expect(registered.nameField).toBe('display_title');
223+
expect(registered.fields.display_title?.type).toBe('formula');
224+
const columns = Object.keys(await driver.getKnex()(DELIVERY_OBJECT).columnInfo());
225+
expect(columns).toContain('recipient_user_id');
226+
expect(columns).not.toContain('display_title');
227+
});
228+
});

‎packages/services/service-messaging/src/fan-out-system-context.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@
2222
* locale alone), the read that picks the language a keyed notification text is
2323
* rendered in, at delivery, for that recipient.
2424
*
25+
* [#25194] And by `MessagingService.userRecipients` (`sys_user`, the id alone),
26+
* the one read per emit that tells which recipients are a user, for the
27+
* delivery rows' `recipient_user_id`.
28+
*
2529
* `emit()` takes no caller context. The door in front of an emitting caller
2630
* has already decided whether it may notify, and every row these calls read or
2731
* write belongs to a RECIPIENT, not to the emitter — so no caller's grants

‎packages/services/service-messaging/src/messaging-service.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { type InboxCaller, resolveInboxRecipient } from './inbox-caller.js';
2525
import { FAN_OUT_SYSTEM_CONTEXT } from './fan-out-system-context.js';
2626
import { INBOX_SYSTEM_CONTEXT } from './inbox-system-context.js';
2727
import { assertActorReferenceResolves } from './actor-reference.js';
28+
import { USER_OBJECT } from './recipient-locale.js';
2829
import {
2930
assertNotificationTextKeys,
3031
type NotificationTextKeys,
@@ -1368,6 +1369,9 @@ export class MessagingService {
13681369
// report a channel as both refused and enqueued — which is exactly the
13691370
// ambiguity the once-per-channel refusal count below exists to remove.
13701371
const mounted = new Map<string, boolean>();
1372+
// [#25194] Which recipients are a user's id, read once per emit: the
1373+
// row's `recipient_user_id`, which titles it by the person.
1374+
const users = await this.userRecipients(targets.map((t) => t.recipient));
13711375
const isMounted = (id: string): boolean => {
13721376
let answer = mounted.get(id);
13731377
if (answer === undefined) {
@@ -1392,6 +1396,7 @@ export class MessagingService {
13921396
const id = await outbox.enqueue({
13931397
notificationId,
13941398
recipientId: recipient,
1399+
recipientUserId: users.has(recipient) ? recipient : undefined,
13951400
channel,
13961401
topic: input.topic,
13971402
payload: deliveryPayload,
@@ -1419,6 +1424,43 @@ export class MessagingService {
14191424
return deliveries;
14201425
}
14211426

1427+
/**
1428+
* [#25194] The recipients that are a `sys_user` id, in ONE read per emit.
1429+
*
1430+
* A resolved recipient is a user id or an address kept verbatim (an email
1431+
* no user holds, a phone number), and only the store can tell which: a
1432+
* bare spec passes through `RecipientResolver` unchecked. The answer stamps
1433+
* the delivery row's `recipient_user_id`, which the outbox writes past the
1434+
* engine's reference check (system context), so it is stamped only for an
1435+
* id this read found.
1436+
*
1437+
* The read is display-only: the row's title falls back to `recipient_id`
1438+
* without it, and no delivery is routed by it. So a failed read is a
1439+
* `warn` and an empty answer, never a failed emit.
1440+
*/
1441+
private async userRecipients(recipients: readonly string[]): Promise<ReadonlySet<string>> {
1442+
const data = this.ctx.getData?.();
1443+
const ids = [...new Set(recipients)];
1444+
if (!data || ids.length === 0) return new Set();
1445+
try {
1446+
// The explicit system opt-in — see FAN_OUT_SYSTEM_CONTEXT: a
1447+
// directory read on the recipients' behalf whose only use is the id.
1448+
const rows = await data.find(
1449+
USER_OBJECT,
1450+
{ where: { id: { $in: ids } }, fields: ['id'], limit: ids.length },
1451+
{ context: FAN_OUT_SYSTEM_CONTEXT },
1452+
);
1453+
return new Set((rows ?? []).map((r: any) => String(r?.id ?? '')).filter(Boolean));
1454+
} catch (err) {
1455+
this.ctx.logger.warn(
1456+
`[messaging] emit: the '${USER_OBJECT}' read for ${ids.length} recipient(s) failed ` +
1457+
`(${(err as Error)?.message ?? String(err)}); their delivery rows name no recipient user ` +
1458+
`and are titled by the raw recipient`,
1459+
);
1460+
return new Set();
1461+
}
1462+
}
1463+
14221464
/** Find an existing event id by its dedup key, tolerating lookup failure. */
14231465
private async findEventByDedupKey(data: IDataEngine, dedupKey: string): Promise<string | undefined> {
14241466
try {

‎packages/services/service-messaging/src/objects/notification-display-title.test.ts‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
/**
44
* The record title of the notification objects that declared a
5-
* `titleFormat` and no title pointer (#20044): `sys_notification_delivery`
6-
* and `sys_notification_receipt` (`sys_notification_subscription` is retired).
5+
* `titleFormat` and no title pointer (#20044): `sys_notification_receipt`
6+
* (`sys_notification_subscription` is retired).
77
*
88
* `sys_notification_preference` left this table when its `user_id` became a
99
* `sys_user` reference (ADR-0118 D1): its title now names the PERSON one hop
@@ -12,6 +12,11 @@
1212
* others to. Its case — the raw-id title 'usr_alice · billing.invoice · email'
1313
* — is reversed in `../preference-user-reference.integration.test.ts`.
1414
*
15+
* `sys_notification_delivery` left it the same way (#25194): its title names
16+
* the person one hop through `recipient_user_id`, and the address itself where
17+
* the recipient is not a user. Its case — the raw-id title 'email → usr_alice'
18+
* — is reversed in `../delivery-recipient-user.integration.test.ts`.
19+
*
1520
* ADR-0079 resolves a record's title as `nameField ?? displayNameField ??
1621
* derivation`, and an explicit `nameField` takes precedence over the
1722
* render-only `titleFormat`. With no pointer declared, the registry's
@@ -21,8 +26,9 @@
2126
* A renderer honouring the order therefore drew the raw id as the record
2227
* page's H1.
2328
*
24-
* The composite title points at `display_title`, a text formula over the
25-
* columns `titleFormat` names. `sys_notification_receipt`'s title is one
29+
* A composite title points at `display_title`, a text formula over the
30+
* columns `titleFormat` names; the `display_title` legs below hold any such
31+
* object to that. `sys_notification_receipt`'s title is one
2632
* column (`{state}`), so its `nameField` names that column directly — the
2733
* describe's own migration for a single-field title. An explicit pointer is
2834
* honoured whatever the field's type (ADR-0079 D4); `select` is only kept out
@@ -52,7 +58,6 @@ import {
5258
} from '@objectstack/objectql';
5359
import { SqlDriver } from '@objectstack/driver-sql';
5460
import { resolveDisplayField } from '@objectstack/spec/data';
55-
import { NotificationDelivery } from './notification-delivery.object.js';
5661
import { NotificationReceipt } from './notification-receipt.object.js';
5762
import { installDefaultOrganization } from '../default-organization.testkit.js';
5863

@@ -78,17 +83,6 @@ let seq = 0;
7883
const unique = (prefix: string) => `${prefix}_${++seq}`;
7984

8085
const CASES: Case[] = [
81-
{
82-
schema: NotificationDelivery,
83-
pointer: 'display_title',
84-
row: () => ({
85-
notification_id: unique('ntf'), recipient_id: 'usr_alice', channel: 'email',
86-
status: 'pending', created_at: NOW, updated_at: NOW,
87-
}),
88-
title: 'email → usr_alice',
89-
refused: ['channel', 'recipient_id'],
90-
defaulted: {},
91-
},
9286
{
9387
schema: NotificationReceipt,
9488
pointer: 'state',

0 commit comments

Comments
 (0)