Skip to content

Commit 0603bfa

Browse files
fix(service-messaging)!: sys_notification_template.channel accepts only email and sms, the channels that read a template (#24976)
Fixes #24924 Clause-②: no — nothing a write could send before is refused now; a save naming another channel gains one advisory in the write answer's existing `warnings`. ## What changes This is the 18.x half of execution card ② of the maintainer's ruling D on #24770. `sys_notification_template.channel` names a channel that reads a template row (`email` or `sms`), or the write is warned about. Patch round 1 replaced the refusal this PR first shipped: the seat's correction is on the card, comment 6300407976. - **Write warning.** The object declares one `format` validation rule, `channel_renders_template`, at `severity: 'warning'`. Its pattern is built from a single `TEMPLATE_CHANNELS` constant (`email`, `sms`). A write naming any other channel is **accepted and stored**, and it reports one advisory. The advisory carries `rule: 'channel_renders_template'`, `severity: 'warning'`, `field: 'channel'`, `code: 'invalid_format'`, and this message: "This channel never uses a notification template: only email and sms do, so this template will never be sent. For in-app (inbox) content, write an email template (email_template) and name it in the notify step's template; email and the inbox both render it." - **Copy.** The object description, the `all` view's empty state, and the `channel` and `subject` help now say which channels use a row and that the inbox never does. They are translated in zh-CN, ja-JP and es-ES, as is the warning. The es-ES and ja-JP leaves for the changed keys were untranslated English fills; they are translations now, so their provenance hashes drop. - **Docs.** In `content/docs/permissions/authentication.mdx`, the "SMS text customisation & localisation" paragraph loses the retired-seed sentence. It now states what PR #24930 made true: the auth SMS row is read in `tenancy.defaultOrgId()`. Under `single` that is the Default Organization's row. Under `isolated` and `group` no row is read and the built-in text is sent. - **Not touched:** `packages/spec/**` (the `script.config.template` tombstone half belongs to #24965 in the spec lane), `template-renderer.ts`, and the channels. ## Release reading (standing rule 甲) The ruling record (`6293376907`) reads: "② refuses stored or new non-email/sms rows. If no stored-row conversion carries that, 18.x warns on write and corrects the copy, and the refusal is filed for 19.0." - No ADR-0087 conversion carries this narrowing, because the rows are stored data of a platform object, not authored metadata. - So 18.x ships the warning half and the corrected copy. - The refusal of stored and new rows is filed for 19.0, by the seat. - Nothing in this PR refuses a write, and no pin asserts a refusal. ## Where the warning reaches (measured) Measured on a real ObjectQL with SqlDriver at this branch; the protocol half is a scratch test that is not committed. | Write | Accepted and stored | What the caller is answered | Server log | |---|---|---|---| | single-record create (`createData`) | yes | `warnings` with the hit | one line | | single-record update (`updateData`) | yes | `warnings` with the hit | one line | | import commit (`insertManyData`) | yes | outcome `ok: true`, no `warnings` | one line | | bulk update (`updateManyData`) | yes | `success`, no `warnings` | one line | | `email` / `sms` write; an edit of a stored row not naming `channel` | yes | no `warnings` key | none | - At the engine, insert, by-id update and predicate update each hand exactly one hit to that write's own `onValidationAdvisory` listener. These are pinned. - REST's create and update routes relay the protocol answer whole (`crud-routes.ts`, `res.status(201).json(result)` and `res.json(result)`), so their callers receive `warnings`. - The `warnings` member is the existing write-answer field the spec already declares (`CreateDataResponseSchema` / `UpdateDataResponseSchema` / `CloneDataResponseSchema`). No response field or other surface is added. - Bulk and import commits report the hit only in the log. Whether the console displays a create or update answer's `warnings` was not measured. - The log line reads: `Validation rule 'channel_renders_template' (warning): This channel never uses a notification template: …`. ## Why a `format` rule and not a `select` Option membership on a `select` is a field constraint in objectql's record validator. It refuses, and has no advisory severity, so it would be the 19.0 refusal and cannot be the 18.x warning. For whoever builds 19.0, measured in round 1 at this branch's base, the `select` spelling: - refuses with the generic "Channel must be one of: email, sms", with no per-field sentence to carry the email-template pointer; - admits `inbox` on an insert carrying an import's `keptOptionValues`; - moves the column from `varchar(64)` to `varchar(255)`. At the console pin `616df6598`, objectui's `SelectField` clears an out-of-set stored value on load. A Setup edit of a stored `inbox` row would therefore save `channel: null`, which is refused as `required`. That is a source reading, not a browser run. ## A correction to the copy the dispatch asked for The dispatch asked the copy to say that a notify node naming a `template` uses that email template on every channel. That is false for SMS at this commit. `sms-channel.ts` has no template path: it never reads `payload.template`, and it loads the `(topic, 'sms', locale)` row whether or not the notification names an email template. On the template path, `notify-node.ts` sets no title or body, so with no row the SMS body is the topic. The shipped copy says what is true: email and the inbox render the named email template, and SMS still uses its row. ## Tests - **New test file.** `packages/services/service-messaging/src/objects/notification-template-channel.integration.test.ts` has 15 cases on a real ObjectQL with SqlDriver over better-sqlite3. - Warnings: an insert naming `inbox`, `push`, `webhook`, `Email` or `' sms'`, an insert carrying `keptOptionValues`, a by-id update, a predicate update, and a write re-naming a stored row's channel. Each is accepted, the row is stored as written, and exactly one advisory names `channel_renders_template` on `channel` (`invalid_format`, `severity: 'warning'`), with the message head and `email_template`. - Silence: `email` and `sms` inserts, and an email-to-sms update, produce no advisory. - Stored-row preservation: editing other columns gives no advisory, delete works, and the row renders on neither channel. - **Ablation, rule inert.** The pattern became `[\s\S]*` through `scripts/ablation-replace.mjs`. The 9 warning cases went red ("expected [] to have a length of 1 but got +0") and the other 6 stayed green. Restored blob == HEAD `84c52411f6b9`; `git diff HEAD` is empty. - **Inverse ablation, rule warns on everything.** The pattern became `^(?!)$`. The 3 silence cases went red ("expected [ { …(5) } ] to deeply equal []") and the other 12 stayed green. Restored blob == HEAD `84c52411f6b9`. - **Package checks at `c7102b32f`.** `pnpm --filter @objectstack/service-messaging run typecheck` exits 0. The suite passes: 54 files, 608 tests. - **Import side.** `@objectstack/plugin-auth`'s `phone-sms-template-organization.integration.test.ts` and `phone-sms-seed-retired.test.ts` pass: 7 tests. - **Changeset level.** `check-changeset-no-major --base origin/main --event` with this body's declaration reads "LEVEL AXIS: this PR declares clause-② `no`", with no direction arm, and passes at `patch`. `check-adr-0087-registration` reads "this PR adds no declared-breaking changeset". - **Gates.** `dispatch-gates --commands` derives 98 commands at `c7102b32f`; each was run with its exit code captured, and the result is in the os-dev-report on the card. ## Acceptance notes - **SMS ignores a notify `template`** (see the correction above). Not filed: no measured producer sends a templated notify on `sms`. HotCRM's 19 templated notify nodes use inbox and email, and the examples name no template, so there is no reach reading. Carrier: the `domain:services` seat (the #24770 family). Dedupe words: sms channel notify template ignored; payload.template sms; templated notify sms topic body. - **Bulk updates and import commits** answer the warning only in the server log, not in their answer (measured above). This PR adds no surface to carry it. Noted, not filed. Carrier: none. - `origin/main` is ahead of this branch's last merge (`5d27d326c`) and does not conflict (a local merge-tree is clean), so it was not merged again. --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ba69b16 commit 0603bfa

10 files changed

Lines changed: 350 additions & 33 deletions
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 `sys_notification_template` saved for a channel other than `email` or `sms` is now warned about, because no channel ever sends it
6+
7+
A notification template row is read by exactly two channels: the email channel (when the notification names no email template) and the SMS channel, which also renders the phone OTP and invitation texts. The in-app inbox reads no row on any path. `channel` is free text, so a row saved for `inbox`, `push` or any other value was stored and then rendered nowhere, with nothing to say so.
8+
9+
The object now carries a `channel_renders_template` rule at `severity: 'warning'`. A write naming another channel is still accepted and stored, and it now reports the warning. A single-record create, clone or update answers it in the write answer's `warnings` (`rule`, `severity`, `field: 'channel'`, `code`, `message`), and the server logs it once per write. Bulk updates and imports log it. The message points in-app content to an email template (`email_template`) named in a notify step's `template`. The object's description, its empty state and the `channel` / `subject` help now say which channels use a row and that the inbox never does.
10+
11+
Clause-②: no — nothing a write could send before is refused now; a save naming another channel gains one advisory in the write answer's existing `warnings`.
12+
13+
- **Unchanged.** Every write that was accepted is still accepted, and stored rows are not read, rewritten or flagged by anything but a write that names their channel. `email` and `sms` rows, how every channel renders, and the column and its width are unchanged.
14+
- **What to do.** For in-app content, write an email template and name it in the notify step's `template`; the inbox and email both render it. A row for another channel was never sent, so delete it if you no longer want it listed.

‎content/docs/permissions/authentication.mdx‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,22 @@ sets their own password.
443443

444444
#### SMS text customisation & localisation
445445

446-
The OTP and invitation bodies are localised and tenant-customisable: a
447-
`sys_notification_template` row for `(auth.phone_otp | auth.phone_invite,
448-
channel 'sms', locale)` wins — built-in English and Chinese rows are seeded
449-
once (never overwriting your edits) and can be changed under Setup →
450-
Notification Templates. Both bodies resolve the same way: the recipient's own
446+
The OTP and invitation bodies are localised, and built-in English and Chinese
447+
texts are always available. A `sys_notification_template` row for
448+
`(auth.phone_otp | auth.phone_invite, channel 'sms', locale)`, authored under
449+
Setup → Notification Templates, replaces the built-in text. No such row ships
450+
with the platform: one exists only once someone authors it.
451+
452+
An auth SMS belongs to no organization of its own, so the row is read in the
453+
organization the send resolves to, `tenancy.defaultOrgId()`:
454+
455+
- Under the `single` posture that is the Default Organization, so the Default
456+
Organization's row is used.
457+
- Under `isolated` and `group` it resolves to no organization: no row is read
458+
and the built-in text is sent, so one organization's row never decides the
459+
text another organization's user receives.
460+
461+
Both bodies pick their language the same way: the recipient's own
451462
`sys_user.locale` when their account has one, and the deployment default
452463
(`localization.locale` setting) otherwise — the account is matched on its
453464
`phone_number`, so a number no account carries takes the deployment default
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `sys_notification_template.channel` names a channel that reads a row, or the
5+
* write is WARNED about: the 18.x half of the maintainer's ruling D on #24770
6+
* (card #24924).
7+
*
8+
* Only `email` and `sms` read a row (the email channel's inline path, the SMS
9+
* channel, and plugin-auth's phone OTP / invitation SMS); the inbox reads none
10+
* on any path. A row naming any other channel used to be stored and then
11+
* rendered nowhere, silently. The ruling refuses such rows, but no ADR-0087
12+
* conversion carries stored DATA rows, so in 18.x the write is ACCEPTED with a
13+
* warning and the refusal waits for 19.0. ⛔ Nothing here asserts a refusal.
14+
*
15+
* The warning is the object's `channel_renders_template` rule at
16+
* `severity: 'warning'` — objectql's advisory path. One hit is handed to the
17+
* write's own `onValidationAdvisory` listener (the data protocol answers it as
18+
* the single-record write answer's `warnings`) and logged once per write.
19+
*
20+
* Pinned on a real `ObjectQL` over `SqlDriver` (better-sqlite3), through every
21+
* engine write shape the rule validator runs on:
22+
*
23+
* - WARNING: an insert, an insert carrying an import's kept option values, a
24+
* by-id update and a predicate (`multi`) update that name another channel
25+
* are accepted, the row is stored as written, and exactly one advisory names
26+
* `channel_renders_template` / `channel` with the message head.
27+
* - SILENCE: `email` and `sms` writes produce no advisory (the positive twins,
28+
* so no warning pin passes against a rule that warns about everything).
29+
* - STORED ROWS: a row already stored with another channel (written under
30+
* the rule's absence, straight through the driver) is read, edited in any
31+
* other column and deleted with no advisory, and still renders on no
32+
* channel. A write that names its channel again is warned about, and
33+
* accepted.
34+
*/
35+
36+
import { describe, it, expect, afterEach } from 'vitest';
37+
import { ObjectQL } from '@objectstack/objectql';
38+
import { SqlDriver } from '@objectstack/driver-sql';
39+
import type { ExecutionContext } from '@objectstack/spec/kernel';
40+
import type { ValidationAdvisoryEvent } from '@objectstack/spec/data';
41+
import { NotificationTemplate } from './notification-template.object.js';
42+
import { NotificationTemplateStore, TEMPLATE_OBJECT } from '../template-renderer.js';
43+
44+
const SYS_ORGANIZATION = { name: 'sys_organization', label: 'Organization', managedBy: 'better-auth', fields: { name: { type: 'text', label: 'Name' } } };
45+
46+
const ORG = 'org_a';
47+
const TOPIC = 'deal.won';
48+
const CTX = { isSystem: true, tenantId: ORG } as ExecutionContext;
49+
50+
const RULE = 'channel_renders_template';
51+
/** The first sentence of the warning — what the person who saved reads. */
52+
const WARNING_HEAD = 'This channel never uses a notification template';
53+
54+
let engine: ObjectQL | undefined;
55+
56+
afterEach(async () => {
57+
try { await engine?.destroy(); } catch { /* noop */ }
58+
engine = undefined;
59+
});
60+
61+
async function boot(): Promise<{ ql: ObjectQL; driver: SqlDriver }> {
62+
const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
63+
const ql = new ObjectQL();
64+
ql.registerDriver(driver, true);
65+
await ql.init();
66+
for (const def of [SYS_ORGANIZATION, NotificationTemplate]) {
67+
ql.registry.registerObject(def as any, '@objectstack/service-messaging');
68+
}
69+
await ql.syncSchemas();
70+
ql.setTenancyPostureProvider(() => 'single');
71+
await ql.insert('sys_organization', { id: ORG, name: ORG }, { context: { isSystem: true } as ExecutionContext });
72+
engine = ql;
73+
return { ql, driver };
74+
}
75+
76+
function row(channel: string, text = channel): Record<string, unknown> {
77+
return { topic: TOPIC, channel, locale: 'en', subject: `${text} subject`, body: `${text} body`, format: 'text', is_active: true };
78+
}
79+
80+
async function channelsStored(ql: ObjectQL): Promise<string[]> {
81+
const rows = (await ql.find(TEMPLATE_OBJECT, { fields: ['channel'] }, { context: CTX })) ?? [];
82+
return rows.map((r: Record<string, unknown>) => String(r.channel)).sort();
83+
}
84+
85+
/** A fresh listener for ONE write: the advisories that write reported. */
86+
function advisories(): { events: ValidationAdvisoryEvent[]; onValidationAdvisory: (e: ValidationAdvisoryEvent) => void } {
87+
const events: ValidationAdvisoryEvent[] = [];
88+
return { events, onValidationAdvisory: (e) => { events.push(e); } };
89+
}
90+
91+
/** Exactly one advisory, and it is this rule's, on `channel`, with the warning's head and prescription. */
92+
function expectOneChannelWarning(events: readonly ValidationAdvisoryEvent[]): void {
93+
expect(events).toHaveLength(1);
94+
expect(events[0]).toMatchObject({ rule: RULE, severity: 'warning', field: 'channel', code: 'invalid_format' });
95+
expect(String(events[0].message).startsWith(WARNING_HEAD)).toBe(true);
96+
// The prescription: in-app content belongs to an email template, named by
97+
// a notify step's `template`.
98+
expect(String(events[0].message)).toContain('email_template');
99+
}
100+
101+
describe('sys_notification_template.channel: a channel that reads no row is accepted with a warning', () => {
102+
it.each(['email', 'sms'])('an insert naming %j is stored with no advisory (the positive twins)', async (channel) => {
103+
const { ql } = await boot();
104+
const seen = advisories();
105+
await ql.insert(TEMPLATE_OBJECT, row(channel), { context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
106+
expect(seen.events).toEqual([]);
107+
expect(await channelsStored(ql)).toEqual([channel]);
108+
});
109+
110+
it.each(['inbox', 'push', 'webhook', 'Email', ' sms'])('an insert naming %j is stored, with one channel advisory', async (channel) => {
111+
const { ql } = await boot();
112+
const seen = advisories();
113+
await ql.insert(TEMPLATE_OBJECT, row(channel), { context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
114+
expectOneChannelWarning(seen.events);
115+
expect(await channelsStored(ql)).toEqual([channel]);
116+
});
117+
118+
it('an insert carrying an import\'s kept option values is stored, with one channel advisory', async () => {
119+
const { ql } = await boot();
120+
const seen = advisories();
121+
const importCtx = { ...CTX, keptOptionValues: { channel: ['inbox'] } } as ExecutionContext;
122+
await ql.insert(TEMPLATE_OBJECT, row('inbox'), { context: importCtx, onValidationAdvisory: seen.onValidationAdvisory });
123+
expectOneChannelWarning(seen.events);
124+
expect(await channelsStored(ql)).toEqual(['inbox']);
125+
});
126+
127+
it('a by-id update moving a row to another channel is stored, with one channel advisory', async () => {
128+
const { ql } = await boot();
129+
const created = await ql.insert(TEMPLATE_OBJECT, row('email'), { context: CTX });
130+
const seen = advisories();
131+
await ql.update(TEMPLATE_OBJECT, { channel: 'inbox' }, { where: { id: created.id }, context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
132+
expectOneChannelWarning(seen.events);
133+
expect(await channelsStored(ql)).toEqual(['inbox']);
134+
});
135+
136+
it('a predicate update moving a row to another channel is stored, with one channel advisory', async () => {
137+
const { ql } = await boot();
138+
await ql.insert(TEMPLATE_OBJECT, row('email'), { context: CTX });
139+
const seen = advisories();
140+
await ql.update(TEMPLATE_OBJECT, { channel: 'inbox' }, { where: { topic: TOPIC }, multi: true, context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
141+
expectOneChannelWarning(seen.events);
142+
expect(await channelsStored(ql)).toEqual(['inbox']);
143+
});
144+
145+
it('an update of an email row that does not move its channel produces no advisory', async () => {
146+
const { ql } = await boot();
147+
const created = await ql.insert(TEMPLATE_OBJECT, row('email'), { context: CTX });
148+
const seen = advisories();
149+
await ql.update(TEMPLATE_OBJECT, { channel: 'sms', body: 'moved' }, { where: { id: created.id }, context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
150+
expect(seen.events).toEqual([]);
151+
expect(await channelsStored(ql)).toEqual(['sms']);
152+
});
153+
154+
describe('a row already stored with another channel', () => {
155+
/** Written straight through the driver: the shape a row authored before the rule has. */
156+
async function bootWithStoredInboxRow(): Promise<{ ql: ObjectQL; id: string }> {
157+
const booted = await boot();
158+
const id = 'tpl_stored_inbox';
159+
await booted.driver.create(TEMPLATE_OBJECT, { id, organization_id: ORG, ...row('inbox', 'stored') });
160+
return { ql: booted.ql, id };
161+
}
162+
163+
it('is read and edited in every other column as before, with no advisory', async () => {
164+
const { ql, id } = await bootWithStoredInboxRow();
165+
const seen = advisories();
166+
await ql.update(TEMPLATE_OBJECT, { body: 'edited body', is_active: false }, { where: { id }, context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
167+
expect(seen.events).toEqual([]);
168+
const [stored] = await ql.find(TEMPLATE_OBJECT, { where: { id } }, { context: CTX });
169+
expect(stored).toMatchObject({ id, channel: 'inbox', body: 'edited body', is_active: false });
170+
});
171+
172+
it('still renders on no channel', async () => {
173+
const { ql, id } = await bootWithStoredInboxRow();
174+
// Control: the row is really there for this topic and organization.
175+
expect((await ql.find(TEMPLATE_OBJECT, { where: { id } }, { context: CTX })).length).toBe(1);
176+
const store = new NotificationTemplateStore({ getData: () => ql as any });
177+
expect(await store.load(TOPIC, 'email', 'en', ORG)).toBeNull();
178+
expect(await store.load(TOPIC, 'sms', 'en', ORG)).toBeNull();
179+
});
180+
181+
it('is deleted as before', async () => {
182+
const { ql, id } = await bootWithStoredInboxRow();
183+
await ql.delete(TEMPLATE_OBJECT, { where: { id }, context: CTX });
184+
expect(await channelsStored(ql)).toEqual([]);
185+
});
186+
187+
it('a write that names its channel again is stored, with one channel advisory', async () => {
188+
const { ql, id } = await bootWithStoredInboxRow();
189+
const seen = advisories();
190+
await ql.update(TEMPLATE_OBJECT, { channel: 'inbox', body: 'x' }, { where: { id }, context: CTX, onValidationAdvisory: seen.onValidationAdvisory });
191+
expectOneChannelWarning(seen.events);
192+
const [stored] = await ql.find(TEMPLATE_OBJECT, { where: { id } }, { context: CTX });
193+
expect(stored).toMatchObject({ channel: 'inbox', body: 'x' });
194+
});
195+
});
196+
});

0 commit comments

Comments
 (0)