From 4258f0592659659ba893749b49c3839e2cfe69a9 Mon Sep 17 00:00:00 2001 From: jubnl Date: Wed, 23 Sep 2026 20:33:07 +0200 Subject: [PATCH] refactor(notifications): in-app notifications and channel preferences through repositories --- ...tificationChannelPreferences.repository.ts | 57 +++- .../repositories/Notifications.repository.ts | 257 ++++++++++++++++- .../notification-preferences.service.ts | 66 ++--- .../notifications/notifications.module.ts | 16 +- .../notifications/notifications.service.ts | 158 ++++------- server/tests/helpers/notifications-repos.ts | 26 ++ server/tests/helpers/notifications.ts | 23 +- ...ationChannelPreferences.repository.test.ts | 123 +++++++++ .../Notifications.repository.test.ts | 258 ++++++++++++++++++ .../tests/unit/nest/channel-override.test.ts | 10 +- .../notification-preferences.service.test.ts | 10 +- .../unit/nest/notifications.service.test.ts | 18 ++ 12 files changed, 879 insertions(+), 143 deletions(-) create mode 100644 server/tests/helpers/notifications-repos.ts create mode 100644 server/tests/unit/db/repositories/NotificationChannelPreferences.repository.test.ts create mode 100644 server/tests/unit/db/repositories/Notifications.repository.test.ts diff --git a/server/src/db/repositories/NotificationChannelPreferences.repository.ts b/server/src/db/repositories/NotificationChannelPreferences.repository.ts index fb184f19f..9a3444afc 100644 --- a/server/src/db/repositories/NotificationChannelPreferences.repository.ts +++ b/server/src/db/repositories/NotificationChannelPreferences.repository.ts @@ -1,4 +1,59 @@ import type { NotificationChannelPreferences } from '../entities/NotificationChannelPreferences.entity'; import { TrekRepository } from './_shared/trek-repository'; -export class NotificationChannelPreferencesRepository extends TrekRepository {} +/** NP3's projection — `event_type, channel, enabled` (no need for the `user`/`user_id` PK columns; the caller already knows which user it asked for). */ +export interface NotificationChannelPreferenceRow { + event_type: string; + channel: string; + enabled: number; +} + +/** + * `notification_channel_preferences` — per-user, per-event, per-channel + * opt-outs. A genuine THREE-column composite primary key (`user` relation + + * `event_type` text + `channel` text, all `.primary()` — R5/§14's "wider + * than any prior plan's composite-PK precedent" finding): every write here + * goes through `em.upsert`/`nativeDelete` targeting all three columns as the + * conflict/filter key, never a synthetic `id`. `user_id` is a `persist(false)` + * mirror of the `user` relation (the program-wide trap) — filtered/written + * through the `user` relation property throughout, never the mirror. + */ +export class NotificationChannelPreferencesRepository extends TrekRepository { + /** NP2 — `SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?`. `enabled` is not a persist(false) mirror, so `fields` narrowing is exact. */ + async findEnabled(userId: number, eventType: string, channel: string): Promise<{ enabled: number } | null> { + return await this.findOne({ user: userId, event_type: eventType, channel }, { fields: ['enabled'] }); + } + + /** NP3 — `SELECT event_type, channel, enabled FROM notification_channel_preferences WHERE user_id = ?` (`getPreferencesMatrix`'s full per-user override set, merged with defaults in the service). */ + async listForUser(userId: number): Promise { + return await this.find({ user: userId }, { fields: ['event_type', 'channel', 'enabled'] }); + } + + /** + * NP5 — `applyUserChannelPrefs`'s `INSERT OR REPLACE INTO + * notification_channel_preferences (user_id, event_type, channel, enabled) + * VALUES (?, ?, ?, ?)`, run once per explicitly-disabled preference. The + * composite-PK `em.upsert` shape Task 0 (R5) pinned against the migration + * DDL (`user_id, event_type, channel` PRIMARY KEY) and worked an example + * for — `onConflictFields` names the THREE primary-key properties by their + * entity property names (`user`, not the persist(false) `user_id` mirror), + * and `onConflictAction: 'merge'` reproduces `OR REPLACE`'s "overwrite the + * existing row" semantics (the `enabled` column is the only non-key column, + * so there is nothing REPLACE resets that merge wouldn't also set). + * Rendered SQL pinned directly (not assumed from the shape alone) by + * `NotificationChannelPreferences.repository.test.ts`'s + * `NCPREPO-UPSERT-SQL` case — the FIRST such pin in the program (Task 6/ + * `UserNoticeDismissalsRepository` re-checks against this one, per R5). + */ + async upsertPreference(userId: number, eventType: string, channel: string, enabled: number): Promise { + await this.upsert( + { user: userId, event_type: eventType, channel, enabled }, + { onConflictFields: ['user', 'event_type', 'channel'], onConflictAction: 'merge' }, + ); + } + + /** NP6 — `applyUserChannelPrefs`'s `DELETE FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?` (removing the row is how "enabled (default)" is represented — never a write of `enabled = 1`). */ + async deletePreference(userId: number, eventType: string, channel: string): Promise { + return await this.nativeDelete({ user: userId, event_type: eventType, channel }); + } +} diff --git a/server/src/db/repositories/Notifications.repository.ts b/server/src/db/repositories/Notifications.repository.ts index c7b2ebb9c..11daeacc2 100644 --- a/server/src/db/repositories/Notifications.repository.ts +++ b/server/src/db/repositories/Notifications.repository.ts @@ -1,4 +1,259 @@ import type { Notifications } from '../entities/Notifications.entity'; +import { type AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; -export class NotificationsRepository extends TrekRepository {} +/** + * A bare `notifications` row — every scalar column of the entity, incl. the + * two `persist(false)` relation mirrors (`sender_id`, `recipient_id` — the + * program-wide trap: a bare wildcard/`fields:`-narrowed ORM read silently + * drops these, so every full-row read below goes through Kysely with an + * explicit column list instead). + */ +export interface NotificationRow { + id: number; + type: string; + scope: string; + target: number; + sender_id: number | null; + recipient_id: number; + title_key: string; + title_params: string | null; + text_key: string; + text_params: string | null; + positive_text_key: string | null; + negative_text_key: string | null; + positive_callback: string | null; + negative_callback: string | null; + response: string | null; + navigate_text_key: string | null; + navigate_target: string | null; + is_read: number | null; + created_at: string | null; +} + +const _notificationRowKeys: AssertRowKeys = true; + +/** NT10/NT22's joined shape — `n.*, u.username AS sender_username, u.avatar AS sender_avatar`. */ +export interface NotificationJoinRow extends NotificationRow { + sender_username: string | null; + sender_avatar: string | null; +} + +/** The 15-column insert shape NT5/NT8 share (`id`/`response`/`is_read`/`created_at` are autoincrement/defaulted, matching the legacy column list). */ +export interface NotificationInsertRow { + type: string; + scope: string; + target: number; + sender_id: number | null; + recipient_id: number; + title_key: string; + title_params: string; + text_key: string; + text_params: string; + positive_text_key: string | null; + negative_text_key: string | null; + positive_callback: string | null; + negative_callback: string | null; + navigate_text_key: string | null; + navigate_target: string | null; +} + +/** + * The minimal cross-table read shapes this repository needs for + * `resolveRecipients`/sender-info lookups (NT1/NT2/NT3/NT4/NT6/NT23/NT24). + * `trips`/`trip_members`/`users` are other plans' tables (3c/3b) — reached + * here via Kysely the same way `CollabMessagesRepository`/`BudgetItemsRepository` + * read `users`/`trip_files` for their own domain-specific joins, rather than + * a new cross-domain `@InjectRepository`, which would widen this module's + * `MikroOrmModule.forFeature` registration (and every OTHER module that + * constructs `NotificationsService`, per the BOOT GATE note) for reads that + * are each a single, narrow, already-indexed lookup. + */ +interface NotificationsKyselyDB { + notifications: NotificationRow; + users: { id: number; username: string; avatar: string | null; is_guest: number; role: string }; + trips: { id: number; user_id: number }; + trip_members: { trip_id: number; user_id: number }; +} + +/** + * `notifications` — the in-app notification store. Kysely throughout for + * `notifications` itself: `sender_id`/`recipient_id` are `persist(false)` + * mirrors (the `TripFilesRepository`/`BudgetItemsRepository` class docstring + * trap). The generic ORM helpers (`count`/`nativeUpdate`/`nativeDelete`/ + * `insert`) are used wherever the statement doesn't need a full-row + * projection, since `is_read`/`response`/the relation properties themselves + * are ordinary (non-mirror) columns. + */ +export class NotificationsRepository extends TrekRepository { + private joinedQuery() { + return this.kysely() + .selectFrom('notifications as n') + .leftJoin('users as u', 'u.id', 'n.sender_id') + .selectAll('n') + .select(['u.username as sender_username', 'u.avatar as sender_avatar']); + } + + // ── Recipient resolution (NT1-NT4) ───────────────────────────────────── + + /** NT1 — `SELECT user_id FROM trips WHERE id = ?` (the trip owner). */ + async getTripOwnerId(tripId: number): Promise { + const row = await this.kysely() + .selectFrom('trips') + .select('user_id') + .where('id', '=', tripId) + .executeTakeFirst(); + return row?.user_id ?? null; + } + + /** + * NT2 — the guest-exclusion chokepoint (#1362): `SELECT m.user_id FROM + * trip_members m JOIN users u ON u.id = m.user_id WHERE m.trip_id = ? AND + * COALESCE(u.is_guest, 0) = 0`. `is_guest` is a non-nullable + * `p.integer().default(0)` column on `Users` (never actually NULL), so a + * plain `= 0` reproduces the legacy `COALESCE(..., 0) = 0` result set + * exactly — the COALESCE was defensive, not load-bearing on this schema. + * This is THE single chokepoint for in-app/email/webhook/ntfy recipient + * resolution across the whole app (the service's own doc comment) — + * preserved byte-exact, not "helpfully" folded into a relation walk. + */ + async listNonGuestTripMemberIds(tripId: number): Promise { + const rows = await this.kysely() + .selectFrom('trip_members as m') + .innerJoin('users as u', 'u.id', 'm.user_id') + .select('m.user_id') + .where('m.trip_id', '=', tripId) + .where('u.is_guest', '=', 0) + .execute(); + return rows.map((r) => r.user_id); + } + + /** NT3 — `SELECT is_guest FROM users WHERE id = ?` (a guest todo-assignee, scope='user', is never notified). */ + async findGuestFlag(userId: number): Promise<{ is_guest: number } | undefined> { + return await this.kysely() + .selectFrom('users') + .select('is_guest') + .where('id', '=', userId) + .executeTakeFirst(); + } + + /** NT4 — `SELECT id FROM users WHERE role = ? AND COALESCE(is_guest, 0) = 0` — same non-nullable-column reasoning as `listNonGuestTripMemberIds`. */ + async listNonGuestUserIdsByRole(role: string): Promise { + const rows = await this.kysely() + .selectFrom('users') + .select('id') + .where('role', '=', role) + .where('is_guest', '=', 0) + .execute(); + return rows.map((r) => r.id); + } + + /** NT6/NT23/NT24 — `SELECT username, avatar FROM users WHERE id = ?` (sender info for WS payloads and the dev-fallback self-notification), three identical-text call sites folded onto one method. */ + async findUserBasic(userId: number): Promise<{ username: string; avatar: string | null } | undefined> { + return await this.kysely() + .selectFrom('users') + .select(['username', 'avatar']) + .where('id', '=', userId) + .executeTakeFirst(); + } + + // ── Notification CRUD ─────────────────────────────────────────────────── + + /** NT5/NT8 — the 15-column insert, shared by `createNotification`'s per-recipient loop and `createNotificationForRecipient`'s single insert. Returns the new row's id. */ + async insertNotification(row: NotificationInsertRow): Promise { + return await this.insert({ + type: row.type, + scope: row.scope, + target: row.target, + sender: row.sender_id, + recipient: row.recipient_id, + title_key: row.title_key, + title_params: row.title_params, + text_key: row.text_key, + text_params: row.text_params, + positive_text_key: row.positive_text_key, + negative_text_key: row.negative_text_key, + positive_callback: row.positive_callback, + negative_callback: row.negative_callback, + navigate_text_key: row.navigate_text_key, + navigate_target: row.navigate_target, + }); + } + + /** NT7/NT9/NT19 — `SELECT * FROM notifications WHERE id = ?`, full row incl. the persist(false) mirrors. */ + async findById(id: number): Promise { + return await this.kysely().selectFrom('notifications').selectAll().where('id', '=', id).executeTakeFirst(); + } + + /** NT19 — `respond`'s recipient-scoped initial read: `SELECT * FROM notifications WHERE id = ? AND recipient_id = ?`. */ + async findByIdForRecipient(id: number, recipientId: number): Promise { + return await this.kysely() + .selectFrom('notifications') + .selectAll() + .where('id', '=', id) + .where('recipient_id', '=', recipientId) + .executeTakeFirst(); + } + + /** NT10 — `listInApp`'s joined page read, `unreadOnly` toggling the extra `is_read = 0` predicate (the legacy statement's two dynamic-WHERE variants). */ + async listForRecipient(recipientId: number, limit: number, offset: number, unreadOnly: boolean): Promise { + let q = this.joinedQuery().where('n.recipient_id', '=', recipientId); + if (unreadOnly) q = q.where('n.is_read', '=', 0); + return await q.orderBy('n.created_at', 'desc').limit(limit).offset(offset).execute(); + } + + /** NT22 — `respond`'s post-update re-select: the same joined shape, `WHERE n.id = ?`, deliberately unscoped by recipient (the id was already recipient-scope-verified by `findByIdForRecipient` earlier in the same call). */ + async findWithSenderById(id: number): Promise { + return await this.joinedQuery().where('n.id', '=', id).executeTakeFirst(); + } + + /** NT11/NT12 — `listInApp`'s `total`/`unread_count` reads. Neither `recipient`(relation)/`is_read` is a persist(false) mirror, so the generic `count()` helper is exact. */ + async countForRecipient(recipientId: number, unreadOnly: boolean): Promise { + return await this.count(unreadOnly ? { recipient: recipientId, is_read: 0 } : { recipient: recipientId }); + } + + /** NT12/NT13 — `SELECT COUNT(*) ... WHERE recipient_id = ? AND is_read = 0`, the same predicate `listInApp`'s `unread_count` and the standalone `unreadCount` method both need. */ + async countUnreadForRecipient(recipientId: number): Promise { + return await this.count({ recipient: recipientId, is_read: 0 }); + } + + /** NT14/NT15 — `markRead`/`markUnread`: `UPDATE notifications SET is_read = ? WHERE id = ? AND recipient_id = ?`. Returns the affected-row count (0 or 1). */ + async setRead(id: number, recipientId: number, isRead: 0 | 1): Promise { + return await this.nativeUpdate({ id, recipient: recipientId }, { is_read: isRead }); + } + + /** NT16 — `markAllRead`: `UPDATE notifications SET is_read = 1 WHERE recipient_id = ? AND is_read = 0`. */ + async markAllRead(recipientId: number): Promise { + return await this.nativeUpdate({ recipient: recipientId, is_read: 0 }, { is_read: 1 }); + } + + /** NT17 — `deleteOne`: `DELETE FROM notifications WHERE id = ? AND recipient_id = ?`. */ + async deleteForRecipient(id: number, recipientId: number): Promise { + return await this.nativeDelete({ id, recipient: recipientId }); + } + + /** NT18 — `deleteAll`: `DELETE FROM notifications WHERE recipient_id = ?`. */ + async deleteAllForRecipient(recipientId: number): Promise { + return await this.nativeDelete({ recipient: recipientId }); + } + + /** + * NT20 — `respond`'s atomic claim, preserved byte-exact: `UPDATE + * notifications SET response = ?, is_read = 1 WHERE id = ? AND + * recipient_id = ? AND response IS NULL`. `response: null` in the filter + * compiles to `response IS NULL` (MikroORM's standard null-filter + * semantics) — only updates, and only returns a nonzero count, if no + * concurrent responder has already claimed this notification. This is the + * security-critical double-submit fix (R8): claim BEFORE the handler runs, + * so a concurrent second call's UPDATE affects 0 rows and can never + * execute the callback action a second time. + */ + async claimResponse(id: number, recipientId: number, response: string): Promise { + return await this.nativeUpdate({ id, recipient: recipientId, response: null }, { response, is_read: 1 }); + } + + /** NT21 — `respond`'s handler-failure release: `UPDATE notifications SET response = NULL, is_read = ? WHERE id = ? AND recipient_id = ?`, restoring the claim so the user can retry. */ + async releaseResponse(id: number, recipientId: number, isRead: number | null): Promise { + await this.nativeUpdate({ id, recipient: recipientId }, { response: null, is_read: isRead }); + } +} diff --git a/server/src/nest/notifications/notification-preferences.service.ts b/server/src/nest/notifications/notification-preferences.service.ts index 430d4f590..e80543ff8 100644 --- a/server/src/nest/notifications/notification-preferences.service.ts +++ b/server/src/nest/notifications/notification-preferences.service.ts @@ -1,5 +1,9 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { InjectRepository } from '@mikro-orm/nestjs'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; +import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository'; +import { NotificationChannelPreferences } from '../../db/entities/NotificationChannelPreferences.entity'; +import type { NotificationChannelPreferencesRepository } from '../../db/repositories/NotificationChannelPreferences.repository'; import { UnitOfWork } from '../database/unit-of-work'; import { MailerService } from './mailer/mailer.service'; import { listChannels } from './channel-registry'; @@ -30,19 +34,27 @@ export interface PreferencesMatrix { * * It reads the live channel set from the registry but never sends anything — * the dispatch loop in NotificationsService applies what is decided here. + * + * DI-native (Plan 3f Task 3): the `SELECT value FROM app_settings WHERE + * key = ?`/`INSERT OR REPLACE INTO app_settings ...` reads/writes (NP1/NP4) + * moved onto the shared `AppSettingsRepository.getValue`/`.setValue` pair + * (already built, Plan 3a) rather than a new method — this is the first of + * six identical `app_settings` single-key sites in this cluster to converge + * on it (plan3f-sql-inventory.md §15 surprise 8). `notification_channel_preferences`'s + * reads/writes (NP2/NP3/NP5/NP6) go through `NotificationChannelPreferencesRepository`, + * whose `upsertPreference`/`deletePreference` target the table's genuine + * 3-column composite primary key (`user`, `event_type`, `channel`) via + * `em.upsert({ onConflictFields: [...] })`, per Task 0's R5 pin. */ @Injectable() export class NotificationPreferencesService { constructor( - private readonly db: DatabaseService, private readonly mailer: MailerService, private readonly uow: UnitOfWork, + @InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository, + @InjectRepository(NotificationChannelPreferences) private readonly channelPrefs: NotificationChannelPreferencesRepository, ) {} - private async getAppSetting(key: string): Promise { - return this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value || null; - } - /** * Channels implemented for an event. In-app takes everything; external channels * decide for themselves (today: everything except `synology_session_cleared`). @@ -71,7 +83,7 @@ export class NotificationPreferencesService { * would silently drop any that were). */ async getActiveChannels(): Promise { - const raw = (await this.getAppSetting('notification_channels')) || (await this.getAppSetting('notification_channel')) || 'none'; + const raw = (await this.appSettings.getValue('notification_channels')) || (await this.appSettings.getValue('notification_channel')) || 'none'; if (raw === 'none') return []; const builtins = new Set((await listChannels()).filter(c => c.source === 'builtin').map(c => c.id)); return raw.split(',').map(c => c.trim()).filter(c => builtins.has(c)); @@ -89,11 +101,8 @@ export class NotificationPreferencesService { * Default (no row) = enabled. Only returns false if there's an explicit disabled row. */ async isEnabledForEvent(userId: number, eventType: NotifEventType, channel: NotifChannel): Promise { - const row = this.db.get<{ enabled: number }>( - 'SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?', - userId, eventType, channel, - ); - return row === undefined || row.enabled === 1; + const row = await this.channelPrefs.findEnabled(userId, eventType, channel); + return row === null || row.enabled === 1; } // ── Preferences matrix ───────────────────────────────────────────────────── @@ -121,8 +130,8 @@ export class NotificationPreferencesService { // Admin-scoped events go out over the admin's own global credentials, which // are independent of the per-user `notification_channels` toggle. const hasSmtp = await this.mailer.isSmtpConfigured(); - const hasAdminWebhook = !!(await this.getAppSetting('admin_webhook_url')); - const hasAdminNtfy = !!(await this.getAppSetting('admin_ntfy_topic')); + const hasAdminWebhook = !!(await this.appSettings.getValue('admin_webhook_url')); + const hasAdminNtfy = !!(await this.appSettings.getValue('admin_ntfy_topic')); const adminActive: Record = { email: hasSmtp, webhook: hasAdminWebhook, ntfy: hasAdminNtfy }; for (const channel of await listChannels()) { // Plugin channels are user-scoped only — they never carry admin-global events. @@ -162,9 +171,7 @@ export class NotificationPreferencesService { * scope='admin' — returns only admin-scoped events (for admin notifications tab) */ async getPreferencesMatrix(userId: number, userRole: string, scope: 'user' | 'admin' = 'user'): Promise { - const rows = this.db.all<{ event_type: string; channel: string; enabled: number }>( - 'SELECT event_type, channel, enabled FROM notification_channel_preferences WHERE user_id = ?', userId, - ); + const rows = await this.channelPrefs.listForUser(userId); // Build a lookup from stored rows const stored: Partial>>> = {}; @@ -200,7 +207,7 @@ export class NotificationPreferencesService { channels: await this.describeChannels(userId, scope), event_types, implemented_combos, - ...(scope === 'user' && { defaults: { ntfyServer: (await this.getAppSetting('admin_ntfy_server')) || null } }), + ...(scope === 'user' && { defaults: { ntfyServer: (await this.appSettings.getValue('admin_ntfy_server')) || null } }), }; } @@ -212,15 +219,12 @@ export class NotificationPreferencesService { * Defaults to true (enabled) when no row exists. */ async getAdminGlobalPref(event: NotifEventType, channel: AdminGlobalChannel): Promise { - const val = await this.getAppSetting(`admin_notif_pref_${event}_${channel}`); + const val = await this.appSettings.getValue(`admin_notif_pref_${event}_${channel}`); return val !== '0'; } private async setAdminGlobalPref(event: NotifEventType, channel: AdminGlobalChannel, enabled: boolean): Promise { - this.db.run('INSERT OR REPLACE INTO app_settings (key, value) VALUES (?, ?)', - `admin_notif_pref_${event}_${channel}`, - enabled ? '1' : '0', - ); + await this.appSettings.setValue(`admin_notif_pref_${event}_${channel}`, enabled ? '1' : '0'); } // ── Preferences update ───────────────────────────────────────────────────── @@ -230,20 +234,14 @@ export class NotificationPreferencesService { userId: number, prefs: Partial>>>, ): Promise { - const upsert = this.db.prepare( - 'INSERT OR REPLACE INTO notification_channel_preferences (user_id, event_type, channel, enabled) VALUES (?, ?, ?, ?)' - ); - const del = this.db.prepare( - 'DELETE FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?' - ); for (const [eventType, channels] of Object.entries(prefs)) { if (!channels) continue; for (const [channel, enabled] of Object.entries(channels)) { if (enabled) { // Remove explicit row — default is enabled - del.run(userId, eventType, channel); + await this.channelPrefs.deletePreference(userId, eventType, channel); } else { - upsert.run(userId, eventType, channel, 0); + await this.channelPrefs.upsertPreference(userId, eventType, channel, 0); } } } @@ -288,7 +286,11 @@ export class NotificationPreferencesService { } } - // Apply global prefs outside the transaction (they write to app_settings) + // Apply global prefs outside the transaction (they write to app_settings). + // Mixed atomicity, preserved as-is (plan3f-sql-inventory.md §4b's flag, R10): + // these are single-key upserts with no cross-row invariant, so a partial + // failure here is low-risk — widening this into the transaction below is a + // ruling this task does not make on its own. for (const [eventType, channels] of Object.entries(globalPrefs)) { if (!channels) continue; for (const [channel, enabled] of Object.entries(channels)) { diff --git a/server/src/nest/notifications/notifications.module.ts b/server/src/nest/notifications/notifications.module.ts index a5aa951f6..ae3682486 100644 --- a/server/src/nest/notifications/notifications.module.ts +++ b/server/src/nest/notifications/notifications.module.ts @@ -1,4 +1,5 @@ import { Module } from '@nestjs/common'; +import { MikroOrmModule } from '@mikro-orm/nestjs'; import { AdminNotificationPreferencesController, NotificationsController } from './notifications.controller'; import { NotificationsMcp } from './notifications.mcp'; import { NotificationsService } from './notifications.service'; @@ -11,6 +12,9 @@ import { MailerModule } from './mailer/mailer.module'; import { AuthModule } from '../auth/auth.module'; import { SchedulingModule } from '../scheduling/scheduling.module'; import { StorageModule } from '../storage/storage.module'; +import { Notifications } from '../../db/entities/Notifications.entity'; +import { NotificationChannelPreferences } from '../../db/entities/NotificationChannelPreferences.entity'; +import { AppSettings } from '../../db/entities/AppSettings.entity'; /** Notifications domain (L6 leaf module). Registered in AppModule. * AuthModule feeds NotificationsMcp's demo gate; MailerModule carries SMTP, @@ -21,9 +25,17 @@ import { StorageModule } from '../storage/storage.module'; * imports only AppConfig+Audit+Scheduling). * NotificationsService and NotificationPreferencesService are exported for * in-container consumers (AdminController's dev test send and preferences tab, - * the plugin RPC surface, HostSurfaceRpc). */ + * the plugin RPC surface, HostSurfaceRpc). + * MikroOrmModule.forFeature([Notifications, NotificationChannelPreferences, + * AppSettings]) (Plan 3f Task 3) registers NotificationsRepository/ + * NotificationChannelPreferencesRepository/AppSettingsRepository for + * NotificationsService/NotificationPreferencesService's @InjectRepository + * constructor params — the TodoModule/PermissionsModule precedent. Only + * this module needs the registration: NotificationsService/ + * NotificationPreferencesService are providers here alone, every other + * module injects the exported singleton rather than declaring its own. */ @Module({ - imports: [AuthModule, MailerModule, SchedulingModule, StorageModule], + imports: [MikroOrmModule.forFeature([Notifications, NotificationChannelPreferences, AppSettings]), AuthModule, MailerModule, SchedulingModule, StorageModule], controllers: [NotificationsController, AdminNotificationPreferencesController], providers: [ NotificationsService, diff --git a/server/src/nest/notifications/notifications.service.ts b/server/src/nest/notifications/notifications.service.ts index c1d8678bc..140553610 100644 --- a/server/src/nest/notifications/notifications.service.ts +++ b/server/src/nest/notifications/notifications.service.ts @@ -1,4 +1,5 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@mikro-orm/nestjs'; import type { ChannelTestResult } from '@trek/shared'; import { readEnv, getAppUrl } from '../../app-config'; import { logDebug, logError } from '../audit/audit-log.logger'; @@ -18,7 +19,8 @@ import { import { getChannel, listChannels } from './channel-registry'; import { getAction } from './in-app-actions'; import { avatarUrl } from '../common/avatarUrl'; -import { DatabaseService } from '../database/database.service'; +import { Notifications } from '../../db/entities/Notifications.entity'; +import type { NotificationsRepository } from '../../db/repositories/Notifications.repository'; import { UnitOfWork } from '../database/unit-of-work'; import { RealtimeService } from '../realtime/realtime.service'; @@ -76,9 +78,9 @@ interface NotificationRow { sender_avatar?: string | null; recipient_id: number; title_key: string; - title_params: string; + title_params: string | null; text_key: string; - text_params: string; + text_params: string | null; positive_text_key: string | null; negative_text_key: string | null; positive_callback: string | null; @@ -86,7 +88,7 @@ interface NotificationRow { response: NotificationResponse | null; navigate_text_key: string | null; navigate_target: string | null; - is_read: number; + is_read: number | null; created_at: string; } @@ -311,13 +313,13 @@ async function shouldSendToUser( @Injectable() export class NotificationsService { constructor( - private readonly db: DatabaseService, private readonly realtime: RealtimeService, private readonly mailer: MailerService, private readonly webhook: WebhookService, private readonly ntfy: NtfyService, private readonly prefs: NotificationPreferencesService, private readonly uow: UnitOfWork, + @InjectRepository(Notifications) private readonly notificationsRepo: NotificationsRepository, ) { // The registry is a module singleton shared with the plugin runtime and any // separately-constructed instance (which never runs onModuleInit), so @@ -384,19 +386,18 @@ export class NotificationsService { // so they must never be resolved as notification recipients on any scope. This is the // single chokepoint for in-app/email/webhook/ntfy, so filtering here covers all channels. if (scope === 'trip') { - const owner = this.db.get<{ user_id: number }>('SELECT user_id FROM trips WHERE id = ?', target); - const members = this.db.all<{ user_id: number }>('SELECT m.user_id FROM trip_members m JOIN users u ON u.id = m.user_id WHERE m.trip_id = ? AND COALESCE(u.is_guest, 0) = 0', target); + const ownerId = await this.notificationsRepo.getTripOwnerId(target); + const memberIds = await this.notificationsRepo.listNonGuestTripMemberIds(target); const ids = new Set(); - if (owner) ids.add(owner.user_id); - for (const m of members) ids.add(m.user_id); + if (ownerId != null) ids.add(ownerId); + for (const id of memberIds) ids.add(id); userIds = Array.from(ids); } else if (scope === 'user') { // A guest can be a todo assignee (scope='user'); never notify them. - const u = this.db.get<{ is_guest?: number }>('SELECT is_guest FROM users WHERE id = ?', target); + const u = await this.notificationsRepo.findGuestFlag(target); userIds = u && u.is_guest ? [] : [target]; } else if (scope === 'admin') { - const admins = this.db.all<{ id: number }>("SELECT id FROM users WHERE role = ? AND COALESCE(is_guest, 0) = 0", 'admin'); - userIds = admins.map(a => a.id); + userIds = await this.notificationsRepo.listNonGuestUserIdsByRole('admin'); } // Only exclude sender for group scopes (trip/admin) — for user scope, the target is explicit @@ -423,15 +424,6 @@ export class NotificationsService { const insertedPairs: Array<{ id: number; recipientId: number }> = []; await this.uow.transactional(async () => { - const stmt = this.db.prepare(` - INSERT INTO notifications ( - type, scope, target, sender_id, recipient_id, - title_key, title_params, text_key, text_params, - positive_text_key, negative_text_key, positive_callback, negative_callback, - navigate_text_key, navigate_target - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `); - for (const recipientId of recipients) { // Check per-user in-app preference if an event_type is provided if (input.event_type && !(await this.prefs.isEnabledForEvent(recipientId, input.event_type, 'inapp'))) { @@ -455,34 +447,32 @@ export class NotificationsService { navigateTarget = input.navigate_target; } - const result = stmt.run( - input.type, input.scope, input.target, input.sender_id, recipientId, - input.title_key, titleParams, input.text_key, textParams, - positiveTextKey, negativeTextKey, positiveCallback, negativeCallback, - navigateTextKey, navigateTarget - ); + const id = await this.notificationsRepo.insertNotification({ + type: input.type, scope: input.scope, target: input.target, sender_id: input.sender_id, recipient_id: recipientId, + title_key: input.title_key, title_params: titleParams, text_key: input.text_key, text_params: textParams, + positive_text_key: positiveTextKey, negative_text_key: negativeTextKey, positive_callback: positiveCallback, negative_callback: negativeCallback, + navigate_text_key: navigateTextKey, navigate_target: navigateTarget, + }); - insertedPairs.push({ id: result.lastInsertRowid as number, recipientId }); + insertedPairs.push({ id, recipientId }); } }); // Fetch sender info once for WS payloads - const sender = input.sender_id - ? this.db.get<{ username: string; avatar: string | null }>('SELECT username, avatar FROM users WHERE id = ?', input.sender_id) - : null; + const sender = input.sender_id ? await this.notificationsRepo.findUserBasic(input.sender_id) : undefined; // Broadcast to each recipient for (const { id: notificationId, recipientId } of insertedPairs) { - const row = this.db.get('SELECT * FROM notifications WHERE id = ?', notificationId) as NotificationRow; + const row = await this.notificationsRepo.findById(notificationId); if (!row) continue; this.realtime.broadcastToUser(recipientId, { type: 'notification:new', notification: { ...row, - created_at: toUtcIso(row.created_at), + created_at: toUtcIso(row.created_at!), sender_username: sender?.username ?? null, - sender_avatar: avatarUrl({ avatar: sender?.avatar }), + sender_avatar: avatarUrl({ avatar: sender?.avatar ?? null }), }, }); } @@ -519,31 +509,23 @@ export class NotificationsService { navigateTarget = input.navigate_target; } - const result = this.db.run(` - INSERT INTO notifications ( - type, scope, target, sender_id, recipient_id, - title_key, title_params, text_key, text_params, - positive_text_key, negative_text_key, positive_callback, negative_callback, - navigate_text_key, navigate_target - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `, - input.type, input.scope, input.target, input.sender_id, recipientId, - input.title_key, titleParams, input.text_key, textParams, - positiveTextKey, negativeTextKey, positiveCallback, negativeCallback, - navigateTextKey, navigateTarget - ); + const notificationId = await this.notificationsRepo.insertNotification({ + type: input.type, scope: input.scope, target: input.target, sender_id: input.sender_id, recipient_id: recipientId, + title_key: input.title_key, title_params: titleParams, text_key: input.text_key, text_params: textParams, + positive_text_key: positiveTextKey, negative_text_key: negativeTextKey, positive_callback: positiveCallback, negative_callback: negativeCallback, + navigate_text_key: navigateTextKey, navigate_target: navigateTarget, + }); - const notificationId = result.lastInsertRowid as number; - const row = this.db.get('SELECT * FROM notifications WHERE id = ?', notificationId); + const row = await this.notificationsRepo.findById(notificationId); if (!row) return null; this.realtime.broadcastToUser(recipientId, { type: 'notification:new', notification: { ...row, - created_at: toUtcIso(row.created_at), + created_at: toUtcIso(row.created_at!), sender_username: sender?.username ?? null, - sender_avatar: avatarUrl({ avatar: sender?.avatar }), + sender_avatar: avatarUrl({ avatar: sender?.avatar ?? null }), }, }); @@ -560,58 +542,46 @@ export class NotificationsService { const offset = options.offset ?? 0; const unreadOnly = options.unreadOnly ?? false; - const whereAliased = unreadOnly ? 'WHERE n.recipient_id = ? AND n.is_read = 0' : 'WHERE n.recipient_id = ?'; - const wherePlain = unreadOnly ? 'WHERE recipient_id = ? AND is_read = 0' : 'WHERE recipient_id = ?'; - - const rows = this.db.all(` - SELECT n.*, u.username AS sender_username, u.avatar AS sender_avatar - FROM notifications n - LEFT JOIN users u ON n.sender_id = u.id - ${whereAliased} - ORDER BY n.created_at DESC - LIMIT ? OFFSET ? - `, userId, limit, offset); - - const { total } = this.db.get<{ total: number }>(`SELECT COUNT(*) as total FROM notifications ${wherePlain}`, userId) as { total: number }; - const { unread_count } = this.db.get<{ unread_count: number }>('SELECT COUNT(*) as unread_count FROM notifications WHERE recipient_id = ? AND is_read = 0', userId) as { unread_count: number }; + const rows = await this.notificationsRepo.listForRecipient(userId, limit, offset, unreadOnly); + const total = await this.notificationsRepo.countForRecipient(userId, unreadOnly); + const unread_count = await this.notificationsRepo.countUnreadForRecipient(userId); + // The repository row's `type`/`scope`/`response` are the physical TEXT + // columns (plain `string`); this service's own `NotificationRow` narrows + // them to the literal unions it writes — the same unchecked boundary the + // legacy `db.all(...)` generic drew (SQLite has no enum + // type to check against either way). const mapped = rows.map(r => ({ ...r, - created_at: toUtcIso(r.created_at), + created_at: toUtcIso(r.created_at!), sender_avatar: avatarUrl({ avatar: r.sender_avatar }), - })); + })) as NotificationRow[]; return { notifications: mapped, total, unread_count }; } async unreadCount(userId: number): Promise { - const row = this.db.get<{ count: number }>('SELECT COUNT(*) as count FROM notifications WHERE recipient_id = ? AND is_read = 0', userId) as { count: number }; - return row.count; + return await this.notificationsRepo.countUnreadForRecipient(userId); } async markRead(notificationId: number, userId: number): Promise { - const result = this.db.run('UPDATE notifications SET is_read = 1 WHERE id = ? AND recipient_id = ?', notificationId, userId); - return result.changes > 0; + return (await this.notificationsRepo.setRead(notificationId, userId, 1)) > 0; } async markUnread(notificationId: number, userId: number): Promise { - const result = this.db.run('UPDATE notifications SET is_read = 0 WHERE id = ? AND recipient_id = ?', notificationId, userId); - return result.changes > 0; + return (await this.notificationsRepo.setRead(notificationId, userId, 0)) > 0; } async markAllRead(userId: number): Promise { - const result = this.db.run('UPDATE notifications SET is_read = 1 WHERE recipient_id = ? AND is_read = 0', userId); - return result.changes; + return await this.notificationsRepo.markAllRead(userId); } async deleteOne(notificationId: number, userId: number): Promise { - const result = this.db.run('DELETE FROM notifications WHERE id = ? AND recipient_id = ?', notificationId, userId); - return result.changes > 0; + return (await this.notificationsRepo.deleteForRecipient(notificationId, userId)) > 0; } async deleteAll(userId: number): Promise { - const result = this.db.run('DELETE FROM notifications WHERE recipient_id = ?', userId); - return result.changes; + return await this.notificationsRepo.deleteAllForRecipient(userId); } async respond( @@ -619,7 +589,7 @@ export class NotificationsService { userId: number, response: NotificationResponse ): Promise { - const notification = this.db.get('SELECT * FROM notifications WHERE id = ? AND recipient_id = ?', notificationId, userId); + const notification = await this.notificationsRepo.findByIdForRecipient(notificationId, userId); if (!notification) return { success: false, error: 'Notification not found' }; if (notification.type !== 'boolean') return { success: false, error: 'Not a boolean notification' }; @@ -641,37 +611,27 @@ export class NotificationsService { // Atomic claim BEFORE the handler runs — only updates if response is still // NULL, so a concurrent double-submit can never execute the action twice // (the legacy order ran the handler first, letting both submits through). - const result = this.db.run( - 'UPDATE notifications SET response = ?, is_read = 1 WHERE id = ? AND recipient_id = ? AND response IS NULL', - response, notificationId, userId - ); + const changes = await this.notificationsRepo.claimResponse(notificationId, userId, response); - if (result.changes === 0) return { success: false, error: 'Already responded' }; + if (changes === 0) return { success: false, error: 'Already responded' }; try { await handler(callback.payload, userId); } catch (err) { // Release the claim so the user can retry — legacy contract: a handler // failure returns its message and leaves the notification unresponded. - this.db.run( - 'UPDATE notifications SET response = NULL, is_read = ? WHERE id = ? AND recipient_id = ?', - notification.is_read, notificationId, userId - ); + await this.notificationsRepo.releaseResponse(notificationId, userId, notification.is_read); return { success: false, error: err instanceof Error ? err.message : 'Action failed' }; } - const updated = this.db.get(` - SELECT n.*, u.username AS sender_username, u.avatar AS sender_avatar - FROM notifications n - LEFT JOIN users u ON n.sender_id = u.id - WHERE n.id = ? - `, notificationId) as NotificationRow; + const updated = (await this.notificationsRepo.findWithSenderById(notificationId))!; + // Same unchecked-narrowing boundary as `listInApp`'s `mapped` above. const mappedUpdated = { ...updated, - created_at: toUtcIso(updated.created_at), + created_at: toUtcIso(updated.created_at!), sender_avatar: avatarUrl({ avatar: updated.sender_avatar }), - }; + } as NotificationRow; this.realtime.broadcastToUser(userId, { type: 'notification:updated', notification: mappedUpdated }); @@ -691,7 +651,7 @@ export class NotificationsService { if (!configEntry) { logDebug(`notificationService.send: unknown event type "${event}", using fallback`); if (readEnv().app.isDevelopment && actorId != null) { - const devSender = this.db.get<{ username: string; avatar: string | null }>('SELECT username, avatar FROM users WHERE id = ?', actorId) ?? null; + const devSender = (await this.notificationsRepo.findUserBasic(actorId)) ?? null; await this.createNotificationForRecipient({ type: 'simple', scope: 'user', @@ -718,7 +678,7 @@ export class NotificationsService { // Fetch sender info once for in-app WS payloads const sender = actorId - ? this.db.get<{ username: string; avatar: string | null }>('SELECT username, avatar FROM users WHERE id = ?', actorId) ?? null + ? (await this.notificationsRepo.findUserBasic(actorId)) ?? null : null; logDebug(`notificationService.send event=${event} scope=${scope} targetId=${targetId} recipients=${recipients.length} channels=inapp,${activeChannels.join(',')}`); diff --git a/server/tests/helpers/notifications-repos.ts b/server/tests/helpers/notifications-repos.ts new file mode 100644 index 000000000..a93e00aeb --- /dev/null +++ b/server/tests/helpers/notifications-repos.ts @@ -0,0 +1,26 @@ +import type Database from 'better-sqlite3'; +import { sharedTestOrm } from './test-uow'; +import { Notifications } from '../../src/db/entities/Notifications.entity'; +import type { NotificationsRepository } from '../../src/db/repositories/Notifications.repository'; +import { NotificationChannelPreferences } from '../../src/db/entities/NotificationChannelPreferences.entity'; +import type { NotificationChannelPreferencesRepository } from '../../src/db/repositories/NotificationChannelPreferences.repository'; + +/** + * Plan 3f Task 3 (`NotificationsService`/`NotificationPreferencesService`) + * test-only repository factories, bound to a suite's own better-sqlite3 + * handle via the SAME memoised `sharedTestOrm` `test-uow.ts` exports — the + * `todo-repos.ts`/`packing-repos.ts` precedent (a repository from a SECOND, + * independent ORM instance would resolve a different transaction fork than + * the `UnitOfWork`/`AppSettingsRepository` a hand-built service also needs). + * + * A NEW file rather than an addition to `test-uow.ts`, same reasoning those + * two files give for themselves: avoids colliding with a concurrently + * in-flight task's edits to that shared file. + */ +export function createTestNotificationsRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(Notifications)); +} + +export function createTestNotificationChannelPreferencesRepo(db: Database.Database): Promise { + return sharedTestOrm(db).then((t) => t.repo(NotificationChannelPreferences)); +} diff --git a/server/tests/helpers/notifications.ts b/server/tests/helpers/notifications.ts index 82ed7382f..353674333 100644 --- a/server/tests/helpers/notifications.ts +++ b/server/tests/helpers/notifications.ts @@ -5,7 +5,8 @@ import { NotificationPreferencesService } from '../../src/nest/notifications/not import { NotificationsService } from '../../src/nest/notifications/notifications.service'; import { NtfyService } from '../../src/nest/notifications/transports/ntfy.service'; import { WebhookService } from '../../src/nest/notifications/transports/webhook.service'; -import { createTestUnitOfWork } from './test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo } from './test-uow'; +import { createTestNotificationsRepo, createTestNotificationChannelPreferencesRepo } from './notifications-repos'; /** * A NotificationsService wired the way Nest wires it. @@ -13,24 +14,38 @@ import { createTestUnitOfWork } from './test-uow'; * The domain takes six providers since the fold, and eight places used to build * it by hand — every added constructor parameter was an eight-file diff. One * helper keeps that at one. + * + * Plan 3f Task 3: `NotificationsService`/`NotificationPreferencesService` no + * longer take a `DatabaseService` — both now take repositories, resolved + * through `notifications-repos.ts`/`test-uow.ts`'s `sharedTestOrm`-memoised + * factories, bound to `dbs.connection` the same way every other converted + * domain's test helper does. The helper's own OUTER signature (`dbs`, + * `realtime`) is unchanged, so every caller that only goes through this + * function (`mcp-test-controllers.ts`, `plugin-host.ts`, both test-only) needs + * no edit of its own. */ export async function makeNotificationsService(dbs: DatabaseService, realtime = new RealtimeService()): Promise { const mailer = new MailerService(dbs); const uow = await createTestUnitOfWork(dbs.connection); + const appSettings = await createTestAppSettingsRepo(dbs.connection); + const channelPrefsRepo = await createTestNotificationChannelPreferencesRepo(dbs.connection); + const notificationsRepo = await createTestNotificationsRepo(dbs.connection); return new NotificationsService( - dbs, realtime, mailer, new WebhookService(dbs), new NtfyService(dbs), - new NotificationPreferencesService(dbs, mailer, uow), + new NotificationPreferencesService(mailer, uow, appSettings, channelPrefsRepo), uow, + notificationsRepo, ); } /** The preferences half on its own, over the same connection. */ export async function makeNotificationPreferencesService(dbs: DatabaseService): Promise { - return new NotificationPreferencesService(dbs, new MailerService(dbs), await createTestUnitOfWork(dbs.connection)); + const appSettings = await createTestAppSettingsRepo(dbs.connection); + const channelPrefsRepo = await createTestNotificationChannelPreferencesRepo(dbs.connection); + return new NotificationPreferencesService(new MailerService(dbs), await createTestUnitOfWork(dbs.connection), appSettings, channelPrefsRepo); } /** diff --git a/server/tests/unit/db/repositories/NotificationChannelPreferences.repository.test.ts b/server/tests/unit/db/repositories/NotificationChannelPreferences.repository.test.ts new file mode 100644 index 000000000..7e62ba12c --- /dev/null +++ b/server/tests/unit/db/repositories/NotificationChannelPreferences.repository.test.ts @@ -0,0 +1,123 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser, type TestUser } from '../../../helpers/factories'; +import { NotificationChannelPreferences } from '../../../../src/db/entities/NotificationChannelPreferences.entity'; +import type { NotificationChannelPreferencesRepository } from '../../../../src/db/repositories/NotificationChannelPreferences.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let repo: NotificationChannelPreferencesRepository; +let user: TestUser; +let otherUser: TestUser; + +beforeAll(async () => { + t = await createTestOrm(testDb); + repo = t.repo(NotificationChannelPreferences); +}); +beforeEach(() => { + resetTestDb(testDb); + t.clear(); + user = createUser(testDb).user; + otherUser = createUser(testDb).user; +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +function insertRaw(userId: number, eventType: string, channel: string, enabled: number): void { + testDb.prepare('INSERT INTO notification_channel_preferences (user_id, event_type, channel, enabled) VALUES (?, ?, ?, ?)').run(userId, eventType, channel, enabled); +} + +describe('notification_channel_preferences — the (user_id, event_type, channel) composite PK the ON CONFLICT target relies on (R5/§14)', () => { + it("NCPREPO-SCHEMA-001: PRAGMA table_info('notification_channel_preferences') reports a 3-column PRIMARY KEY over exactly (user_id, event_type, channel), matching the migration DDL", () => { + const cols = testDb.prepare("PRAGMA table_info('notification_channel_preferences')").all() as { name: string; pk: number }[]; + const pkCols = cols.filter((c) => c.pk > 0).sort((a, b) => a.pk - b.pk).map((c) => c.name); + expect(pkCols).toEqual(['user_id', 'event_type', 'channel']); + }); + + it('NCPREPO-SCHEMA-002: the entity metadata declares the same 3-property composite PK, naming the RELATION property, not its persist(false) twin', () => { + const meta = t.orm.getMetadata(NotificationChannelPreferences); + expect(meta.getPrimaryProps().map((p) => p.name).sort()).toEqual(['channel', 'event_type', 'user'].sort()); + }); +}); + +describe('NotificationChannelPreferencesRepository — reads (NP2/NP3 parity)', () => { + it('NCPREPO-001 — findEnabled matches `SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?`, and is null for an absent row (default-enabled)', async () => { + insertRaw(user.id, 'trip_invite', 'email', 0); + const legacy = testDb.prepare('SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'email') as { enabled: number }; + const converted = await repo.findEnabled(user.id, 'trip_invite', 'email'); + // A plain-object equality against the MikroORM-returned entity would also + // compare its (identical, but differently-shaped) relation/metadata + // fields — the field values are the parity contract here, not the + // wrapper shape (the `AppSettingsRepository.getValue` precedent). + expect(converted?.enabled).toBe(legacy.enabled); + expect(await repo.findEnabled(user.id, 'trip_invite', 'webhook')).toBeNull(); + }); + + it('NCPREPO-002 — listForUser matches `SELECT event_type, channel, enabled FROM notification_channel_preferences WHERE user_id = ?`, fully seeded across every event×channel combination, and never leaks another user', async () => { + const combos: Array<[string, string, number]> = [ + ['trip_invite', 'email', 0], ['trip_invite', 'webhook', 1], ['trip_invite', 'inapp', 0], + ['booking_change', 'email', 1], ['booking_change', 'ntfy', 0], + ['vacay_invite', 'inapp', 0], + ]; + for (const [event_type, channel, enabled] of combos) insertRaw(user.id, event_type, channel, enabled); + insertRaw(otherUser.id, 'trip_invite', 'email', 0); // a different user's row — must not leak in + + const legacy = (testDb.prepare('SELECT event_type, channel, enabled FROM notification_channel_preferences WHERE user_id = ?').all(user.id) as { event_type: string; channel: string; enabled: number }[]) + .map((r) => ({ event_type: r.event_type, channel: r.channel, enabled: r.enabled })) + .sort((a, b) => a.event_type.localeCompare(b.event_type) || a.channel.localeCompare(b.channel)); + const converted = (await repo.listForUser(user.id)) + .map((r) => ({ event_type: r.event_type, channel: r.channel, enabled: r.enabled })) + .sort((a, b) => a.event_type.localeCompare(b.event_type) || a.channel.localeCompare(b.channel)); + + expect(converted.length).toBe(combos.length); + expect(converted).toEqual(legacy); + }); +}); + +describe('NotificationChannelPreferencesRepository — writes (NP5/NP6, R5)', () => { + it( + "NCPREPO-UPSERT-SQL: em.upsert renders `insert into \"notification_channel_preferences\" ... on conflict (`user_id`, `event_type`, `channel`) do update set ...` — the composite-PK conflict target Task 0's R5 worked example pinned, checked directly against the rendered SQL, not assumed from onConflictFields alone", + async () => { + const connection = t.orm.em.getConnection(); + const spy = vi.spyOn(connection, 'execute'); + try { + // Insert branch: no existing row. + await repo.upsertPreference(user.id, 'trip_invite', 'email', 0); + expect(testDb.prepare('SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'email')).toEqual({ enabled: 0 }); + + const insertSql = spy.mock.calls.map(([sql]) => sql).find((sql): sql is string => typeof sql === 'string' && /insert into/i.test(sql)); + expect(insertSql).toContain('on conflict (`user_id`, `event_type`, `channel`)'); + spy.mockClear(); + + // Merge branch: an existing row, same call shape — the SAME conflict + // target both creates and merges, matching `OR REPLACE`'s semantics + // (there is no other non-key column REPLACE would reset that merge + // does not also set). + await repo.upsertPreference(user.id, 'trip_invite', 'email', 1); + expect(testDb.prepare('SELECT enabled FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'email')).toEqual({ enabled: 1 }); + expect((testDb.prepare('SELECT COUNT(*) as c FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'email') as { c: number }).c).toBe(1); + + const mergeSql = spy.mock.calls.map(([sql]) => sql).find((sql): sql is string => typeof sql === 'string' && /insert into/i.test(sql)); + expect(mergeSql).toContain('on conflict (`user_id`, `event_type`, `channel`)'); + } finally { + spy.mockRestore(); + } + }, + ); + + it('NCPREPO-003 — deletePreference matches `DELETE FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?`, scoped to exactly that triple', async () => { + insertRaw(user.id, 'trip_invite', 'email', 0); + insertRaw(user.id, 'trip_invite', 'webhook', 0); // a different channel — must survive + insertRaw(otherUser.id, 'trip_invite', 'email', 0); // a different user — must survive + + expect(await repo.deletePreference(user.id, 'trip_invite', 'email')).toBe(1); + expect(testDb.prepare('SELECT * FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'email')).toBeUndefined(); + expect(testDb.prepare('SELECT * FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(user.id, 'trip_invite', 'webhook')).toBeDefined(); + expect(testDb.prepare('SELECT * FROM notification_channel_preferences WHERE user_id = ? AND event_type = ? AND channel = ?').get(otherUser.id, 'trip_invite', 'email')).toBeDefined(); + expect(await repo.deletePreference(user.id, 'trip_invite', 'email')).toBe(0); // already gone — no-op + }); +}); diff --git a/server/tests/unit/db/repositories/Notifications.repository.test.ts b/server/tests/unit/db/repositories/Notifications.repository.test.ts new file mode 100644 index 000000000..1ec4520f4 --- /dev/null +++ b/server/tests/unit/db/repositories/Notifications.repository.test.ts @@ -0,0 +1,258 @@ +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { createSnapshotTestDb } from '../../../helpers/db-mock'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; +import { createUser, createTrip, addTripMember, type TestUser, type TestTrip } from '../../../helpers/factories'; +import { Notifications } from '../../../../src/db/entities/Notifications.entity'; +import type { NotificationsRepository } from '../../../../src/db/repositories/Notifications.repository'; + +const testDb = createSnapshotTestDb(); +let t: TestOrm; +let repo: NotificationsRepository; +let owner: TestUser; +let trip: TestTrip; + +beforeAll(async () => { + t = await createTestOrm(testDb); + repo = t.repo(Notifications); +}); +beforeEach(() => { + resetTestDb(testDb); + t.clear(); + owner = createUser(testDb).user; + trip = createTrip(testDb, owner.id); +}); +afterAll(async () => { + await t.close(); + testDb.close(); +}); + +/** Inserts one of each NT-inventory notification type, with every optional field set, for `recipientId`. Returns the three ids in insertion order (simple, boolean, navigate). */ +function seedThreeTypes(recipientId: number, senderId: number | null): { simpleId: number; booleanId: number; navigateId: number } { + const simpleId = testDb.prepare(` + INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, title_params, text_key, text_params) + VALUES ('simple', 'trip', ?, ?, ?, 'notif.trip_invite.title', '{"trip":"Rome"}', 'notif.trip_invite.text', '{"actor":"Alice"}') + `).run(trip.id, senderId, recipientId).lastInsertRowid as number; + + const booleanId = testDb.prepare(` + INSERT INTO notifications ( + type, scope, target, sender_id, recipient_id, title_key, title_params, text_key, text_params, + positive_text_key, negative_text_key, positive_callback, negative_callback + ) VALUES ('boolean', 'trip', ?, ?, ?, 'notif.vacay_invite.title', '{"trip":"Rome"}', 'notif.vacay_invite.text', '{"actor":"Bob"}', + 'notif.action.accept', 'notif.action.decline', '{"action":"noop","payload":{"x":1}}', '{"action":"noop","payload":{"x":2}}') + `).run(trip.id, senderId, recipientId).lastInsertRowid as number; + + const navigateId = testDb.prepare(` + INSERT INTO notifications ( + type, scope, target, sender_id, recipient_id, title_key, title_params, text_key, text_params, + navigate_text_key, navigate_target + ) VALUES ('navigate', 'trip', ?, ?, ?, 'notif.booking_change.title', '{"trip":"Rome"}', 'notif.booking_change.text', '{"actor":"Carl"}', + 'notif.action.view_trip', '/trips/1') + `).run(trip.id, senderId, recipientId).lastInsertRowid as number; + + return { simpleId, booleanId, navigateId }; +} + +/** The legacy `SELECT * FROM notifications WHERE id = ?`, run raw, for a full-key parity comparison. */ +function legacyFindById(id: number): unknown { + return testDb.prepare('SELECT * FROM notifications WHERE id = ?').get(id); +} + +describe('NotificationsRepository — findById/findByIdForRecipient parity (fully seeded: simple/boolean/navigate, every optional field set)', () => { + it('NOTREPO-001 — findById matches the legacy full-row SELECT, byte for byte, for all three notification types', async () => { + const { user: recipient } = createUser(testDb); + const { user: sender } = createUser(testDb); + const { simpleId, booleanId, navigateId } = seedThreeTypes(recipient.id, sender.id); + + for (const id of [simpleId, booleanId, navigateId]) { + const legacy = legacyFindById(id); + const converted = await repo.findById(id); + expect(converted).toEqual(legacy); + } + }); + + it('NOTREPO-002 — findByIdForRecipient matches the legacy recipient-scoped SELECT, and returns undefined for the wrong recipient', async () => { + const { user: recipient } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const { booleanId } = seedThreeTypes(recipient.id, null); + + const legacy = testDb.prepare('SELECT * FROM notifications WHERE id = ? AND recipient_id = ?').get(booleanId, recipient.id); + expect(await repo.findByIdForRecipient(booleanId, recipient.id)).toEqual(legacy); + expect(await repo.findByIdForRecipient(booleanId, stranger.id)).toBeUndefined(); + }); + + it('NOTREPO-003 — findWithSenderById matches the legacy LEFT JOIN users projection (sender_username/sender_avatar), including a NULL sender', async () => { + const { user: recipient } = createUser(testDb); + const { user: sender } = createUser(testDb, { username: 'Alice', email: 'alice@test.example.com' }); + const { simpleId } = seedThreeTypes(recipient.id, sender.id); + + const legacy = testDb.prepare(` + SELECT n.*, u.username AS sender_username, u.avatar AS sender_avatar + FROM notifications n LEFT JOIN users u ON n.sender_id = u.id + WHERE n.id = ? + `).get(simpleId); + expect(await repo.findWithSenderById(simpleId)).toEqual(legacy); + + // A system notification (no sender) — the LEFT JOIN's NULL side. + const systemId = testDb.prepare(` + INSERT INTO notifications (type, scope, target, sender_id, recipient_id, title_key, title_params, text_key, text_params) + VALUES ('simple', 'admin', 0, NULL, ?, 'notif.version_available.title', '{}', 'notif.version_available.text', '{}') + `).run(recipient.id).lastInsertRowid as number; + const legacySystem = testDb.prepare(` + SELECT n.*, u.username AS sender_username, u.avatar AS sender_avatar + FROM notifications n LEFT JOIN users u ON n.sender_id = u.id + WHERE n.id = ? + `).get(systemId); + expect(await repo.findWithSenderById(systemId)).toEqual(legacySystem); + }); + + it('NOTREPO-004 — listForRecipient matches the legacy joined page read, for both unreadOnly variants', async () => { + const { user: recipient } = createUser(testDb); + const { user: sender } = createUser(testDb); + seedThreeTypes(recipient.id, sender.id); + testDb.prepare('UPDATE notifications SET is_read = 1 WHERE recipient_id = ? LIMIT 1').run(recipient.id); + // SQLite compiled without LIMIT-on-UPDATE support in better-sqlite3 by default — fall back to an id-scoped update. + const firstId = (testDb.prepare('SELECT id FROM notifications WHERE recipient_id = ? ORDER BY id LIMIT 1').get(recipient.id) as { id: number }).id; + testDb.prepare('UPDATE notifications SET is_read = 1 WHERE id = ?').run(firstId); + + for (const unreadOnly of [false, true]) { + const whereAliased = unreadOnly ? 'WHERE n.recipient_id = ? AND n.is_read = 0' : 'WHERE n.recipient_id = ?'; + const legacy = testDb.prepare(` + SELECT n.*, u.username AS sender_username, u.avatar AS sender_avatar + FROM notifications n LEFT JOIN users u ON n.sender_id = u.id + ${whereAliased} + ORDER BY n.created_at DESC + LIMIT ? OFFSET ? + `).all(recipient.id, 20, 0); + const converted = await repo.listForRecipient(recipient.id, 20, 0, unreadOnly); + expect(converted).toEqual(legacy); + } + }); +}); + +describe('NotificationsRepository — recipient resolution (NT1-NT4)', () => { + it('NOTREPO-005 — getTripOwnerId matches `SELECT user_id FROM trips WHERE id = ?`', async () => { + const legacy = (testDb.prepare('SELECT user_id FROM trips WHERE id = ?').get(trip.id) as { user_id: number }).user_id; + expect(await repo.getTripOwnerId(trip.id)).toBe(legacy); + expect(await repo.getTripOwnerId(999999)).toBeNull(); + }); + + it('NOTREPO-006 — listNonGuestTripMemberIds (NT2, the guest-exclusion chokepoint, #1362): a guest member never appears, a real member always does', async () => { + const { user: member } = createUser(testDb); + addTripMember(testDb, trip.id, member.id); + const guestId = (testDb.prepare( + "INSERT INTO users (username, email, password_hash, role, is_guest) VALUES ('Guest', 'guest-repo@guests.invalid', '', 'user', 1)", + ).run()).lastInsertRowid as number; + addTripMember(testDb, trip.id, guestId); + + const legacy = (testDb.prepare( + 'SELECT m.user_id FROM trip_members m JOIN users u ON u.id = m.user_id WHERE m.trip_id = ? AND COALESCE(u.is_guest, 0) = 0', + ).all(trip.id) as { user_id: number }[]).map((r) => r.user_id); + const converted = await repo.listNonGuestTripMemberIds(trip.id); + + expect(converted.sort()).toEqual(legacy.sort()); + expect(converted).toContain(member.id); + expect(converted).not.toContain(guestId); + }); + + it('NOTREPO-007 — findGuestFlag matches `SELECT is_guest FROM users WHERE id = ?`, and is undefined for a nonexistent user', async () => { + const { user } = createUser(testDb); + expect(await repo.findGuestFlag(user.id)).toEqual({ is_guest: 0 }); + expect(await repo.findGuestFlag(999999)).toBeUndefined(); + }); + + it('NOTREPO-008 — listNonGuestUserIdsByRole matches `SELECT id FROM users WHERE role = ? AND COALESCE(is_guest, 0) = 0`', async () => { + const { user: admin1 } = createUser(testDb, { role: 'admin' }); + const { user: admin2 } = createUser(testDb, { role: 'admin' }); + createUser(testDb); // a regular user — must not appear + const guestAdminId = (testDb.prepare( + "INSERT INTO users (username, email, password_hash, role, is_guest) VALUES ('GuestAdmin', 'guest-admin@guests.invalid', '', 'admin', 1)", + ).run()).lastInsertRowid as number; + + const legacy = (testDb.prepare( + 'SELECT id FROM users WHERE role = ? AND COALESCE(is_guest, 0) = 0', + ).all('admin') as { id: number }[]).map((r) => r.id); + const converted = await repo.listNonGuestUserIdsByRole('admin'); + + expect(converted.sort()).toEqual(legacy.sort()); + expect(converted).toContain(admin1.id); + expect(converted).toContain(admin2.id); + expect(converted).not.toContain(guestAdminId); + }); + + it('NOTREPO-009 — findUserBasic matches `SELECT username, avatar FROM users WHERE id = ?`', async () => { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET avatar = ? WHERE id = ?').run('avatar.png', user.id); + const legacy = testDb.prepare('SELECT username, avatar FROM users WHERE id = ?').get(user.id); + expect(await repo.findUserBasic(user.id)).toEqual(legacy); + expect(await repo.findUserBasic(999999)).toBeUndefined(); + }); +}); + +describe('NotificationsRepository — writes', () => { + it('NOTREPO-010 — insertNotification writes all 15 columns and returns the new id', async () => { + const { user: recipient } = createUser(testDb); + const { user: sender } = createUser(testDb); + + const id = await repo.insertNotification({ + type: 'boolean', scope: 'trip', target: trip.id, sender_id: sender.id, recipient_id: recipient.id, + title_key: 'notif.trip_invite.title', title_params: '{"trip":"Rome"}', text_key: 'notif.trip_invite.text', text_params: '{"actor":"A"}', + positive_text_key: 'notif.action.accept', negative_text_key: 'notif.action.decline', + positive_callback: '{"action":"noop","payload":{}}', negative_callback: '{"action":"noop","payload":{}}', + navigate_text_key: null, navigate_target: null, + }); + + const row = testDb.prepare('SELECT * FROM notifications WHERE id = ?').get(id) as Record; + expect(row.type).toBe('boolean'); + expect(row.scope).toBe('trip'); + expect(row.target).toBe(trip.id); + expect(row.sender_id).toBe(sender.id); + expect(row.recipient_id).toBe(recipient.id); + expect(row.positive_callback).toBe('{"action":"noop","payload":{}}'); + expect(row.is_read).toBe(0); + expect(row.response).toBeNull(); + }); + + it('NOTREPO-011 — setRead/markAllRead/deleteForRecipient/deleteAllForRecipient are recipient-scoped and return the affected-row count', async () => { + const { user: recipient } = createUser(testDb); + const { user: stranger } = createUser(testDb); + const { simpleId, booleanId } = seedThreeTypes(recipient.id, null); + + expect(await repo.setRead(simpleId, stranger.id, 1)).toBe(0); // wrong recipient — no-op + expect(await repo.setRead(simpleId, recipient.id, 1)).toBe(1); + expect((testDb.prepare('SELECT is_read FROM notifications WHERE id = ?').get(simpleId) as { is_read: number }).is_read).toBe(1); + + expect(await repo.setRead(simpleId, recipient.id, 0)).toBe(1); + + expect(await repo.markAllRead(recipient.id)).toBe(3); // simple + boolean + navigate, all currently unread + expect(await repo.countUnreadForRecipient(recipient.id)).toBe(0); + + expect(await repo.deleteForRecipient(booleanId, stranger.id)).toBe(0); + expect(await repo.deleteForRecipient(booleanId, recipient.id)).toBe(1); + expect(await repo.deleteAllForRecipient(recipient.id)).toBe(2); // simple + navigate remain + expect(await repo.countForRecipient(recipient.id, false)).toBe(0); + }); + + it('NOTREPO-012 — claimResponse (NT20) only succeeds once: the atomic double-submit claim', async () => { + const { user: recipient } = createUser(testDb); + const { booleanId } = seedThreeTypes(recipient.id, null); + + expect(await repo.claimResponse(booleanId, recipient.id, 'positive')).toBe(1); + // A second claim on the same, now-claimed notification affects 0 rows — + // the `response IS NULL` guard is the load-bearing part of this statement. + expect(await repo.claimResponse(booleanId, recipient.id, 'negative')).toBe(0); + expect((testDb.prepare('SELECT response FROM notifications WHERE id = ?').get(booleanId) as { response: string }).response).toBe('positive'); + }); + + it('NOTREPO-013 — releaseResponse (NT21) restores the claim so a retry can succeed', async () => { + const { user: recipient } = createUser(testDb); + const { booleanId } = seedThreeTypes(recipient.id, null); + + await repo.claimResponse(booleanId, recipient.id, 'positive'); + await repo.releaseResponse(booleanId, recipient.id, 0); + const row = testDb.prepare('SELECT response, is_read FROM notifications WHERE id = ?').get(booleanId) as { response: string | null; is_read: number }; + expect(row.response).toBeNull(); + expect(row.is_read).toBe(0); + expect(await repo.claimResponse(booleanId, recipient.id, 'negative')).toBe(1); + }); +}); diff --git a/server/tests/unit/nest/channel-override.test.ts b/server/tests/unit/nest/channel-override.test.ts index 4e9957029..712a86e55 100644 --- a/server/tests/unit/nest/channel-override.test.ts +++ b/server/tests/unit/nest/channel-override.test.ts @@ -68,7 +68,8 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { createPluginRuntime } from '../../helpers/plugin-host'; import { MailerService } from '../../../src/nest/notifications/mailer/mailer.service'; import { NotificationPreferencesService } from '../../../src/nest/notifications/notification-preferences.service'; -import { createTestUnitOfWork } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow'; +import { createTestNotificationChannelPreferencesRepo } from '../../helpers/notifications-repos'; import { setPluginChannelSource, @@ -91,7 +92,12 @@ void WebhookService; void NtfyService; beforeAll(async () => { createTables(testDb); runMigrations(testDb); - prefsSvc = new NotificationPreferencesService(prefsDbs, new MailerService(prefsDbs), await createTestUnitOfWork(testDb)); + prefsSvc = new NotificationPreferencesService( + new MailerService(prefsDbs), + await createTestUnitOfWork(testDb), + await createTestAppSettingsRepo(testDb), + await createTestNotificationChannelPreferencesRepo(testDb), + ); notifications = await makeNotificationsService(new DatabaseService(testDb)); }); beforeEach(() => { resetTestDb(testDb); setPluginChannelSource(null); }); diff --git a/server/tests/unit/nest/notification-preferences.service.test.ts b/server/tests/unit/nest/notification-preferences.service.test.ts index 0105329f3..b0c9895c8 100644 --- a/server/tests/unit/nest/notification-preferences.service.test.ts +++ b/server/tests/unit/nest/notification-preferences.service.test.ts @@ -43,7 +43,8 @@ import { registerBuiltinChannels } from '../../../src/nest/notifications/channel import { NtfyService } from '../../../src/nest/notifications/transports/ntfy.service'; import { WebhookService } from '../../../src/nest/notifications/transports/webhook.service'; import { __resetChannelsForTest } from '../../../src/nest/notifications/channel-registry'; -import { createTestUnitOfWork } from '../../helpers/test-uow'; +import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow'; +import { createTestNotificationChannelPreferencesRepo } from '../../helpers/notifications-repos'; const dbs = new DatabaseService(testDb); const mailer = new MailerService(dbs); @@ -69,7 +70,12 @@ const isWebhookConfigured = (...a: Parameters) => sv beforeAll(async () => { createTables(testDb); runMigrations(testDb); - svc = new NotificationPreferencesService(dbs, mailer, await createTestUnitOfWork(testDb)); + svc = new NotificationPreferencesService( + mailer, + await createTestUnitOfWork(testDb), + await createTestAppSettingsRepo(testDb), + await createTestNotificationChannelPreferencesRepo(testDb), + ); }); beforeEach(() => { diff --git a/server/tests/unit/nest/notifications.service.test.ts b/server/tests/unit/nest/notifications.service.test.ts index 396e42550..05d9d238c 100644 --- a/server/tests/unit/nest/notifications.service.test.ts +++ b/server/tests/unit/nest/notifications.service.test.ts @@ -312,6 +312,24 @@ describe('send() — recipient resolution', () => { expect(recipients).not.toContain(guestId); }); + it('NSVC-007c — guest-exclusion (#1362), the direct call: resolveRecipients itself never returns a guest for trip or user scope (Plan 3f Task 3 — the NT2 chokepoint, R8)', async () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + + const tripId = (testDb.prepare('INSERT INTO trips (title, user_id) VALUES (?, ?)').run('Trip', owner.id)).lastInsertRowid as number; + testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, member.id); + const guestId = (testDb.prepare("INSERT INTO users (username, email, password_hash, role, is_guest) VALUES ('Guest', 'guest-y@guests.invalid', '', 'user', 1)").run()).lastInsertRowid as number; + testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(tripId, guestId); + + const tripRecipients = await notifications.resolveRecipients('trip', tripId); + expect(tripRecipients).toContain(owner.id); + expect(tripRecipients).toContain(member.id); + expect(tripRecipients).not.toContain(guestId); + + const userRecipients = await notifications.resolveRecipients('user', guestId); + expect(userRecipients).toEqual([]); + }); + it('NSVC-008 — user scope sends to exactly one user', async () => { const { user: target } = createUser(testDb); const { user: other } = createUser(testDb);