diff --git a/server/src/index.ts b/server/src/index.ts index c1c3563f8..aa0e5bdba 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -101,8 +101,6 @@ const onListen = () => { // belongs at boot. airtrail: nestApp.get(AirtrailSyncService), }); - scheduler.startTripReminders(); - scheduler.startTodoReminders(); scheduler.startVersionCheck(); scheduler.startDemoReset(); scheduler.startIdempotencyCleanup(); diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 9acdc5b68..5b55f9e5b 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -496,7 +496,7 @@ export class CollabService { /** Fire-and-forget collab notification (mirrors the legacy route's dynamic import). */ notifyCollab(tripId: string, actor: User, preview?: string): void { - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts index c482b85cc..78824f719 100644 --- a/server/src/nest/collections/collections.service.ts +++ b/server/src/nest/collections/collections.service.ts @@ -1014,7 +1014,7 @@ export class CollectionsService { this.realtime.broadcastToUser(targetUserId, { type: 'collections:invite', from: { id: inviterId, username: inviterUsername }, collectionId }); - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/nest/memories/synology.service.ts b/server/src/nest/memories/synology.service.ts index 25ad36a25..d7c61f903 100644 --- a/server/src/nest/memories/synology.service.ts +++ b/server/src/nest/memories/synology.service.ts @@ -389,7 +389,7 @@ export class SynologyService { this._clearSynologySession(userId); // Fire-and-forget like unifiedService's photos_shared send — without the // .catch an in-app insert failure became an unhandled rejection. - // Injected, not imported from notifications.bridge — that module builds + // Injected, not imported from the old notifications bridge — that module built // its own NotificationsService outside the container. this.notifications.send({ event: 'synology_session_cleared', diff --git a/server/src/nest/notifications/channel-registry.ts b/server/src/nest/notifications/channel-registry.ts index da0f0838c..50712c1b5 100644 --- a/server/src/nest/notifications/channel-registry.ts +++ b/server/src/nest/notifications/channel-registry.ts @@ -4,12 +4,13 @@ import type { ExternalChannel } from './notification-events'; * The live set of notification channels. * * Deliberately a module singleton and not a provider. Two things read it from - * outside the container: notifications.bridge.ts constructs its own - * NotificationsService for the scheduler crons and the memories senders, and - * PluginRuntimeService pushes its channel getter in at onModuleInit. Make this a - * provider and the bridge gets a second, empty registry - plugin channels would - * then go quiet with no error anywhere. The built-ins are registered from - * NotificationsService's constructor, so every path that can dispatch has them. + * outside the container: notifications.instance.ts hands its own + * NotificationsService to the surviving cycle-dodge bridges + * (trips/reservations/airtrail/packing), and PluginRuntimeService pushes its + * channel getter in at onModuleInit. Make this a provider and that instance + * gets a second, empty registry - plugin channels would then go quiet with no + * error anywhere. The built-ins are registered from NotificationsService's + * constructor, so every path that can dispatch has them. */ /** Namespace for plugin channel ids, so a plugin can never claim `email`. */ diff --git a/server/src/nest/notifications/channels/builtins.ts b/server/src/nest/notifications/channels/builtins.ts index 3a789e8b1..aa5573f99 100644 --- a/server/src/nest/notifications/channels/builtins.ts +++ b/server/src/nest/notifications/channels/builtins.ts @@ -25,7 +25,7 @@ export interface BuiltinChannelDeps { * They take their dependencies as an argument rather than importing them, * because the registry they land in is a module singleton while the transports * are providers. Both NotificationsService instances - the container singleton - * and the one notifications.bridge.ts constructs - register from their + * and the one notifications.instance.ts constructs - register from their * constructor, so the registry is populated on every path that can dispatch. * It used to happen through a side-effect import in the preferences service, * which meant the built-ins existed only because somebody happened to import diff --git a/server/src/nest/notifications/notifications.bridge.ts b/server/src/nest/notifications/notifications.bridge.ts deleted file mode 100644 index a055c1ade..000000000 --- a/server/src/nest/notifications/notifications.bridge.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { NotificationsService } from './notifications.service'; -import type { NotificationPayload } from './notifications.service'; -import { notificationsInstance } from './notifications.instance'; - -/** - * Non-Nest entry point for the notifications domain — for code running OUTSIDE - * the Nest container: the scheduler's trip_reminder/todo_due cron requires and - * the legacy services/adminService sender. Exports only the legacy - * services/notificationService name still consumed there, 1:1, so repointing a - * consumer is an import-path-only diff. Inside the container, inject - * NotificationsService — every domain service does now. - * - * The six fire-and-forget senders that used to reach this file through a lazy - * `import(...).then(({ send }) => …)` inject instead. The laziness was working - * around a cycle that no longer exists, and it hid the edge from the module - * graph while handing the send a service built outside the container. - * - * The instance itself lives in notifications.instance.ts, not here: several - * suites replace this module wholesale with `vi.mock(..., () => ({ send }))`, - * and a sibling bridge importing the instance from here would break under those - * mocks. Delete this file when the scheduler and adminService migrate. - */ -const notifications = notificationsInstance(); - -export function send(payload: NotificationPayload): Promise { - return notifications.send(payload); -} - diff --git a/server/src/nest/notifications/notifications.instance.ts b/server/src/nest/notifications/notifications.instance.ts index bab3eea3c..cc55fb5f9 100644 --- a/server/src/nest/notifications/notifications.instance.ts +++ b/server/src/nest/notifications/notifications.instance.ts @@ -9,13 +9,10 @@ import { NotificationsService } from './notifications.service'; /** * The out-of-container NotificationsService, for the bridges that hand-build - * their collaborators. - * - * Separate from notifications.bridge.ts on purpose. That file is the *send* - * entry point for non-Nest callers and several suites replace it wholesale with - * `vi.mock(..., () => ({ send }))`; a sibling bridge importing its instance from - * there would break the moment one of those mocks ran. This module only hands - * out the object. + * their collaborators (trips/reservations/airtrail/packing — the permanent + * cycle-dodges). The reminder crons that used to reach it through a dedicated + * send bridge inject the container singleton now (ReminderJobsService); that + * bridge is gone. * * One instance, module-scoped, because the channel registry is: the built-ins * registered in the constructor and the channels the plugin runtime pushes in diff --git a/server/src/nest/notifications/notifications.module.ts b/server/src/nest/notifications/notifications.module.ts index a6b6e00fb..275132121 100644 --- a/server/src/nest/notifications/notifications.module.ts +++ b/server/src/nest/notifications/notifications.module.ts @@ -3,10 +3,12 @@ import { AdminNotificationPreferencesController, NotificationsController } from import { NotificationsMcp } from './notifications.mcp'; import { NotificationsService } from './notifications.service'; import { NotificationPreferencesService } from './notification-preferences.service'; +import { ReminderJobsService } from './reminder-jobs.service'; import { NtfyService } from './transports/ntfy.service'; import { WebhookService } from './transports/webhook.service'; import { MailerModule } from './mailer/mailer.module'; import { AuthModule } from '../auth/auth.module'; +import { SchedulingModule } from '../scheduling/scheduling.module'; /** Notifications domain (L6 leaf module). Registered in AppModule. * AuthModule feeds NotificationsMcp's demo gate; MailerModule carries SMTP, @@ -16,9 +18,9 @@ import { AuthModule } from '../auth/auth.module'; * in-container consumers (AdminController's dev test send and preferences tab, * the plugin RPC surface, HostSurfaceRpc). */ @Module({ - imports: [AuthModule, MailerModule], + imports: [AuthModule, MailerModule, SchedulingModule], controllers: [NotificationsController, AdminNotificationPreferencesController], - providers: [NotificationsService, NotificationPreferencesService, WebhookService, NtfyService, NotificationsMcp], + providers: [NotificationsService, NotificationPreferencesService, WebhookService, NtfyService, NotificationsMcp, ReminderJobsService], exports: [NotificationsService, NotificationPreferencesService], }) export class NotificationsModule {} diff --git a/server/src/nest/notifications/notifications.service.ts b/server/src/nest/notifications/notifications.service.ts index fd431532b..06220b509 100644 --- a/server/src/nest/notifications/notifications.service.ts +++ b/server/src/nest/notifications/notifications.service.ts @@ -293,13 +293,12 @@ function shouldSendToUser( * no longer execute the action twice; a handler failure releases the claim). * The channel transports and the preference matrix are injected providers * since the notifications fold; the channel registry stays a module singleton - * because notifications.bridge.ts and the plugin runtime both reach it from + * because notifications.instance.ts and the plugin runtime both reach it from * outside the container. * - * Non-Nest consumers (scheduler, legacy adminService/memories, the lazy - * fire-and-forget sends in migrated Nest services) go through - * `notifications.bridge.ts`; the plugin RPC host injects this service via - * `HostSurfaceRpc`. + * Non-Nest consumers (the surviving cycle-dodge bridges) go through + * `notifications.instance.ts`; the plugin RPC host injects this service via + * `HostSurfaceRpc`. The reminder crons inject it too (ReminderJobsService). */ @Injectable() export class NotificationsService { @@ -311,10 +310,10 @@ export class NotificationsService { private readonly ntfy: NtfyService, private readonly prefs: NotificationPreferencesService, ) { - // The registry is a module singleton shared with notifications.bridge.ts and - // the plugin runtime, so registering from here means every path that can - // dispatch has the built-ins — including the bridge instance, which never - // runs onModuleInit. registerChannel is an idempotent Map.set. + // The registry is a module singleton shared with notifications.instance.ts + // and the plugin runtime, so registering from here means every path that can + // dispatch has the built-ins — including the instance the surviving bridges + // use, which never runs onModuleInit. registerChannel is an idempotent Map.set. registerBuiltinChannels({ mailer, webhook, ntfy }); } diff --git a/server/src/nest/notifications/reminder-jobs.service.ts b/server/src/nest/notifications/reminder-jobs.service.ts new file mode 100644 index 000000000..d78a458de --- /dev/null +++ b/server/src/nest/notifications/reminder-jobs.service.ts @@ -0,0 +1,135 @@ +import { Injectable, type OnApplicationBootstrap } from '@nestjs/common'; +import { logInfo, logError } from '../audit/audit-log.logger'; +import { DatabaseService } from '../database/database.service'; +import { NotificationsService } from './notifications.service'; +import { CronRegistrarService } from '../scheduling/cron-registrar.service'; + +/** + * The trip-reminder and todo-due reminder crons, in the domain that owns them + * (moved from src/scheduler.ts — they were the last consumers of the old + * notifications bridge, which died with the move). Both run daily at 9 AM + * app-tz and read their enable gate from app_settings per tick, so toggling + * notify_trip_reminder / notify_todo_due takes effect at the next run without + * a restart. + */ + +// Each todo is reminded at most once per ~24 h (tracked via +// todo_items.reminded_at) so the cron doesn't spam the user every morning +// leading up to the deadline. +const TODO_REMINDER_LEAD_DAYS = 3; + +@Injectable() +export class ReminderJobsService implements OnApplicationBootstrap { + constructor( + private readonly db: DatabaseService, + private readonly notifications: NotificationsService, + private readonly registrar: CronRegistrarService, + ) {} + + private getSetting(key: string): string | undefined { + return this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', key)?.value; + } + + onApplicationBootstrap(): void { + if (!this.registrar.isEnabled()) return; + + // Boot banners only — the enable gates are read per tick below; these + // reflect the state at boot. + try { + const reminderEnabled = this.getSetting('notify_trip_reminder') !== 'false'; + const channelsRaw = this.getSetting('notification_channels') || this.getSetting('notification_channel') || 'none'; + const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map(c => c.trim()); + if (!reminderEnabled) { + logInfo('Trip reminders: disabled in settings'); + } else { + const tripCount = this.db.get<{ c: number }>('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL')?.c ?? 0; + logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); + } + + if (this.getSetting('notify_todo_due') !== 'false') { + logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); + } else { + logInfo('Todo due reminders: disabled in settings'); + } + } catch { + /* banners are best-effort */ + } + + this.registrar.register('trip-reminders', '0 9 * * *', () => this.tripTick()); + this.registrar.register('todo-reminders', '0 9 * * *', () => this.todoTick()); + } + + /** Daily check for trips starting exactly reminder_days from now. */ + async tripTick(): Promise { + try { + if (this.getSetting('notify_trip_reminder') === 'false') return; + + const trips = this.db.all<{ id: number; title: string; user_id: number; reminder_days: number }>(` + SELECT t.id, t.title, t.user_id, t.reminder_days FROM trips t + WHERE t.reminder_days > 0 + AND t.start_date IS NOT NULL + AND t.start_date = date('now', '+' || t.reminder_days || ' days') + `); + + for (const trip of trips) { + await this.notifications.send({ event: 'trip_reminder', actorId: null, scope: 'trip', targetId: trip.id, params: { trip: trip.title, tripId: String(trip.id) } }).catch(() => {}); + } + + if (trips.length > 0) { + logInfo(`Trip reminders sent for ${trips.length} trip(s): ${trips.map(t => `"${t.title}" (${t.reminder_days}d)`).join(', ')}`); + } + } catch (err: unknown) { + logError(`Trip reminder check failed: ${err instanceof Error ? err.message : err}`); + } + } + + /** Daily check for unchecked todos due inside the lead window. */ + async todoTick(): Promise { + try { + if (this.getSetting('notify_todo_due') === 'false') return; + + // Select unchecked todos with a due date inside the lead window + // that haven't been reminded in the last 24 hours. `due_date` is + // stored as a YYYY-MM-DD text; SQLite date() handles it directly. + const todos = this.db.all<{ + id: number; trip_id: number; name: string; due_date: string; + assigned_user_id: number | null; trip_title: string; trip_owner_id: number; + }>(` + SELECT ti.id, ti.trip_id, ti.name, ti.due_date, ti.assigned_user_id, + t.title AS trip_title, t.user_id AS trip_owner_id + FROM todo_items ti + JOIN trips t ON t.id = ti.trip_id + WHERE ti.checked = 0 + AND ti.due_date IS NOT NULL + AND ti.due_date <> '' + AND date(ti.due_date) <= date('now', '+' || ? || ' days') + AND date(ti.due_date) >= date('now') + AND (ti.reminded_at IS NULL OR ti.reminded_at <= datetime('now', '-20 hours')) + `, TODO_REMINDER_LEAD_DAYS); + + for (const todo of todos) { + const targetScope: 'user' | 'trip' = todo.assigned_user_id ? 'user' : 'trip'; + const targetId = todo.assigned_user_id ?? todo.trip_id; + await this.notifications.send({ + event: 'todo_due', + actorId: null, + scope: targetScope, + targetId, + params: { + todo: todo.name, + trip: todo.trip_title, + tripId: String(todo.trip_id), + due: todo.due_date, + }, + }).catch(() => {}); + this.db.run('UPDATE todo_items SET reminded_at = CURRENT_TIMESTAMP WHERE id = ?', todo.id); + } + + if (todos.length > 0) { + logInfo(`Todo reminders sent for ${todos.length} item(s)`); + } + } catch (err: unknown) { + logError(`Todo reminder check failed: ${err instanceof Error ? err.message : err}`); + } + } +} diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 756c66cc6..16665994b 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -782,7 +782,7 @@ export class PackingService { /** Fire-and-forget tag notification, mirroring the legacy dynamic import. */ notifyTagged(tripId: string, actor: User, category: string, userIds: unknown): void { if (!Array.isArray(userIds) || userIds.length === 0) return; - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 248fe1c96..6e54f5a71 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -152,7 +152,7 @@ export class ReservationsService { /** Fire-and-forget booking-change notification, mirroring the legacy dynamic import. */ notifyBookingChange(tripId: string | number, actorId: number, booking: string, type: string): void { - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/nest/scheduling/cron-registrar.service.ts b/server/src/nest/scheduling/cron-registrar.service.ts index d0a21eee1..c11b910c7 100644 --- a/server/src/nest/scheduling/cron-registrar.service.ts +++ b/server/src/nest/scheduling/cron-registrar.service.ts @@ -15,10 +15,11 @@ import { RuntimeEnvService } from '../app-config/runtime-env.service'; * time — tests/setup.ts pins NODE_ENV='test' before any app boots). * Providers gate their boot sweeps and banner logs behind isEnabled() for * the same reason. - * - Shutdown stops everything. @nestjs/schedule's orchestrator only closes - * jobs declared via @Cron decorators, never registry-added ones, so this - * service stops its own in onApplicationShutdown — fired by nestApp.close() - * in prod and by the suites' afterAll close() alike. + * - Shutdown stops everything. @nestjs/schedule v6's orchestrator deletes + * every registry cron job in beforeApplicationShutdown; this service keeps + * its own onApplicationShutdown pass anyway (tolerant of jobs already + * removed) so a library that stops covering registry-added jobs — v5 did + * not — can never leak a timer past nestApp.close(). * * The timezone is resolved per register() call (readEnv().app.tz || 'UTC'), * keeping the retired scheduler's read-at-schedule-time behavior: a dynamic @@ -64,7 +65,11 @@ export class CronRegistrarService implements OnApplicationShutdown { /** Stop and drop a job by name. A name that was never registered is a no-op. */ unregister(name: string): void { if (!this.names.has(name)) return; - this.registry.deleteCronJob(name); // deleteCronJob stops the job before dropping it + // The orchestrator may have already cleared the registry (its + // beforeApplicationShutdown runs before our onApplicationShutdown). + if (this.registry.doesExist('cron', name)) { + this.registry.deleteCronJob(name); // deleteCronJob stops the job before dropping it + } this.names.delete(name); } diff --git a/server/src/nest/trip-members/trip-members.service.ts b/server/src/nest/trip-members/trip-members.service.ts index 9b343292c..c3b1ef0e9 100644 --- a/server/src/nest/trip-members/trip-members.service.ts +++ b/server/src/nest/trip-members/trip-members.service.ts @@ -92,7 +92,7 @@ export class TripMembersService { /** Fire-and-forget trip-invite notification (mirrors the route's dynamic import). */ notifyInvite(tripId: string, actor: User, targetUserId: number, tripTitle: string, inviteeEmail: string): void { - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index 9c27c2a5d..f97a283ff 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -639,7 +639,7 @@ export class VacayService { } catch { /* websocket not available */ } // Notify invited user - // Injected, not a lazy import of notifications.bridge. The laziness bought + // Injected, not a lazy import of the old notifications bridge. The laziness bought // nothing the module graph does not already give — NotificationsModule // reaches nothing in this direction — and it hid the edge while handing the // send a second NotificationsService built outside the container. diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index b697ab799..b5b8a09df 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -44,139 +44,6 @@ function startDemoReset(): void { logInfo('Demo hourly reset scheduled'); } -// Trip reminders: daily check at 9 AM local time for trips starting tomorrow -let reminderTask: ScheduledTask | null = null; - -function startTripReminders(): void { - if (reminderTask) { reminderTask.stop(); reminderTask = null; } - - // Boot banner only — the enable gate is read per tick below (airtrail-style), - // so toggling notify_trip_reminder takes effect at the next 9 AM run without - // a restart. The banner reflects the state at boot. - try { - const { db } = require('./db/database'); - const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - const reminderEnabled = getSetting('notify_trip_reminder') !== 'false'; - const channelsRaw = getSetting('notification_channels') || getSetting('notification_channel') || 'none'; - const activeChannels = channelsRaw === 'none' ? [] : channelsRaw.split(',').map((c: string) => c.trim()); - if (!reminderEnabled) { - logInfo('Trip reminders: disabled in settings'); - } else { - const tripCount = (db.prepare('SELECT COUNT(*) as c FROM trips WHERE reminder_days > 0 AND start_date IS NOT NULL').get() as { c: number }).c; - logInfo(`Trip reminders: enabled via [${activeChannels.join(',')}]${tripCount > 0 ? `, ${tripCount} trip(s) with active reminders` : ''}`); - } - } catch { - /* banner is best-effort */ - } - - const tz = readEnv().app.tz || 'UTC'; - reminderTask = cron.schedule('0 9 * * *', async () => { - try { - const { db } = require('./db/database'); - const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - if (getSetting('notify_trip_reminder') === 'false') return; - const { send } = require('./nest/notifications/notifications.bridge'); - - const trips = db.prepare(` - SELECT t.id, t.title, t.user_id, t.reminder_days FROM trips t - WHERE t.reminder_days > 0 - AND t.start_date IS NOT NULL - AND t.start_date = date('now', '+' || t.reminder_days || ' days') - `).all() as { id: number; title: string; user_id: number; reminder_days: number }[]; - - for (const trip of trips) { - await send({ event: 'trip_reminder', actorId: null, scope: 'trip', targetId: trip.id, params: { trip: trip.title, tripId: String(trip.id) } }).catch(() => {}); - } - - if (trips.length > 0) { - logInfo(`Trip reminders sent for ${trips.length} trip(s): ${trips.map(t => `"${t.title}" (${t.reminder_days}d)`).join(', ')}`); - } - } catch (err: unknown) { - logError(`Trip reminder check failed: ${err instanceof Error ? err.message : err}`); - } - }, { timezone: tz }); -} - -// Todo due-date reminders: daily check at 9 AM for unchecked todos -// whose due_date falls within the next TODO_REMINDER_LEAD_DAYS days. -// Each todo gets reminded at most once per 24 h (tracked via -// todo_items.reminded_at) so the scheduler doesn't spam the user every -// morning leading up to the deadline. -const TODO_REMINDER_LEAD_DAYS = 3; -let todoReminderTask: ScheduledTask | null = null; - -function startTodoReminders(): void { - if (todoReminderTask) { todoReminderTask.stop(); todoReminderTask = null; } - - // Boot banner only — the enable gate is read per tick below (airtrail-style), - // so toggling notify_todo_due takes effect at the next 9 AM run without a - // restart. The banner reflects the state at boot. - try { - const { db } = require('./db/database'); - const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - if (getSetting('notify_todo_due') !== 'false') { - logInfo(`Todo due reminders: enabled (lead ${TODO_REMINDER_LEAD_DAYS}d)`); - } else { - logInfo('Todo due reminders: disabled in settings'); - } - } catch { - /* banner is best-effort */ - } - - const tz = readEnv().app.tz || 'UTC'; - todoReminderTask = cron.schedule('0 9 * * *', async () => { - try { - const { db } = require('./db/database'); - const getSetting = (key: string) => (db.prepare('SELECT value FROM app_settings WHERE key = ?').get(key) as { value: string } | undefined)?.value; - if (getSetting('notify_todo_due') === 'false') return; - const { send } = require('./nest/notifications/notifications.bridge'); - - // Select unchecked todos with a due date inside the lead window - // that haven't been reminded in the last 24 hours. `due_date` is - // stored as a YYYY-MM-DD text; SQLite date() handles it directly. - const todos = db.prepare(` - SELECT ti.id, ti.trip_id, ti.name, ti.due_date, ti.assigned_user_id, - t.title AS trip_title, t.user_id AS trip_owner_id - FROM todo_items ti - JOIN trips t ON t.id = ti.trip_id - WHERE ti.checked = 0 - AND ti.due_date IS NOT NULL - AND ti.due_date <> '' - AND date(ti.due_date) <= date('now', '+' || ? || ' days') - AND date(ti.due_date) >= date('now') - AND (ti.reminded_at IS NULL OR ti.reminded_at <= datetime('now', '-20 hours')) - `).all(TODO_REMINDER_LEAD_DAYS) as { - id: number; trip_id: number; name: string; due_date: string; - assigned_user_id: number | null; trip_title: string; trip_owner_id: number; - }[]; - - for (const todo of todos) { - const targetScope: 'user' | 'trip' = todo.assigned_user_id ? 'user' : 'trip'; - const targetId = todo.assigned_user_id ?? todo.trip_id; - await send({ - event: 'todo_due', - actorId: null, - scope: targetScope, - targetId, - params: { - todo: todo.name, - trip: todo.trip_title, - tripId: String(todo.trip_id), - due: todo.due_date, - }, - }).catch(() => {}); - db.prepare('UPDATE todo_items SET reminded_at = CURRENT_TIMESTAMP WHERE id = ?').run(todo.id); - } - - if (todos.length > 0) { - logInfo(`Todo reminders sent for ${todos.length} item(s)`); - } - } catch (err: unknown) { - logError(`Todo reminder check failed: ${err instanceof Error ? err.message : err}`); - } - }, { timezone: tz }); -} - // Version check: daily at 9 AM — notify admins if a new TREK release is available let versionCheckTask: ScheduledTask | null = null; @@ -334,7 +201,6 @@ function startAirTrailSync(): void { function stop(): void { if (demoTask) { demoTask.stop(); demoTask = null; } - if (reminderTask) { reminderTask.stop(); reminderTask = null; } if (versionCheckTask) { versionCheckTask.stop(); versionCheckTask = null; } if (idempotencyCleanupTask) { idempotencyCleanupTask.stop(); idempotencyCleanupTask = null; } if (trekPhotoCacheTask) { trekPhotoCacheTask.stop(); trekPhotoCacheTask = null; } @@ -342,4 +208,4 @@ function stop(): void { if (airtrailSyncTask) { airtrailSyncTask.stop(); airtrailSyncTask = null; } } -export { stop, startDemoReset, startTripReminders, startTodoReminders, startVersionCheck, startIdempotencyCleanup, purgeExpiredIdempotencyKeys, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync }; +export { stop, startDemoReset, startVersionCheck, startIdempotencyCleanup, purgeExpiredIdempotencyKeys, startTrekPhotoCacheCleanup, startPlacePhotoCacheCleanup, startAirTrailSync }; diff --git a/server/tests/e2e/admin.e2e.test.ts b/server/tests/e2e/admin.e2e.test.ts index f1c9d6fde..829701a2a 100644 --- a/server/tests/e2e/admin.e2e.test.ts +++ b/server/tests/e2e/admin.e2e.test.ts @@ -82,7 +82,6 @@ vi.mock('../../src/nest/notifications/notification-preferences.service', async ( actual.NotificationPreferencesService.prototype.setAdminPreferences = vi.fn(); return actual; }); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { AdminModule } from '../../src/nest/admin/admin.module'; // The admin surface is no longer one module: oidc, the account defaults and the admin diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts index 0357672b8..d5cd3e40c 100644 --- a/server/tests/e2e/collab.e2e.test.ts +++ b/server/tests/e2e/collab.e2e.test.ts @@ -58,7 +58,6 @@ vi.mock('../../src/db/database', () => ({ db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { PermissionsService } from '../../src/nest/permissions/permissions.service'; diff --git a/server/tests/e2e/collections.e2e.test.ts b/server/tests/e2e/collections.e2e.test.ts index 0e4edbe30..dce1138e0 100644 --- a/server/tests/e2e/collections.e2e.test.ts +++ b/server/tests/e2e/collections.e2e.test.ts @@ -34,7 +34,6 @@ vi.mock('../../src/db/database', () => ({ const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) })); vi.mock('../../src/websocket', () => ({ broadcastToUser: vi.fn(), broadcast: vi.fn() })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; diff --git a/server/tests/e2e/packing.e2e.test.ts b/server/tests/e2e/packing.e2e.test.ts index 43ec296c1..72ce206f4 100644 --- a/server/tests/e2e/packing.e2e.test.ts +++ b/server/tests/e2e/packing.e2e.test.ts @@ -110,7 +110,6 @@ vi.mock('../../src/db/database', () => ({ reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { PermissionsService } from '../../src/nest/permissions/permissions.service'; diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts index fdf4ae76e..78deb6afe 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -41,7 +41,6 @@ vi.mock('../../src/db/database', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { notificationSend } = vi.hoisted(() => ({ notificationSend: vi.fn().mockResolvedValue(undefined) })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: notificationSend })); // ReservationsService injects NotificationsService now instead of reaching the // bridge through a lazy import, so the e2e app resolves the real provider out of // its own container — the module mock above never sees the send. The override diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index c0f8520a5..f9a04e9ce 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -104,7 +104,6 @@ vi.mock('../../src/db/database', () => ({ db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); // The audit domain is DI-native now: writeAudit runs for real against the temp // db's audit_log table; only the file logger is silenced. vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); diff --git a/server/tests/e2e/vacay.e2e.test.ts b/server/tests/e2e/vacay.e2e.test.ts index 5b161009f..0b5fdff96 100644 --- a/server/tests/e2e/vacay.e2e.test.ts +++ b/server/tests/e2e/vacay.e2e.test.ts @@ -69,7 +69,6 @@ const { db } = vi.hoisted(() => { vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); vi.mock('../../src/websocket', () => ({ broadcastToUser: vi.fn() })); -vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { DatabaseModule } from '../../src/nest/database/database.module'; import { RealtimeModule } from '../../src/nest/realtime/realtime.module'; diff --git a/server/tests/integration/bootstrap.test.ts b/server/tests/integration/bootstrap.test.ts index 3292f88bb..f588acff4 100644 --- a/server/tests/integration/bootstrap.test.ts +++ b/server/tests/integration/bootstrap.test.ts @@ -44,6 +44,19 @@ vi.mock('../../src/config', () => ({ DEFAULT_LANGUAGE: 'en', })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); +// BOOT-007 boots with NODE_ENV=production, which opens the cron gate — keep the +// registrar inert so a bootstrap test never schedules real jobs or runs boot +// sweeps against the real uploads/data dirs. The gate itself is covered by +// tests/integration/scheduler-gate.test.ts. +vi.mock('../../src/nest/scheduling/cron-registrar.service', () => ({ + CronRegistrarService: class { + isEnabled() { return false; } + register() { return false; } + unregister() {} + get jobCount() { return 0; } + onApplicationShutdown() {} + }, +})); import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index 640e173d5..bf3a9de67 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -74,10 +74,6 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); if (!fs.existsSync(uploadsDir)) fs.mkdirSync(uploadsDir, { recursive: true }); - // Warm the notifications bridge module: notifyCollab does a fire-and-forget - // dynamic import of it, and a cold load can otherwise race the worker - // teardown ("Cannot load ... after the environment was torn down"). - await import('../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/integration/packing.test.ts b/server/tests/integration/packing.test.ts index ab673fca3..c637aaa8e 100644 --- a/server/tests/integration/packing.test.ts +++ b/server/tests/integration/packing.test.ts @@ -58,10 +58,6 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notifications bridge module: notifyTagged does a fire-and-forget - // dynamic import of it, and a cold load can otherwise race the worker - // teardown ("Cannot load ... after the environment was torn down"). - await import('../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/integration/trips.test.ts b/server/tests/integration/trips.test.ts index 4726581e2..0535e2eb1 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -67,10 +67,6 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notifications bridge module: notifyInvite does a fire-and-forget - // dynamic import of it, and a cold load can otherwise race the worker - // teardown ("Cannot load ... after the environment was torn down"). - await import('../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { resetTestDb(testDb); diff --git a/server/tests/integration/vacay.test.ts b/server/tests/integration/vacay.test.ts index c66424d2e..8e8167472 100644 --- a/server/tests/integration/vacay.test.ts +++ b/server/tests/integration/vacay.test.ts @@ -75,10 +75,6 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notifications bridge module: sendInvite/shareCalendar do a - // fire-and-forget dynamic import of it, and a cold load can otherwise race - // the worker teardown ("Cannot load ... after the environment was torn down"). - await import('../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/mcp/tools-collections.test.ts b/server/tests/unit/mcp/tools-collections.test.ts index 51d2b1dae..55979b8b5 100644 --- a/server/tests/unit/mcp/tools-collections.test.ts +++ b/server/tests/unit/mcp/tools-collections.test.ts @@ -39,7 +39,6 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser })); const { notifSend } = vi.hoisted(() => ({ notifSend: vi.fn().mockResolvedValue(undefined) })); -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifSend })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -70,10 +69,6 @@ beforeAll(async () => { // The collections addon is seeded DISABLED by default and every tool rides // the `when:` addon gate (post-fold quirk fix) — enable it for the suite. testDb.prepare('UPDATE addons SET enabled = 1 WHERE id = ?').run(ADDON_IDS.COLLECTIONS); - // Warm the notifications bridge module: sendInvite does a fire-and-forget - // dynamic import of it, and a cold load can otherwise race the mock registry - // under slow (coverage) runs — same precaution as tests/integration/vacay.test.ts. - await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/mcp/tools-vacay.test.ts b/server/tests/unit/mcp/tools-vacay.test.ts index 1bb36341d..7d13c3ae1 100644 --- a/server/tests/unit/mcp/tools-vacay.test.ts +++ b/server/tests/unit/mcp/tools-vacay.test.ts @@ -43,7 +43,6 @@ const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); // share_vacay_calendar fires a user notification after inserting; stub it out -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/channel-override.test.ts b/server/tests/unit/nest/channel-override.test.ts index 49bc1811b..e9d18a1e5 100644 --- a/server/tests/unit/nest/channel-override.test.ts +++ b/server/tests/unit/nest/channel-override.test.ts @@ -28,8 +28,12 @@ import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, setAppSetting, setNotificationChannels } from '../../helpers/factories'; // The send dispatcher is DI-native since the notifications fold; drive it -// through the same outside-container entry point the scheduler uses. -import { send } from '../../../src/nest/notifications/notifications.bridge'; +// through the same outside-container entry point the surviving cycle-dodge +// bridges (trips/reservations/airtrail/packing) use. +import { notificationsInstance } from '../../../src/nest/notifications/notifications.instance'; +import type { NotificationPayload } from '../../../src/nest/notifications/notifications.service'; + +const send = (payload: NotificationPayload) => notificationsInstance().send(payload); import { NtfyService } from '../../../src/nest/notifications/transports/ntfy.service'; import { WebhookService } from '../../../src/nest/notifications/transports/webhook.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; @@ -210,15 +214,16 @@ describe('a live plugin channel needs no second opt-in', () => { * The seam itself, end to end. * * PluginRuntimeService pushes its channel getter into the registry at - * onModuleInit; notifications.bridge.ts constructs its own NotificationsService - * for the scheduler crons and the memories senders. Both only meet because the - * registry is a module singleton. Make it a provider and the bridge gets an - * empty one — plugin channels then go quiet with no error anywhere, which is - * exactly the kind of failure nothing else here would catch. Every case above - * sets the source by hand; this one goes through the runtime. + * onModuleInit; notifications.instance.ts constructs its own + * NotificationsService for the surviving cycle-dodge bridges + * (trips/reservations/airtrail/packing). Both only meet because the registry is + * a module singleton. Make it a provider and that instance gets an empty one — + * plugin channels then go quiet with no error anywhere, which is exactly the + * kind of failure nothing else here would catch. Every case above sets the + * source by hand; this one goes through the runtime. */ -describe('the plugin channel source reaches the bridge instance', () => { - it('CHOVR-015 — a channel the runtime publishes is delivered by notifications.bridge.send', async () => { +describe('the plugin channel source reaches the outside-container instance', () => { + it('CHOVR-015 — a channel the runtime publishes is delivered by notificationsInstance().send', async () => { const { user } = createUser(testDb, { username: 'recipient' }); setNotificationChannels(testDb, 'none'); const delivered: Array<{ userId: number; title: string }> = []; diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index fe737f6b8..98319ccf3 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -42,7 +42,6 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcastToUser, broadcast: vi.fn() })); const notifSend = vi.fn().mockResolvedValue(undefined); -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifSend })); import fs from 'fs'; import path from 'path'; @@ -84,10 +83,6 @@ function clearCollections() { beforeAll(async () => { createTables(testDb); runMigrations(testDb); - // Warm the notifications bridge module: sendInvite does a fire-and-forget - // dynamic import of it, and a cold load can otherwise race the mock registry - // under slow (coverage) runs — same precaution as tests/integration/vacay.test.ts. - await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/nest/cron-registrar.service.test.ts b/server/tests/unit/nest/cron-registrar.service.test.ts index bb9ee135d..2e2a4e0ac 100644 --- a/server/tests/unit/nest/cron-registrar.service.test.ts +++ b/server/tests/unit/nest/cron-registrar.service.test.ts @@ -127,6 +127,16 @@ describe('CronRegistrarService', () => { expect(registrar.jobCount).toBe(0); }); + it("CRONREG-009 — shutdown tolerates the orchestrator having already cleared the registry", () => { + // @nestjs/schedule v6 deletes every registry cron job in its own + // beforeApplicationShutdown, which runs before our onApplicationShutdown. + const { registrar, registry } = makeRegistrar(false); + registrar.register('job', '0 2 * * *', () => {}); + registry.deleteCronJob('job'); // the orchestrator's pass + expect(() => registrar.onApplicationShutdown()).not.toThrow(); + expect(registrar.jobCount).toBe(0); + }); + it('CRONREG-008 — the disabled path still clears an already-registered name (restart parity)', () => { // A dynamic job (auto-backup) restarts by re-registering; if settings turn // it off the register call must still drop the old schedule. diff --git a/server/tests/unit/nest/notifications.service.test.ts b/server/tests/unit/nest/notifications.service.test.ts index 4a920ecb0..38d49f906 100644 --- a/server/tests/unit/nest/notifications.service.test.ts +++ b/server/tests/unit/nest/notifications.service.test.ts @@ -76,7 +76,7 @@ import { createUser, createAdmin, setAppSetting, setNotificationChannels, disabl import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { NotificationsService, type NotificationPayload } from '../../../src/nest/notifications/notifications.service'; -import { send as bridgeSend } from '../../../src/nest/notifications/notifications.bridge'; +import { notificationsInstance } from '../../../src/nest/notifications/notifications.instance'; import { setPluginChannelSource } from '../../../src/nest/notifications/channel-registry'; // The channel interface lives in notification-events; channel-registry only imports // it for its own signatures and never re-exported it. @@ -719,15 +719,17 @@ describe('send() — plugin notification channels', () => { }); // ───────────────────────────────────────────────────────────────────────────── -// Bridge delegation (notifications.bridge.ts — outside-container entry point) +// Outside-container delegation (notifications.instance.ts — the entry point the +// surviving cycle-dodge bridges use; its old send bridge died with the +// reminder-cron migration) // ───────────────────────────────────────────────────────────────────────────── -describe('notifications.bridge', () => { +describe('notifications.instance', () => { it('NSVC-020 — send delegates to the DI service over the shared db proxy', async () => { const { user } = createUser(testDb); setNotificationChannels(testDb, 'none'); - await bridgeSend({ event: 'trip_invite', actorId: null, scope: 'user', targetId: user.id, params: { trip: 'Lisbon', actor: 'Alice', invitee: 'Bob', tripId: '7' } }); + await notificationsInstance().send({ event: 'trip_invite', actorId: null, scope: 'user', targetId: user.id, params: { trip: 'Lisbon', actor: 'Alice', invitee: 'Bob', tripId: '7' } }); expect(countAllNotifications()).toBe(1); expect(broadcastMock).toHaveBeenCalledTimes(1); diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index 8b2ae0b54..837809db8 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -55,7 +55,6 @@ const checkPermission = vi.fn(() => true); const permissionsStub = { checkPermission } as unknown as PermissionsService; const { send } = vi.hoisted(() => ({ send: vi.fn(() => Promise.resolve()) })); -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/reminder-jobs.service.test.ts b/server/tests/unit/nest/reminder-jobs.service.test.ts new file mode 100644 index 000000000..9ae11914d --- /dev/null +++ b/server/tests/unit/nest/reminder-jobs.service.test.ts @@ -0,0 +1,237 @@ +/** + * ReminderJobsService — the trip + todo reminder crons in their owning domain + * (moved from src/scheduler.ts). Proves the bootstrap registration + banners, + * the per-tick enable gates, the reminder selection windows, the todo 20h + * dedup via reminded_at, the user-vs-trip scope routing, and that a failing + * tick is contained to a log line. + */ +import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest'; + +const { testDb, dbMock } = vi.hoisted(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + return { + testDb: db, + dbMock: { db, closeDb: () => {}, reinitialize: () => {}, getPlaceWithTags: () => null, canAccessTrip: () => undefined, isOwner: () => false }, + }; +}); + +const logMock = vi.hoisted(() => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() })); + +vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); +vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock); +vi.mock('../../../src/config', () => ({ + JWT_SECRET: 'test-secret', + ENCRYPTION_KEY: 'a'.repeat(64), + updateJwtSecret: () => {}, +})); + +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { createUser, createTrip, createTodoItem, setAppSetting, setNotificationChannels } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { ReminderJobsService } from '../../../src/nest/notifications/reminder-jobs.service'; +import { notificationsStub } from '../../helpers/notifications'; +import type { NotificationsService } from '../../../src/nest/notifications/notifications.service'; +import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service'; + +interface Registered { + name: string; + expr: string; + onTick: () => Promise | void; +} + +function makeJobs(overrides: { notifications?: NotificationsService } = {}) { + const registered: Registered[] = []; + const registrar = { + isEnabled: vi.fn(() => true), + register: vi.fn((name: string, expr: string, onTick: Registered['onTick']) => { + registered.push({ name, expr, onTick }); + return true; + }), + unregister: vi.fn(), + }; + const send = vi.fn().mockResolvedValue(undefined); + const svc = new ReminderJobsService( + new DatabaseService(testDb), + overrides.notifications ?? notificationsStub(send), + registrar as unknown as CronRegistrarService, + ); + return { svc, registered, registrar, send }; +} + +/** A trip whose start_date sits exactly `days` ahead, with reminders on. */ +function tripWithReminder(userId: number, days: number, title = 'Lisbon'): number { + const trip = createTrip(testDb, userId, { title }); + testDb.prepare("UPDATE trips SET reminder_days = ?, start_date = date('now', '+' || ? || ' days') WHERE id = ?").run(days, days, trip.id); + return trip.id; +} + +beforeAll(() => { + createTables(testDb); + runMigrations(testDb); +}); + +beforeEach(() => { + vi.clearAllMocks(); + testDb.exec(` + DELETE FROM todo_items; + DELETE FROM app_settings; + DELETE FROM trip_members; + DELETE FROM trips; + DELETE FROM users; + `); +}); + +describe('ReminderJobsService bootstrap', () => { + it('RJOB-001 — registers both 9 AM crons under their names', () => { + const { svc, registered } = makeJobs(); + svc.onApplicationBootstrap(); + expect(registered.map(r => [r.name, r.expr])).toEqual([ + ['trip-reminders', '0 9 * * *'], + ['todo-reminders', '0 9 * * *'], + ]); + }); + + it('RJOB-002 — does nothing under the test gate (no crons, no banners)', () => { + const { svc, registered, registrar } = makeJobs(); + registrar.isEnabled.mockReturnValue(false); + svc.onApplicationBootstrap(); + expect(registered).toHaveLength(0); + expect(logMock.logInfo).not.toHaveBeenCalled(); + }); + + it('RJOB-003 — logs the enabled banners by default and the disabled ones when toggled off', () => { + const { svc } = makeJobs(); + svc.onApplicationBootstrap(); + expect(logMock.logInfo).toHaveBeenCalledWith('Trip reminders: enabled via []'); + expect(logMock.logInfo).toHaveBeenCalledWith('Todo due reminders: enabled (lead 3d)'); + + logMock.logInfo.mockClear(); + setAppSetting(testDb, 'notify_trip_reminder', 'false'); + setAppSetting(testDb, 'notify_todo_due', 'false'); + const { svc: svc2 } = makeJobs(); + svc2.onApplicationBootstrap(); + expect(logMock.logInfo).toHaveBeenCalledWith('Trip reminders: disabled in settings'); + expect(logMock.logInfo).toHaveBeenCalledWith('Todo due reminders: disabled in settings'); + }); + + it('RJOB-004 — the trip banner carries the active channels and the reminder-trip count', () => { + const { user } = createUser(testDb); + setNotificationChannels(testDb, 'email'); + tripWithReminder(user.id, 5); + const { svc } = makeJobs(); + svc.onApplicationBootstrap(); + expect(logMock.logInfo).toHaveBeenCalledWith('Trip reminders: enabled via [email], 1 trip(s) with active reminders'); + }); +}); + +describe('trip reminder tick', () => { + it('RJOB-005 — sends for a trip starting exactly reminder_days out and logs the summary', async () => { + const { user } = createUser(testDb); + const tripId = tripWithReminder(user.id, 3); + tripWithReminder(user.id, 5, 'Also due'); // its own window lines up too (start = now + reminder_days) + // A trip whose start date does NOT line up with its reminder window: + const off = createTrip(testDb, user.id, { title: 'Off-window' }); + testDb.prepare("UPDATE trips SET reminder_days = 2, start_date = date('now', '+9 days') WHERE id = ?").run(off.id); + + const { svc, send } = makeJobs(); + await svc.tripTick(); + + const targets = send.mock.calls.map(([p]) => p.targetId); + expect(targets).toContain(tripId); + expect(targets).not.toContain(off.id); + expect(send).toHaveBeenCalledWith(expect.objectContaining({ event: 'trip_reminder', scope: 'trip', targetId: tripId, params: expect.objectContaining({ trip: 'Lisbon', tripId: String(tripId) }) })); + expect(logMock.logInfo).toHaveBeenCalledWith(expect.stringMatching(/^Trip reminders sent for 2 trip\(s\): /)); + }); + + it('RJOB-006 — the per-tick gate skips everything when notify_trip_reminder is false', async () => { + const { user } = createUser(testDb); + tripWithReminder(user.id, 3); + setAppSetting(testDb, 'notify_trip_reminder', 'false'); + const { svc, send } = makeJobs(); + await svc.tripTick(); + expect(send).not.toHaveBeenCalled(); + expect(logMock.logInfo).not.toHaveBeenCalled(); + }); + + it('RJOB-007 — a failing tick is contained to the check-failed log line', async () => { + const broken = { send: vi.fn() } as unknown as NotificationsService; + const svc = new ReminderJobsService( + { get: () => { throw new Error('db gone'); }, all: () => { throw new Error('db gone'); }, run: () => { throw new Error('db gone'); } } as unknown as DatabaseService, + broken, + { isEnabled: () => true, register: () => true, unregister: () => {} } as unknown as CronRegistrarService, + ); + await expect(svc.tripTick()).resolves.toBeUndefined(); + expect(logMock.logError).toHaveBeenCalledWith('Trip reminder check failed: db gone'); + await expect(svc.todoTick()).resolves.toBeUndefined(); + expect(logMock.logError).toHaveBeenCalledWith('Todo reminder check failed: db gone'); + }); +}); + +describe('todo reminder tick', () => { + it('RJOB-008 — sends for a due todo, routes trip-scope, stamps reminded_at, and dedups within 20h', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Lisbon' }); + const todo = createTodoItem(testDb, trip.id, { name: 'Pack bags' }); + testDb.prepare("UPDATE todo_items SET due_date = date('now', '+1 day') WHERE id = ?").run(todo.id); + + const { svc, send } = makeJobs(); + await svc.todoTick(); + + expect(send).toHaveBeenCalledTimes(1); + expect(send).toHaveBeenCalledWith(expect.objectContaining({ event: 'todo_due', scope: 'trip', targetId: trip.id, params: expect.objectContaining({ todo: 'Pack bags', trip: 'Lisbon' }) })); + expect(logMock.logInfo).toHaveBeenCalledWith('Todo reminders sent for 1 item(s)'); + const { reminded_at } = testDb.prepare('SELECT reminded_at FROM todo_items WHERE id = ?').get(todo.id) as { reminded_at: string | null }; + expect(reminded_at).not.toBeNull(); + + // The 20h dedup keeps a second same-day run silent. + send.mockClear(); + await svc.todoTick(); + expect(send).not.toHaveBeenCalled(); + }); + + it('RJOB-009 — an assigned todo notifies the assignee (user scope)', async () => { + const { user } = createUser(testDb); + const { user: assignee } = createUser(testDb, { email: 'b@example.test', username: 'assignee' }); + const trip = createTrip(testDb, user.id, { title: 'Lisbon' }); + const todo = createTodoItem(testDb, trip.id, { name: 'Book train' }); + testDb.prepare("UPDATE todo_items SET due_date = date('now'), assigned_user_id = ? WHERE id = ?").run(assignee.id, todo.id); + + const { svc, send } = makeJobs(); + await svc.todoTick(); + expect(send).toHaveBeenCalledWith(expect.objectContaining({ event: 'todo_due', scope: 'user', targetId: assignee.id })); + }); + + it('RJOB-010 — checked, far-future and past-due todos are not selected', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Lisbon' }); + const checked = createTodoItem(testDb, trip.id, { name: 'Done', checked: 1 }); + testDb.prepare("UPDATE todo_items SET due_date = date('now') WHERE id = ?").run(checked.id); + const far = createTodoItem(testDb, trip.id, { name: 'Far' }); + testDb.prepare("UPDATE todo_items SET due_date = date('now', '+10 days') WHERE id = ?").run(far.id); + const past = createTodoItem(testDb, trip.id, { name: 'Past' }); + testDb.prepare("UPDATE todo_items SET due_date = date('now', '-1 day') WHERE id = ?").run(past.id); + + const { svc, send } = makeJobs(); + await svc.todoTick(); + expect(send).not.toHaveBeenCalled(); + expect(logMock.logInfo).not.toHaveBeenCalled(); + }); + + it('RJOB-011 — the per-tick gate skips everything when notify_todo_due is false', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Lisbon' }); + const todo = createTodoItem(testDb, trip.id, { name: 'Pack bags' }); + testDb.prepare("UPDATE todo_items SET due_date = date('now') WHERE id = ?").run(todo.id); + setAppSetting(testDb, 'notify_todo_due', 'false'); + + const { svc, send } = makeJobs(); + await svc.todoTick(); + expect(send).not.toHaveBeenCalled(); + }); +}); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 9ebbd0230..15c77e97a 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -42,7 +42,6 @@ const permissionsStub = { checkPermission } as unknown as PermissionsService; const budget = { createBudgetItem: vi.fn(), updateBudgetItem: vi.fn(), deleteBudgetItem: vi.fn(), linkBudgetItemToReservation: vi.fn() }; const { notif } = vi.hoisted(() => ({ notif: { send: vi.fn().mockResolvedValue(undefined) } })); -vi.mock('../../../src/nest/notifications/notifications.bridge', () => notif); // The bridge builds its own collaborators, so its NotificationsService comes // from notifications.instance rather than from the module above — RESV-BRIDGE-009 // asserts on what the bridge sends, so that is the module to intercept. diff --git a/server/tests/unit/nest/synology.service.test.ts b/server/tests/unit/nest/synology.service.test.ts index b9c07915b..bbe958a98 100644 --- a/server/tests/unit/nest/synology.service.test.ts +++ b/server/tests/unit/nest/synology.service.test.ts @@ -43,7 +43,6 @@ vi.mock('../../../src/utils/ssrfGuard', () => ({ })); // The service fires the session-cleared notice and .catch()es it, so the stub // has to be a promise. -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn(async () => undefined) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/trip-members.service.test.ts b/server/tests/unit/nest/trip-members.service.test.ts index ca5d0f3e2..7b3d4bc97 100644 --- a/server/tests/unit/nest/trip-members.service.test.ts +++ b/server/tests/unit/nest/trip-members.service.test.ts @@ -50,7 +50,6 @@ vi.mock('../../../src/config', () => ({ vi.mock('../../../src/websocket', () => ({ broadcast })); // notifyInvite reaches the bridge through a dynamic import — keep the send in scope // but out of the transports. -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifySend })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index aea947f94..78219402c 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -49,7 +49,6 @@ vi.mock('../../../src/config', () => ({ const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); // notifyInvite fires a notification via a dynamic import — keep it out of unit scope -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/unified-memories.service.test.ts b/server/tests/unit/nest/unified-memories.service.test.ts index 91be7fb09..bf1bc6e34 100644 --- a/server/tests/unit/nest/unified-memories.service.test.ts +++ b/server/tests/unit/nest/unified-memories.service.test.ts @@ -37,9 +37,6 @@ vi.mock('../../../src/config', () => ({ updateJwtSecret: () => {}, })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ - send: vi.fn().mockResolvedValue(undefined), -})); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; diff --git a/server/tests/unit/nest/vacay.service.test.ts b/server/tests/unit/nest/vacay.service.test.ts index 2abbf9cac..7bcb2f147 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -26,7 +26,6 @@ vi.mock('../../../src/config', () => ({ // Mock websocket so notifyPlanUsers doesn't throw vi.mock('../../../src/websocket', () => ({ broadcastToUser: vi.fn() })); // shareCalendar fires a notification after inserting — keep that out of unit scope -vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -51,10 +50,6 @@ const svc = new VacayService(new DatabaseService(testDb), new RealtimeService(), beforeAll(async () => { createTables(testDb); runMigrations(testDb); - // Warm the (mocked) notificationService module: sendInvite/shareCalendar do a - // fire-and-forget dynamic import of it, and a cold load can otherwise race the - // worker teardown ("Cannot load ... after the environment was torn down"). - await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => {