From d203d8cccd9ec6c80132df6d1d7072b3b26d7159 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 2 Aug 2026 11:36:06 +0200 Subject: [PATCH] chore: move notifications backend to proper DI The send() dispatcher (services/notificationService.ts) and the in-app store (services/inAppNotifications.ts) fold together into the DI-native NotificationsService over DatabaseService + RealtimeService; the prefs matrix, transports, channel registry and inAppNotificationActions stay plain infra modules. A 1-export notifications.bridge covers scheduler, legacy adminService/memories and the six lazy fire-and-forget sends; AdminController and the plugin RPC host (24th factory dep) inject. The 5-tool registrar + notifications-in-app resource move to notifications.mcp.ts. Tests move with case IDs preserved (NSVC-*, INOTIF-*, + NSVC-020 bridge pin); ~18 suites repoint path mocks/warm-ups; the module e2e goes DI-native. Also carries the verified-quirk fixes that would normally trail as a separate fix(server) commit (file-overlap with the fold): toUtcIso on all three created_at paths (INOTIF-013/014), respond claims the response CAS before running the action handler with release-on-failure (INOTIF-011/012), synology's fire-and-forget send gains .catch, the dispatch-failure log unwraps Error messages (NSVC-021), and the MCP resource description drops its false 'unread first' claim. --- migrate.md | 4 +- server/src/mcp/resources.ts | 14 +- server/src/mcp/tools.ts | 4 +- server/src/mcp/tools/notifications.ts | 99 --- server/src/nest/README.md | 31 +- server/src/nest/admin/admin.controller.ts | 7 +- server/src/nest/admin/admin.module.ts | 4 +- server/src/nest/collab/collab.service.ts | 2 +- .../nest/collections/collections.service.ts | 4 +- .../notifications/notifications.bridge.ts | 28 + .../nest/notifications/notifications.mcp.ts | 126 +++ .../notifications/notifications.module.ts | 11 +- .../notifications/notifications.service.ts | 761 +++++++++++++++++- server/src/nest/packing/packing.service.ts | 2 +- server/src/nest/plugins/DI-MIGRATION.md | 13 +- .../plugins/host/plugin-host-deps.factory.ts | 11 +- server/src/nest/plugins/plugins.module.ts | 3 +- .../nest/reservations/reservations.service.ts | 2 +- server/src/nest/trips/trips.service.ts | 2 +- server/src/nest/vacay/vacay.service.ts | 4 +- server/src/scheduler.ts | 4 +- server/src/services/adminService.ts | 2 +- server/src/services/inAppNotifications.ts | 371 --------- .../src/services/memories/synologyService.ts | 6 +- .../src/services/memories/unifiedService.ts | 2 +- server/src/services/notificationService.ts | 353 -------- server/tests/e2e/admin.e2e.test.ts | 2 +- server/tests/e2e/collab.e2e.test.ts | 2 +- server/tests/e2e/collections.e2e.test.ts | 2 +- server/tests/e2e/notifications.e2e.test.ts | 76 +- server/tests/e2e/packing.e2e.test.ts | 2 +- server/tests/e2e/reservations.e2e.test.ts | 2 +- server/tests/e2e/trips.e2e.test.ts | 2 +- server/tests/e2e/vacay.e2e.test.ts | 2 +- server/tests/helpers/mcp-test-controllers.ts | 3 + server/tests/helpers/plugin-host.ts | 2 + server/tests/integration/collab.test.ts | 4 +- server/tests/integration/packing.test.ts | 4 +- server/tests/integration/trips.test.ts | 4 +- server/tests/integration/vacay.test.ts | 4 +- .../tests/unit/mcp/tools-collections.test.ts | 6 +- .../unit/mcp/tools-notifications.test.ts | 14 +- server/tests/unit/mcp/tools-vacay.test.ts | 2 +- .../tests/unit/nest/admin.controller.test.ts | 9 +- .../unit/nest/collections.service.test.ts | 6 +- .../notifications.inapp-prefs.test.ts} | 114 ++- .../notifications.service.test.ts} | 64 +- .../tests/unit/nest/packing.service.test.ts | 2 +- .../unit/nest/reservations.service.test.ts | 2 +- server/tests/unit/nest/trips.service.test.ts | 2 +- server/tests/unit/nest/vacay.service.test.ts | 4 +- .../plugins/plugin-host-deps.factory.test.ts | 11 +- .../unit/services/channelOverride.test.ts | 6 +- .../unit/services/memoriesUnified.test.ts | 2 +- 54 files changed, 1257 insertions(+), 968 deletions(-) delete mode 100644 server/src/mcp/tools/notifications.ts create mode 100644 server/src/nest/notifications/notifications.bridge.ts create mode 100644 server/src/nest/notifications/notifications.mcp.ts delete mode 100644 server/src/services/inAppNotifications.ts delete mode 100644 server/src/services/notificationService.ts rename server/tests/unit/{services/inAppNotificationPrefs.test.ts => nest/notifications.inapp-prefs.test.ts} (68%) rename server/tests/unit/{services/notificationService.test.ts => nest/notifications.service.test.ts} (90%) diff --git a/migrate.md b/migrate.md index 629c38b69..eaa56781b 100644 --- a/migrate.md +++ b/migrate.md @@ -28,7 +28,7 @@ Wave 4 — the coupled cluster (order matters here) dayService; the budget/day coupling lives in the Nest wrapper's budget-sync seam and the MCP registrars, which keep their legacy imports until those domains migrate. Reservations went first as the frontier residue fold; dayService (done 2026-07 — the 592-line service folded into the wrapper `DaysService`, the accommodations seam in `nest/reservations/` now injects it, and the hand-rolled reorder/insert transactions became `db.transaction()`) followed; budgetService followed (done 2026-07 — the 755-line money core folded into the wrapper `BudgetService` (settlement maths, FX freeze/rebase, items/members/payers CRUD; the raw settlement writes became `insertSettlement`/`applySettlementUpdate` so the MCP paths keep skipping the freeze); the 11-tool mcp registrar + 3 budget resources moved to the decorator-driven `budget.mcp.ts`; TripsService/ReservationsService/BookingImportService now inject `BudgetService`; a 4-export `budget.bridge.ts` serves tripService, userCleanupService and the legacy trips/transports registrars; `exchange-rates.bridge.ts` was deleted with its last consumers; and the controller adopted `budget.dto.ts` over the shared schemas, clearing all nine budget allow-list entries — **tripService is now unblocked**). -9. tripService (done 2026-07 — the 1121-line hub folded into the wrapper `TripsService`: generateDays, the updateTrip date-shift transaction, members/guests (#973/#1362), the ICS export, copyTripById and getTripSummary; its six bridge imports became injected services (CollabService/VacayService joined the constructor); the 10-tool trips registrar + 3 trip resources + the trip-summary prompt moved to the decorator-driven `trips.mcp.ts` (first `@Prompt`; the static-token deprecation notice rides the `registry.attach` ctx) and the 3 share-link tools to `share.mcp.ts` on the `canShareTrips` predicate; FeedsService and the plugin RPC host inject `TripsService`; a 3-export `trips.bridge.ts` serves the legacy prompts registrar and budget.mcp.ts's owner/member seam; todo/share/collab/vacay bridges deleted with their last consumers; the controller adopted `trips.dto.ts`, clearing all seven trip allow-list entries). placeService followed (done 2026-07 — the 1029-line place core folded into the wrapper `PlacesService` with its pure half in `places.helpers.ts`; the 10-tool registrar + the trip-places resource moved to the decorator-driven `places.mcp.ts` (`search_place` injects `MapsService`); TripsService/DaysMcp/BookingImportService/the plugin RPC host all inject it, so the domain needs **no bridge at all**; the sibling `placeEnrichment` fold absorbed the last `maps.bridge` consumer and deleted that bridge too, and the DTO ratchet cleared all seven `PlacesController` allow-list entries). transitItineraryService followed (done 2026-08 — the last step-4 link and the first pure-helpers relocation with no service fold: the 287-line module is 100% pure (no SQL, no DB, no broadcasts), so the schemas + endpoint/metadata builders moved byte-identical to `nest/transit/transit-itinerary.helpers.ts` (the schemas must stay module-level plain exports — `transit.mcp.ts` consumes them inside `@Tool` decorators, which evaluate before the container exists), its sole consumer `transit.mcp.ts` was a one-import repoint, and a new 21-case `TRANSIT-ITIN-*` characterization suite pins the previously untested superRefine error strings and builder output — **step 4 is complete; the notifications fan-in heads the frontier**). +9. tripService (done 2026-07 — the 1121-line hub folded into the wrapper `TripsService`: generateDays, the updateTrip date-shift transaction, members/guests (#973/#1362), the ICS export, copyTripById and getTripSummary; its six bridge imports became injected services (CollabService/VacayService joined the constructor); the 10-tool trips registrar + 3 trip resources + the trip-summary prompt moved to the decorator-driven `trips.mcp.ts` (first `@Prompt`; the static-token deprecation notice rides the `registry.attach` ctx) and the 3 share-link tools to `share.mcp.ts` on the `canShareTrips` predicate; FeedsService and the plugin RPC host inject `TripsService`; a 3-export `trips.bridge.ts` serves the legacy prompts registrar and budget.mcp.ts's owner/member seam; todo/share/collab/vacay bridges deleted with their last consumers; the controller adopted `trips.dto.ts`, clearing all seven trip allow-list entries). placeService followed (done 2026-07 — the 1029-line place core folded into the wrapper `PlacesService` with its pure half in `places.helpers.ts`; the 10-tool registrar + the trip-places resource moved to the decorator-driven `places.mcp.ts` (`search_place` injects `MapsService`); TripsService/DaysMcp/BookingImportService/the plugin RPC host all inject it, so the domain needs **no bridge at all**; the sibling `placeEnrichment` fold absorbed the last `maps.bridge` consumer and deleted that bridge too, and the DTO ratchet cleared all seven `PlacesController` allow-list entries). transitItineraryService followed (done 2026-08 — the last step-4 link and the first pure-helpers relocation with no service fold: the 287-line module is 100% pure (no SQL, no DB, no broadcasts), so the schemas + endpoint/metadata builders moved byte-identical to `nest/transit/transit-itinerary.helpers.ts` (the schemas must stay module-level plain exports — `transit.mcp.ts` consumes them inside `@Tool` decorators, which evaluate before the container exists), its sole consumer `transit.mcp.ts` was a one-import repoint, and a new 21-case `TRANSIT-ITIN-*` characterization suite pins the previously untested superRefine error strings and builder output — **step 4 is complete**). notificationService followed (done 2026-08 — the notifications fan-in, step 3 of the dependency-honest order, deferred while step 4 ran: the `send()` dispatcher **and** the `inAppNotifications.ts` store folded together into the existing wrapper `NotificationsService` over DatabaseService + RealtimeService; the prefs matrix, smtp/webhook/ntfy transports, channel registry and `inAppNotificationActions` stay plain infra modules (their registry⇄prefs cycle included); a 1-export `notifications.bridge.ts` (`send`) covers scheduler's two cron `require`s, legacy adminService + memories/{unified,synology}, and the six deliberately-lazy fire-and-forget `import()` sends in migrated Nest services (path-only repoints); `AdminController` and the plugin RPC host (24th constructor dep) inject instead; the 5-tool registrar + the `notifications-in-app` resource moved to the decorator-driven `notifications.mcp.ts`; the DTO ratchet cleared all five `NotificationsController` allow-list entries (`testNtfyRequestSchema` gained `.nullable()` server/token — the client sends null to mean "use the saved value") — **adminService is now unblocked and heads the frontier**). Wave 5 — the heavyweights, last @@ -36,7 +36,7 @@ Wave 5 — the heavyweights, last outside the DI wrapper. Two special cases to keep in mind: airportService is lazily required by db/database.ts at boot (the flight-endpoint backfill) — whenever it migrates, that call should move into Nest's bootstrap instead, or you recreate the circular -import; and notifications/notificationService are imported by 6+ other services, so like the wave-4 cluster they should migrate before their dependents or keep function shims in the interim. +import; and notifications/notificationService were imported by 6+ other services, so they had to migrate before their dependents — **cashed in 2026-08**: the fan-in fold above landed exactly that way, `send` shimmed 1:1 through `notifications.bridge.ts` for the remaining legacy dependents (adminService, memories) until their own folds. The through-line: each finished migration lets that domain's tests drop vi.mock('db/database') for overrideProvider(DATABASE_CONNECTION), so the mock seam shrinks wave by wave and disappears with the auth stack. Want me to start on tagService as the pilot? diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index 622fd4ca8..dd5cedf00 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -1,5 +1,4 @@ import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mcp'; -import { getNotifications } from '../services/inAppNotifications'; import { isAddonEnabled } from '../nest/addons/addons.bridge'; import { ADDON_IDS } from '../addons'; import { canAccessJourney, getJourneyFull, listEntries, listJourneys } from '../services/journeyService'; @@ -91,16 +90,9 @@ export function registerResources(server: McpServer, userId: number, scopes: str // The trip-packing-bags resource moved to the DI-discovered // src/nest/packing/packing.mcp.ts (@ResourceTemplate). - // In-app notifications - if (canRead(scopes, 'notifications')) server.registerResource( - 'notifications-in-app', - 'trek://notifications/in-app', - { description: "The current user's in-app notifications (most recent 50, unread first)", mimeType: 'application/json' }, - async (uri) => { - const result = getNotifications(userId, { limit: 50 }); - return jsonContent(uri.href, result); - } - ); + // The notifications-in-app resource moved to the DI-discovered + // src/nest/notifications/notifications.mcp.ts (@Resource, attached via the + // nest-mcp registry in registerTools). // The atlas-stats and atlas-regions resources moved to the DI-discovered // src/nest/atlas/atlas.mcp.ts (@Resource, attached via the nest-mcp registry diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index 9dfe55ee3..d2b635568 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -1,7 +1,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; import { registerJourneyTools } from './tools/journey'; import { registerMapsWeatherTools } from './tools/mapsWeather'; -import { registerNotificationTools } from './tools/notifications'; import { registerTransportTools } from './tools/transports'; import { registerMcpPrompts } from './tools/prompts'; import { getMcpRegistry } from './registry-handoff'; @@ -35,7 +34,8 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ registerMapsWeatherTools(server, userId, scopes); - registerNotificationTools(server, userId, scopes); + // The notification tools moved to the DI-discovered src/nest/notifications/ + // notifications.mcp.ts (@McpController, attached via the nest-mcp registry below). // The atlas tools moved to the DI-discovered src/nest/atlas/atlas.mcp.ts // (@McpController, attached via the nest-mcp registry below). diff --git a/server/src/mcp/tools/notifications.ts b/server/src/mcp/tools/notifications.ts deleted file mode 100644 index a7d51b51d..000000000 --- a/server/src/mcp/tools/notifications.ts +++ /dev/null @@ -1,99 +0,0 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { z } from 'zod'; -import { isDemoUser } from '../../nest/auth/auth.bridge'; -import { - getNotifications, getUnreadCount, - markRead as markNotificationRead, markUnread as markNotificationUnread, - markAllRead, -} from '../../services/inAppNotifications'; -import { - TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, - TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, ok, -} from './_shared'; -import { canRead, canWrite } from '../scopes'; - -export function registerNotificationTools(server: McpServer, userId: number, scopes: string[] | null): void { - const R = canRead(scopes, 'notifications'); - const W = canWrite(scopes, 'notifications'); - - // --- NOTIFICATIONS --- - - if (R) server.registerTool( - 'list_notifications', - { - description: 'List in-app notifications for the current user.', - inputSchema: { - limit: z.number().int().positive().optional().default(20), - offset: z.number().int().min(0).optional().default(0), - unread_only: z.boolean().optional().default(false), - }, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async ({ limit, offset, unread_only }) => { - const result = getNotifications(userId, { limit: limit ?? 20, offset: offset ?? 0, unreadOnly: unread_only ?? false }); - return ok(result); - } - ); - - if (R) server.registerTool( - 'get_unread_notification_count', - { - description: 'Get the number of unread in-app notifications.', - inputSchema: {}, - annotations: TOOL_ANNOTATIONS_READONLY, - }, - async () => { - const count = getUnreadCount(userId); - return ok({ count }); - } - ); - - if (W) server.registerTool( - 'mark_notification_read', - { - description: 'Mark a single notification as read.', - inputSchema: { - notificationId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ notificationId }) => { - if (isDemoUser(userId)) return demoDenied(); - const success = markNotificationRead(notificationId, userId); - if (!success) return { content: [{ type: 'text' as const, text: 'Notification not found.' }], isError: true }; - return ok({ success: true }); - } - ); - - if (W) server.registerTool( - 'mark_notification_unread', - { - description: 'Mark a single notification as unread.', - inputSchema: { - notificationId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ notificationId }) => { - if (isDemoUser(userId)) return demoDenied(); - const success = markNotificationUnread(notificationId, userId); - if (!success) return { content: [{ type: 'text' as const, text: 'Notification not found.' }], isError: true }; - return ok({ success: true }); - } - ); - - if (W) server.registerTool( - 'mark_all_notifications_read', - { - description: "Mark all of the current user's notifications as read.", - inputSchema: {}, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async () => { - if (isDemoUser(userId)) return demoDenied(); - const count = markAllRead(userId); - return ok({ success: true, count }); - } - ); -} diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 139966658..e85a87906 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -29,8 +29,8 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, todo, packing, day-notes, trip-invite, assignments, share, settings, files, collab, vacay, reservations, day, permissions, audit, budget, trip, maps, - transit, place, transit-itinerary, collections, atlas, auth, oidc, passkey — - see the migration recipe below. + transit, place, transit-itinerary, collections, atlas, auth, oidc, passkey, + notifications — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -415,8 +415,33 @@ PASSKEY-SVC-001…030 were written fresh with the fold (characterization over a real `:memory:` DB, `@simplewebauthn/server` mocked at the ceremony-verdict boundary — the repo's first such mock); auth-guard.test.ts's PasskeyController block swapped its path mock for a constructor stub.) +notificationService followed (the notifications fan-in, step 3 of the +dependency-honest order: the `send()` dispatcher **and** the +`inAppNotifications.ts` store folded together into the existing +`NotificationsService` over DatabaseService + RealtimeService — the wrapper's +in-app delegation became the real SQL, while the prefs matrix, the smtp/ +webhook/ntfy transports, the channel registry and `inAppNotificationActions` +stay plain-module imports (graph-classified infra helpers, complete with +their registry⇄prefs cycle). A one-export `notifications.bridge.ts` (`send`) +covers the outside-container consumers — scheduler's two cron `require`s, +legacy adminService and memories/{unified,synology} — **and** the six +deliberately-lazy fire-and-forget `import().then(({ send }) => …)` sends in +migrated Nest services (collab/collections/packing/reservations/trips/vacay, +path-only repoints; kept lazy so a send can never block or cycle a domain +module). In-container static consumers inject instead: `AdminController`'s +dev test send (AdminModule → NotificationsModule) and the plugin host +(`NotificationsService` = `PluginHostDepsFactory`'s 24th dep). The 5-tool +legacy registrar + the `notifications-in-app` resource moved 1:1 to +`notifications.mcp.ts` (no `when:` — the domain is core, not addon-gated). +Tests moved with IDs preserved: notificationService.test.ts → +notifications.service.test.ts (NSVC-001…019/NTFY-SVCB-*/NSVC-PLUG-001…007, +plus the NSVC-020 bridge pin), inAppNotificationPrefs.test.ts → +notifications.inapp-prefs.test.ts (INOTIF-*); ~18 suites repointed their +path mocks/warm-ups to the bridge; admin.controller.test and the plugin-host +suite converted theirs to constructor stubs; the module e2e went DI-native +(real notifications DDL, prefs/transports still path-mocked).) Repeat these steps per -service (next up: **the notifications fan-in** — +service (next up: **adminService**, unblocked by the notifications fold — per the dependency-honest order in `migration-graph.md`). This is a **pure relocation** — byte-identical diff --git a/server/src/nest/admin/admin.controller.ts b/server/src/nest/admin/admin.controller.ts index c2baddffc..cb7a943e3 100644 --- a/server/src/nest/admin/admin.controller.ts +++ b/server/src/nest/admin/admin.controller.ts @@ -9,7 +9,7 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { getClientIp } from '../audit/client-ip'; import { logInfo } from '../audit/audit-log.logger'; import { AuditService } from '../audit/audit.service'; -import { send as sendNotification } from '../../services/notificationService'; +import { NotificationsService } from '../notifications/notifications.service'; import type { User } from '../../types'; /** Throw the legacy {error,status} envelope when a service call reports failure. */ @@ -38,6 +38,7 @@ export class AdminController { private readonly admin: AdminService, private readonly pluginRuntime: PluginRuntimeService, private readonly audit: AuditService, + private readonly notifications: NotificationsService, ) {} // ── Users ── @@ -353,14 +354,14 @@ export class AdminController { throw new NotFoundException(); } try { - await sendNotification({ + await this.notifications.send({ event: body.event ?? 'trip_reminder', actorId: user.id, scope: body.scope ?? 'user', targetId: body.targetId ?? user.id, params: { actor: user.email, ...(body.params ?? {}) }, inApp: body.inApp, - } as unknown as Parameters[0]); + } as unknown as Parameters[0]); return { success: true }; } catch (err) { throw new HttpException({ error: err instanceof Error ? err.message : String(err) }, 400); diff --git a/server/src/nest/admin/admin.module.ts b/server/src/nest/admin/admin.module.ts index af866a5ce..66759c984 100644 --- a/server/src/nest/admin/admin.module.ts +++ b/server/src/nest/admin/admin.module.ts @@ -7,9 +7,11 @@ import { AuditModule } from '../audit/audit.module'; import { AddonsModule } from '../addons/addons.module'; // AuthModule exports PasskeyService for the admin passkey-reset endpoint. import { AuthModule } from '../auth/auth.module'; +// NotificationsModule exports NotificationsService for the dev test-notification send. +import { NotificationsModule } from '../notifications/notifications.module'; @Module({ - imports: [PluginsModule, SettingsModule, AuditModule, AddonsModule, AuthModule], + imports: [PluginsModule, SettingsModule, AuditModule, AddonsModule, AuthModule, NotificationsModule], controllers: [AdminController], providers: [AdminService], }) diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 8937769bd..cf1f0f933 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -495,7 +495,7 @@ export class CollabService { /** Fire-and-forget collab notification (mirrors the legacy route's dynamic import). */ notifyCollab(tripId: string, actor: User, preview?: string): void { - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { const tripInfo = this.db.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId); const params: Record = { trip: tripInfo?.title || 'Untitled', actor: actor.email, tripId: String(tripId) }; if (preview !== undefined) params.preview = preview; diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts index b0bda4b06..bd6cf8bda 100644 --- a/server/src/nest/collections/collections.service.ts +++ b/server/src/nest/collections/collections.service.ts @@ -104,7 +104,7 @@ const MAX_LABELS_PER_COLLECTION = 50; * per-user broadcast fan-outs). The `place_edit` trip permission (copyToTrip) * goes through PermissionsService; per-user WebSocket delivery goes through * RealtimeService; the notification send in sendInvite keeps its call-time - * lazy import of the legacy notificationService (collab precedent). + * lazy import of the notifications bridge (collab precedent). */ @Injectable() export class CollectionsService { @@ -1012,7 +1012,7 @@ export class CollectionsService { this.realtime.broadcastToUser(targetUserId, { type: 'collections:invite', from: { id: inviterId, username: inviterUsername }, collectionId }); - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { send({ event: 'collection_invite', actorId: inviterId, scope: 'user', targetId: targetUserId, params: { actor: inviterEmail, collectionId: String(collectionId) } }).catch(() => {}); }).catch(() => {}); diff --git a/server/src/nest/notifications/notifications.bridge.ts b/server/src/nest/notifications/notifications.bridge.ts new file mode 100644 index 000000000..ef1cc824c --- /dev/null +++ b/server/src/nest/notifications/notifications.bridge.ts @@ -0,0 +1,28 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { RealtimeService } from '../realtime/realtime.service'; +import { NotificationsService } from './notifications.service'; +import type { NotificationPayload } from './notifications.service'; + +/** + * Non-Nest entry point for the notifications domain — for code running + * OUTSIDE the Nest container (the scheduler's trip_reminder/todo_due cron + * requires, the legacy services/adminService and services/memories/* senders) + * plus the deliberately-lazy fire-and-forget `import(...).then(({ send }) => + * …)` sends in migrated Nest services (collab/collections/packing/ + * reservations/trips/vacay — kept lazy so a notification send can never + * block or cycle a domain module). 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 instead (AdminController and the plugin RPC host do). + * Delete this file when the legacy admin/memories services migrate and the + * lazy senders inject. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton. + */ +const notifications = new NotificationsService(new DatabaseService(db), new RealtimeService()); + +export function send(payload: NotificationPayload): Promise { + return notifications.send(payload); +} diff --git a/server/src/nest/notifications/notifications.mcp.ts b/server/src/nest/notifications/notifications.mcp.ts new file mode 100644 index 000000000..a546602c0 --- /dev/null +++ b/server/src/nest/notifications/notifications.mcp.ts @@ -0,0 +1,126 @@ +import { + McpController, Tool, Resource, type McpContext, + TOOL_ANNOTATIONS_READONLY, TOOL_ANNOTATIONS_WRITE, + demoDenied, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { AuthService } from '../auth/auth.service'; +import { NotificationsService } from './notifications.service'; + +function jsonContent(uri: string, data: unknown) { + return { + contents: [{ + uri, + mimeType: 'application/json', + text: JSON.stringify(data, null, 2), + }], + }; +} + +/** + * Notifications MCP surface — ported 1:1 from the legacy registrars: the five + * tools from src/mcp/tools/notifications.ts and the trek://notifications/in-app + * resource from src/mcp/resources.ts (identical names, descriptions, zod input + * schemas, annotations and error/payload shapes — including the legacy + * 'Notification not found.' wording, which differs from the REST route's + * 'Not found'). The legacy `if (R)` / `if (W)` scope checks map to the + * declarative notifications read/write access markers (resolved by + * trekMcpAccessPolicy); there is no `when:` gate — notifications are core, + * not addon-gated. The WS notification:new/updated broadcasts live inside + * NotificationsService, so no safeBroadcast here. + */ +@McpController() +export class NotificationsMcp { + constructor( + private readonly notifications: NotificationsService, + private readonly auth: AuthService, + ) {} + + @Tool({ + name: 'list_notifications', + description: 'List in-app notifications for the current user.', + inputSchema: { + limit: z.number().int().positive().optional().default(20), + offset: z.number().int().min(0).optional().default(0), + unread_only: z.boolean().optional().default(false), + }, + annotations: TOOL_ANNOTATIONS_READONLY, + access: { group: 'notifications', mode: 'read' }, + }) + async listNotifications( + { limit, offset, unread_only }: { limit?: number; offset?: number; unread_only?: boolean }, + ctx: McpContext, + ) { + const result = this.notifications.listInApp(ctx.userId, { limit: limit ?? 20, offset: offset ?? 0, unreadOnly: unread_only ?? false }); + return ok(result); + } + + @Tool({ + name: 'get_unread_notification_count', + description: 'Get the number of unread in-app notifications.', + inputSchema: {}, + annotations: TOOL_ANNOTATIONS_READONLY, + access: { group: 'notifications', mode: 'read' }, + }) + async getUnreadNotificationCount(_input: Record, ctx: McpContext) { + const count = this.notifications.unreadCount(ctx.userId); + return ok({ count }); + } + + @Tool({ + name: 'mark_notification_read', + description: 'Mark a single notification as read.', + inputSchema: { + notificationId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'notifications', mode: 'write' }, + }) + async markNotificationRead({ notificationId }: { notificationId: number }, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + const success = this.notifications.markRead(notificationId, ctx.userId); + if (!success) return { content: [{ type: 'text' as const, text: 'Notification not found.' }], isError: true }; + return ok({ success: true }); + } + + @Tool({ + name: 'mark_notification_unread', + description: 'Mark a single notification as unread.', + inputSchema: { + notificationId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'notifications', mode: 'write' }, + }) + async markNotificationUnread({ notificationId }: { notificationId: number }, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + const success = this.notifications.markUnread(notificationId, ctx.userId); + if (!success) return { content: [{ type: 'text' as const, text: 'Notification not found.' }], isError: true }; + return ok({ success: true }); + } + + @Tool({ + name: 'mark_all_notifications_read', + description: "Mark all of the current user's notifications as read.", + inputSchema: {}, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'notifications', mode: 'write' }, + }) + async markAllNotificationsRead(_input: Record, ctx: McpContext) { + if (this.auth.isDemoUser(ctx.userId)) return demoDenied(); + const count = this.notifications.markAllRead(ctx.userId); + return ok({ success: true, count }); + } + + @Resource({ + name: 'notifications-in-app', + uri: 'trek://notifications/in-app', + description: "The current user's in-app notifications (most recent 50)", + mimeType: 'application/json', + access: { group: 'notifications', mode: 'read' }, + }) + async inAppNotificationsResource(uri: URL, ctx: McpContext) { + const result = this.notifications.listInApp(ctx.userId, { limit: 50 }); + return jsonContent(uri.href, result); + } +} diff --git a/server/src/nest/notifications/notifications.module.ts b/server/src/nest/notifications/notifications.module.ts index 8591ffdfb..e5cd6c225 100644 --- a/server/src/nest/notifications/notifications.module.ts +++ b/server/src/nest/notifications/notifications.module.ts @@ -1,10 +1,17 @@ import { Module } from '@nestjs/common'; import { NotificationsController } from './notifications.controller'; +import { NotificationsMcp } from './notifications.mcp'; import { NotificationsService } from './notifications.service'; +import { AuthModule } from '../auth/auth.module'; -/** Notifications domain (L6 leaf module). Registered in AppModule. */ +/** Notifications domain (L6 leaf module). Registered in AppModule. + * AuthModule feeds NotificationsMcp's demo gate; NotificationsService is + * exported for in-container consumers (AdminController's dev test send and + * the plugin RPC host's PluginHostDepsFactory). */ @Module({ + imports: [AuthModule], controllers: [NotificationsController], - providers: [NotificationsService], + providers: [NotificationsService, NotificationsMcp], + exports: [NotificationsService], }) export class NotificationsModule {} diff --git a/server/src/nest/notifications/notifications.service.ts b/server/src/nest/notifications/notifications.service.ts index abea15e3b..66dbf7f16 100644 --- a/server/src/nest/notifications/notifications.service.ts +++ b/server/src/nest/notifications/notifications.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; import type { ChannelTestResult } from '@trek/shared'; +import { readEnv, getAppUrl } from '../../app-config'; +import { logDebug, logError } from '../audit/audit-log.logger'; import { testSmtp, testWebhook, @@ -8,34 +10,313 @@ import { getUserWebhookUrl, getUserNtfyConfig, getAdminNtfyConfig, + getEventText, + getUserLanguage, } from '../../services/notifications'; import { - getNotifications, - getUnreadCount, - markRead, - markUnread, - markAllRead, - deleteNotification, - deleteAll, - respondToBoolean, -} from '../../services/inAppNotifications'; -import { getPreferencesMatrix, setPreferences } from '../../services/notificationPreferencesService'; -import { getChannel } from '../../services/notifications/channelRegistry'; + getPreferencesMatrix, + setPreferences, + getActiveChannels, + isEnabledForEvent, + getAdminGlobalPref, + isAdminGlobalChannel, + ADMIN_SCOPED_EVENTS, + type NotifEventType, +} from '../../services/notificationPreferencesService'; +import { getChannel, listChannels, type ChannelMessage, type ExternalChannel } from '../../services/notifications/channelRegistry'; +import { getAction } from '../../services/inAppNotificationActions'; +import { avatarUrl } from '../../services/avatarUrl'; +import { DatabaseService } from '../database/database.service'; +import { RealtimeService } from '../realtime/realtime.service'; type NtfyConfig = ReturnType; -type RespondResult = Awaited>; type PreferencesMatrix = ReturnType; +// SQLite's CURRENT_TIMESTAMP is UTC but the string ('YYYY-MM-DD HH:MM:SS') has +// no 'T'/'Z', so `new Date(...)` parses it as LOCAL time. Normalize to ISO-UTC +// so the client renders notification times in the viewer's own timezone (#1149). +function toUtcIso(ts: string): string { + return ts.endsWith('Z') ? ts : ts.replace(' ', 'T') + 'Z'; +} + +type NotificationType = 'simple' | 'boolean' | 'navigate'; +type NotificationScope = 'trip' | 'user' | 'admin'; +type NotificationResponse = 'positive' | 'negative'; + +interface BaseNotificationInput { + type: NotificationType; + scope: NotificationScope; + target: number; + sender_id: number | null; + event_type?: NotifEventType; + title_key: string; + title_params?: Record; + text_key: string; + text_params?: Record; +} + +interface SimpleNotificationInput extends BaseNotificationInput { + type: 'simple'; +} + +interface BooleanNotificationInput extends BaseNotificationInput { + type: 'boolean'; + positive_text_key: string; + negative_text_key: string; + positive_callback: { action: string; payload: Record }; + negative_callback: { action: string; payload: Record }; +} + +interface NavigateNotificationInput extends BaseNotificationInput { + type: 'navigate'; + navigate_text_key: string; + navigate_target: string; +} + +type NotificationInput = SimpleNotificationInput | BooleanNotificationInput | NavigateNotificationInput; + +interface NotificationRow { + id: number; + type: NotificationType; + scope: NotificationScope; + target: number; + sender_id: number | null; + sender_username?: string | null; + sender_avatar?: string | 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; + response: NotificationResponse | null; + navigate_text_key: string | null; + navigate_target: string | null; + is_read: number; + created_at: string; +} + +type RespondResult = { success: boolean; error?: string; notification?: NotificationRow }; + +// ── Event config map ─────────────────────────────────────────────────────── + +interface EventNotifConfig { + inAppType: 'simple' | 'navigate'; + titleKey: string; + textKey: string; + navigateTextKey?: string; + navigateTarget: (params: Record) => string | null; +} + +const EVENT_NOTIFICATION_CONFIG: Record = { + // ── Dev-only test events ────────────────────────────────────────────────── + test_simple: { + inAppType: 'simple', + titleKey: 'notif.test.title', + textKey: 'notif.test.simple.text', + navigateTarget: () => null, + }, + test_boolean: { + inAppType: 'simple', // overridden by inApp.type at call site + titleKey: 'notif.test.title', + textKey: 'notif.test.boolean.text', + navigateTarget: () => null, + }, + test_navigate: { + inAppType: 'navigate', + titleKey: 'notif.test.title', + textKey: 'notif.test.navigate.text', + navigateTextKey: 'notif.action.view', + navigateTarget: () => '/dashboard', + }, + // ── Plugin-mediated notification (#plugins) — raw title/body carried as params, + // rendered by the passthrough keys. The plugin never picks a recipient directly; + // the host forces scope/target to the acting user or a trip they belong to. ────── + plugin_notification: { + inAppType: 'navigate', + titleKey: 'notif.plugin.title', + textKey: 'notif.plugin.text', + navigateTextKey: 'notif.action.view', + navigateTarget: p => p.link || null, + }, + // ── Production events ───────────────────────────────────────────────────── + trip_invite: { + inAppType: 'navigate', + titleKey: 'notif.trip_invite.title', + textKey: 'notif.trip_invite.text', + navigateTextKey: 'notif.action.view_trip', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + booking_change: { + inAppType: 'navigate', + titleKey: 'notif.booking_change.title', + textKey: 'notif.booking_change.text', + navigateTextKey: 'notif.action.view_trip', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + trip_reminder: { + inAppType: 'navigate', + titleKey: 'notif.trip_reminder.title', + textKey: 'notif.trip_reminder.text', + navigateTextKey: 'notif.action.view_trip', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + todo_due: { + inAppType: 'navigate', + titleKey: 'notif.todo_due.title', + textKey: 'notif.todo_due.text', + navigateTextKey: 'notif.action.view_trip', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + vacay_invite: { + inAppType: 'navigate', + titleKey: 'notif.vacay_invite.title', + textKey: 'notif.vacay_invite.text', + navigateTextKey: 'notif.action.view_vacay', + navigateTarget: p => (p.planId ? `/vacay/${p.planId}` : null), + }, + vacay_share: { + inAppType: 'navigate', + titleKey: 'notif.vacay_share.title', + textKey: 'notif.vacay_share.text', + navigateTextKey: 'notif.action.view_vacay', + navigateTarget: () => '/vacay', + }, + collection_invite: { + inAppType: 'navigate', + titleKey: 'notif.collection_invite.title', + textKey: 'notif.collection_invite.text', + navigateTextKey: 'notif.action.view_collection', + navigateTarget: p => (p.collectionId ? `/collections/${p.collectionId}` : '/collections'), + }, + photos_shared: { + inAppType: 'navigate', + titleKey: 'notif.photos_shared.title', + textKey: 'notif.photos_shared.text', + navigateTextKey: 'notif.action.view_trip', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + collab_message: { + inAppType: 'navigate', + titleKey: 'notif.collab_message.title', + textKey: 'notif.collab_message.text', + navigateTextKey: 'notif.action.view_collab', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + packing_tagged: { + inAppType: 'navigate', + titleKey: 'notif.packing_tagged.title', + textKey: 'notif.packing_tagged.text', + navigateTextKey: 'notif.action.view_packing', + navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), + }, + version_available: { + inAppType: 'navigate', + titleKey: 'notif.version_available.title', + textKey: 'notif.version_available.text', + navigateTextKey: 'notif.action.view_admin', + navigateTarget: () => '/admin', + }, + synology_session_cleared: { + inAppType: 'simple', + titleKey: 'notifications.synologySessionCleared.title', + textKey: 'notifications.synologySessionCleared.text', + navigateTarget: () => null, + }, +}; + +// ── Fallback config for unknown event types ──────────────────────────────── + +const FALLBACK_EVENT_CONFIG: EventNotifConfig = { + inAppType: 'simple', + titleKey: 'notif.generic.title', + textKey: 'notif.generic.text', + navigateTarget: () => null, +}; + +// ── Unified send() API ───────────────────────────────────────────────────── + +export interface NotificationPayload { + event: NotifEventType; + actorId: number | null; + params: Record; + scope: 'trip' | 'user' | 'admin'; + targetId: number; // tripId for trip scope, userId for user scope, 0 for admin + /** Optional in-app overrides (e.g. boolean type with callbacks) */ + inApp?: { + type?: 'simple' | 'boolean' | 'navigate'; + positiveTextKey?: string; + negativeTextKey?: string; + positiveCallback?: { action: string; payload: Record }; + negativeCallback?: { action: string; payload: Record }; + navigateTarget?: string; // override the auto-generated navigate target + }; +} + /** - * Thin Nest wrapper around the existing notification services. Channel delivery - * (including the WebSocket push in inAppNotifications) and the preference - * persistence all stay in the upstream services, so behaviour — including - * real-time delivery — is unchanged. The webhook/ntfy fallback resolution that - * the legacy route does inline is exposed here as small accessors so the - * controller can reproduce it exactly. + * Should this channel deliver this event to this recipient? + * + * Encodes, unchanged, the gating the four hand-written dispatch blocks used to do: + * - admin-scoped events (version_available) reach recipients only over a channel + * that bypasses the `notification_channels` toggle (email), gated by the admin + * global pref rather than the per-user opt-out. Their webhook/ntfy copies go out + * once, globally, via sendGlobal() below — not per recipient. + * - everything else: the admin enabled the channel, the user didn't opt out of this + * event on it, and the user has credentials for it. + */ +function shouldSendToUser( + channel: ExternalChannel, + event: NotifEventType, + recipientId: number, + activeChannels: string[], +): boolean { + if (!channel.supportsEvent(event)) return false; + if (channel.isInstanceConfigured && !channel.isInstanceConfigured()) return false; + + if (ADMIN_SCOPED_EVENTS.has(event)) { + if (!channel.bypassesActiveToggleForAdminEvents) return false; + if (!isAdminGlobalChannel(channel.id)) return false; + if (!getAdminGlobalPref(event, channel.id)) return false; + } else { + // A built-in needs the admin's explicit switch; a plugin channel is on because the + // admin enabled the plugin — that IS the opt-in (see getActiveChannels). + if (channel.source === 'builtin' && !activeChannels.includes(channel.id)) return false; + if (!isEnabledForEvent(recipientId, event, channel.id)) return false; + } + + return channel.isConfiguredFor(recipientId); +} + +/** + * DI-native notifications domain service. The in-app notification SQL moved + * 1:1 from the legacy `services/inAppNotifications.ts` (identical statements, + * post-insert re-selects) and the unified `send()` dispatcher moved 1:1 from + * `services/notificationService.ts` (identical event-config map, gating and + * log lines). The trailing fix(server) commit then normalized the carried + * defects: `toUtcIso` now applies on every broadcast/re-select path (#1149 + * was originally patched on only one of three), and `respond` claims the + * response atomically BEFORE running the action handler (double-submits can + * no longer execute the action twice; a handler failure releases the claim). + * Channel transports, the preference matrix and the channel + * registry stay plain-module imports (`services/notifications*`, + * `services/notificationPreferencesService`) — they are classified as + * infra helpers, not wave material, in `migration-graph.md`. + * + * 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 + * `PluginHostDepsFactory`. */ @Injectable() export class NotificationsService { + constructor( + private readonly db: DatabaseService, + private readonly realtime: RealtimeService, + ) {} + getPreferences(userId: number, role: string): PreferencesMatrix { return getPreferencesMatrix(userId, role, 'user'); } @@ -85,37 +366,455 @@ export class NotificationsService { return getAdminNtfyConfig(); } + // ── In-app notification store (from services/inAppNotifications.ts) ─────── + + resolveRecipients(scope: NotificationScope, target: number, excludeUserId?: number | null): number[] { + let userIds: number[] = []; + + // Guests (#1362) are trip members for assignment purposes but have no inbox/email, + // 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 ids = new Set(); + if (owner) ids.add(owner.user_id); + for (const m of members) ids.add(m.user_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); + 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); + } + + // Only exclude sender for group scopes (trip/admin) — for user scope, the target is explicit + if (excludeUserId != null && scope !== 'user') { + userIds = userIds.filter(id => id !== excludeUserId); + } + + return userIds; + } + + /** + * Legacy multi-recipient insert path (resolve + per-recipient pref check in + * one transaction). `send()` resolves recipients itself and uses + * `createNotificationForRecipient`; this remains for direct callers/tests. + */ + createNotification(input: NotificationInput): number[] { + const recipients = this.resolveRecipients(input.scope, input.target, input.sender_id); + if (recipients.length === 0) return []; + + const titleParams = JSON.stringify(input.title_params ?? {}); + const textParams = JSON.stringify(input.text_params ?? {}); + + // Track inserted id → recipientId pairs (some recipients may be skipped by pref check) + const insertedPairs: Array<{ id: number; recipientId: number }> = []; + + this.db.transaction(() => { + 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 && !isEnabledForEvent(recipientId, input.event_type, 'inapp')) { + continue; + } + + let positiveTextKey: string | null = null; + let negativeTextKey: string | null = null; + let positiveCallback: string | null = null; + let negativeCallback: string | null = null; + let navigateTextKey: string | null = null; + let navigateTarget: string | null = null; + + if (input.type === 'boolean') { + positiveTextKey = input.positive_text_key; + negativeTextKey = input.negative_text_key; + positiveCallback = JSON.stringify(input.positive_callback); + negativeCallback = JSON.stringify(input.negative_callback); + } else if (input.type === 'navigate') { + navigateTextKey = input.navigate_text_key; + 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 + ); + + insertedPairs.push({ id: result.lastInsertRowid as number, 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; + + // Broadcast to each recipient + for (const { id: notificationId, recipientId } of insertedPairs) { + const row = this.db.get('SELECT * FROM notifications WHERE id = ?', notificationId) as NotificationRow; + if (!row) continue; + + this.realtime.broadcastToUser(recipientId, { + type: 'notification:new', + notification: { + ...row, + created_at: toUtcIso(row.created_at), + sender_username: sender?.username ?? null, + sender_avatar: avatarUrl({ avatar: sender?.avatar }), + }, + }); + } + + return insertedPairs.map(p => p.id); + } + + /** + * Insert a single in-app notification for one pre-resolved recipient and broadcast via WebSocket. + * Used by send() which handles recipient resolution externally. + */ + createNotificationForRecipient( + input: NotificationInput, + recipientId: number, + sender: { username: string; avatar: string | null } | null + ): number | null { + const titleParams = JSON.stringify(input.title_params ?? {}); + const textParams = JSON.stringify(input.text_params ?? {}); + + let positiveTextKey: string | null = null; + let negativeTextKey: string | null = null; + let positiveCallback: string | null = null; + let negativeCallback: string | null = null; + let navigateTextKey: string | null = null; + let navigateTarget: string | null = null; + + if (input.type === 'boolean') { + positiveTextKey = input.positive_text_key; + negativeTextKey = input.negative_text_key; + positiveCallback = JSON.stringify(input.positive_callback); + negativeCallback = JSON.stringify(input.negative_callback); + } else if (input.type === 'navigate') { + navigateTextKey = input.navigate_text_key; + 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 = result.lastInsertRowid as number; + const row = this.db.get('SELECT * FROM notifications WHERE id = ?', notificationId); + if (!row) return null; + + this.realtime.broadcastToUser(recipientId, { + type: 'notification:new', + notification: { + ...row, + created_at: toUtcIso(row.created_at), + sender_username: sender?.username ?? null, + sender_avatar: avatarUrl({ avatar: sender?.avatar }), + }, + }); + + return notificationId; + } + // Returns the native service shape (NotificationRow[] is a superset of the // client-facing InAppListResult contract); the controller surfaces it as-is. - listInApp(userId: number, options: { limit?: number; offset?: number; unreadOnly?: boolean }) { - return getNotifications(userId, options); + listInApp( + userId: number, + options: { limit?: number; offset?: number; unreadOnly?: boolean } = {} + ): { notifications: NotificationRow[]; total: number; unread_count: number } { + const limit = Math.min(options.limit ?? 20, 50); + 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 mapped = rows.map(r => ({ + ...r, + created_at: toUtcIso(r.created_at), + sender_avatar: avatarUrl({ avatar: r.sender_avatar }), + })); + + return { notifications: mapped, total, unread_count }; } unreadCount(userId: number): number { - return getUnreadCount(userId); + 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; } - markRead(id: number, userId: number): boolean { - return markRead(id, userId); + markRead(notificationId: number, userId: number): boolean { + const result = this.db.run('UPDATE notifications SET is_read = 1 WHERE id = ? AND recipient_id = ?', notificationId, userId); + return result.changes > 0; } - markUnread(id: number, userId: number): boolean { - return markUnread(id, userId); + markUnread(notificationId: number, userId: number): boolean { + const result = this.db.run('UPDATE notifications SET is_read = 0 WHERE id = ? AND recipient_id = ?', notificationId, userId); + return result.changes > 0; } markAllRead(userId: number): number { - return markAllRead(userId); + const result = this.db.run('UPDATE notifications SET is_read = 1 WHERE recipient_id = ? AND is_read = 0', userId); + return result.changes; } - deleteOne(id: number, userId: number): boolean { - return deleteNotification(id, userId); + deleteOne(notificationId: number, userId: number): boolean { + const result = this.db.run('DELETE FROM notifications WHERE id = ? AND recipient_id = ?', notificationId, userId); + return result.changes > 0; } deleteAll(userId: number): number { - return deleteAll(userId); + const result = this.db.run('DELETE FROM notifications WHERE recipient_id = ?', userId); + return result.changes; } - respond(id: number, userId: number, response: 'positive' | 'negative'): Promise { - return respondToBoolean(id, userId, response); + async respond( + notificationId: number, + userId: number, + response: NotificationResponse + ): Promise { + const notification = this.db.get('SELECT * FROM notifications WHERE id = ? AND recipient_id = ?', notificationId, userId); + + if (!notification) return { success: false, error: 'Notification not found' }; + if (notification.type !== 'boolean') return { success: false, error: 'Not a boolean notification' }; + if (notification.response !== null) return { success: false, error: 'Already responded' }; + + const callbackJson = response === 'positive' ? notification.positive_callback : notification.negative_callback; + if (!callbackJson) return { success: false, error: 'No callback defined' }; + + let callback: { action: string; payload: Record }; + try { + callback = JSON.parse(callbackJson); + } catch { + return { success: false, error: 'Invalid callback format' }; + } + + const handler = getAction(callback.action); + if (!handler) return { success: false, error: `Unknown action: ${callback.action}` }; + + // 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 + ); + + if (result.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 + ); + 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 mappedUpdated = { + ...updated, + created_at: toUtcIso(updated.created_at), + sender_avatar: avatarUrl({ avatar: updated.sender_avatar }), + }; + + this.realtime.broadcastToUser(userId, { type: 'notification:updated', notification: mappedUpdated }); + + return { success: true, notification: mappedUpdated }; + } + + // ── Unified dispatcher (from services/notificationService.ts) ───────────── + + async send(payload: NotificationPayload): Promise { + const { event, actorId, params, scope, targetId, inApp } = payload; + + // Resolve recipients based on scope + const recipients = this.resolveRecipients(scope, targetId, actorId); + if (recipients.length === 0) return; + + const configEntry = EVENT_NOTIFICATION_CONFIG[event]; + 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; + this.createNotificationForRecipient({ + type: 'simple', + scope: 'user', + target: actorId, + sender_id: null, + title_key: 'notif.dev.unknown_event.title', + text_key: 'notif.dev.unknown_event.text', + text_params: { event }, + }, actorId, devSender); + } + } + const config = configEntry ?? FALLBACK_EVENT_CONFIG; + const activeChannels = getActiveChannels(); + const channels = listChannels(); + const appUrl = getAppUrl(); + + // Build navigate target (used by email/webhook CTA and in-app navigate) + const navigateTarget = inApp?.navigateTarget ?? config.navigateTarget(params); + const fullLink = navigateTarget ? `${appUrl}${navigateTarget}` : undefined; + + // 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 + : null; + + logDebug(`notificationService.send event=${event} scope=${scope} targetId=${targetId} recipients=${recipients.length} channels=inapp,${activeChannels.join(',')}`); + + // Dispatch to each recipient in parallel + await Promise.all(recipients.map(async (recipientId) => { + const promises: Promise[] = []; + + // ── In-app ────────────────────────────────────────────────────────── + if (isEnabledForEvent(recipientId, event, 'inapp')) { + const inAppType = inApp?.type ?? config.inAppType; + let notifInput: NotificationInput; + + if (inAppType === 'boolean' && inApp?.positiveCallback && inApp?.negativeCallback) { + notifInput = { + type: 'boolean', + scope, + target: targetId, + sender_id: actorId, + event_type: event, + title_key: config.titleKey, + title_params: params, + text_key: config.textKey, + text_params: params, + positive_text_key: inApp.positiveTextKey ?? 'notif.action.accept', + negative_text_key: inApp.negativeTextKey ?? 'notif.action.decline', + positive_callback: inApp.positiveCallback, + negative_callback: inApp.negativeCallback, + }; + } else if (inAppType === 'navigate' && navigateTarget) { + notifInput = { + type: 'navigate', + scope, + target: targetId, + sender_id: actorId, + event_type: event, + title_key: config.titleKey, + title_params: params, + text_key: config.textKey, + text_params: params, + navigate_text_key: config.navigateTextKey ?? 'notif.action.view', + navigate_target: navigateTarget, + }; + } else { + notifInput = { + type: 'simple', + scope, + target: targetId, + sender_id: actorId, + event_type: event, + title_key: config.titleKey, + title_params: params, + text_key: config.textKey, + text_params: params, + }; + } + + promises.push( + Promise.resolve().then(() => this.createNotificationForRecipient(notifInput, recipientId, sender ?? null)) + ); + } + + // ── External channels (email, webhook, ntfy, plugin:*) ─────────────── + // One loop over the registry. The message is rendered once per recipient, in + // their language, and handed to every channel that wants it — so a plugin + // channel never touches i18n. + const deliverable = channels.filter(ch => shouldSendToUser(ch, event, recipientId, activeChannels)); + if (deliverable.length > 0) { + const lang = getUserLanguage(recipientId); + const { title, body } = getEventText(lang, event, params); + const msg: ChannelMessage = { + event, + title, + body, + navigateTarget: navigateTarget ?? undefined, + url: fullLink, + tripName: params.trip, + }; + for (const ch of deliverable) promises.push(ch.sendToUser(recipientId, msg)); + } + + const results = await Promise.allSettled(promises); + for (const result of results) { + if (result.status === 'rejected') { + logError(`notificationService.send channel dispatch failed event=${event} recipient=${recipientId}: ${result.reason instanceof Error ? result.reason.message : result.reason}`); + } + } + })); + + // ── Admin-global copies (scope: admin) ─────────────────────────────── + // One send per channel, over the admin's own credentials, not per-recipient. + // Always rendered in English — there is no single recipient to take a language from. + if (scope === 'admin') { + const globalChannels = channels.filter( + ch => ch.sendGlobal && ch.supportsEvent(event) && isAdminGlobalChannel(ch.id) && getAdminGlobalPref(event, ch.id), + ); + if (globalChannels.length > 0) { + const { title, body } = getEventText('en', event, params); + const msg: ChannelMessage = { event, title, body, navigateTarget: navigateTarget ?? undefined, url: fullLink }; + await Promise.all( + globalChannels.map(ch => + ch.sendGlobal!(msg).catch((err: unknown) => { + logError(`notificationService.send admin ${ch.id} failed event=${event}: ${err instanceof Error ? err.message : err}`); + }), + ), + ); + } + } } } + +export type { NotificationInput, NotificationRow, NotificationType, NotificationScope, NotificationResponse }; diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index 0a25a82f6..6fd1de935 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -591,7 +591,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; - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { const tripInfo = this.db.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId); send({ event: 'packing_tagged', diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 10822d13d..55251dcc2 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -135,8 +135,12 @@ the atlas fold — its nine imported symbols (`listVisitedCountries`, `listManuallyVisitedRegions`, `listBucketList`, `markCountryVisited`, `unmarkCountryVisited`, `markRegionVisited`, `unmarkRegionVisited`, `createBucketItem`, `deleteBucketItem`) became the injected `AtlasService`, -the factory's 23rd constructor dep; the remaining factory import is the -Wave-5 journeys domain). +the factory's 23rd constructor dep; notificationService swapped in 2026-08 +with the notifications fold — its single imported symbol (`send as +sendNotification`, the `sendPluginNotification` closure) became the injected +`NotificationsService`, the factory's 24th constructor dep; the remaining +factory domain imports are `weatherService` (`getWeather`) and the Wave-5 +journeys domain). ### Test impact (as landed) @@ -155,7 +159,10 @@ Wave-5 journeys domain). collections fold (same sentinel behaviors — the status-tagged throws the factory's `mapCollectionError` maps); the atlasService path mock became `atlasStub` with the 2026-08 atlas fold (same sentinel behaviors, keyed by - the service method names — `bucketList`, `markCountry`, …); a + the service method names — `bucketList`, `markCountry`, …); the + notificationService path mock became `notificationsStub` with the 2026-08 + notifications fold (the hoisted `notifySend` handle survived as the stub's + `send`); a file-local shim keeps the historical `createRealRpcHost(id, granted)` call sites and supplies a default no-op router. diff --git a/server/src/nest/plugins/host/plugin-host-deps.factory.ts b/server/src/nest/plugins/host/plugin-host-deps.factory.ts index 0bfd8297a..a92c7b0f0 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -6,7 +6,7 @@ import { isUpdateConflict } from '../../../services/conflictResult'; import { getWeather } from '../../../services/weatherService'; import { BLOCKED_EXTENSIONS, filesDir } from '../../files/files.constants'; import { joinTripAsMember } from '../../../services/tripMembership'; -import { send as sendNotification } from '../../../services/notificationService'; +import { NotificationsService } from '../../notifications/notifications.service'; import { createLlmClient } from '../../llm-parse/llm-client.factory'; import { readUserSettingDecrypted } from '../plugins.service'; import { PluginOAuthService } from '../plugin-oauth.service'; @@ -132,8 +132,8 @@ export interface PluginCallRouter { * * DI-native domain services (budget, exchange-rates, reservations, tags, * categories, todo, packing, day-notes, assignments, oauth, files, collab, - * vacay, trips, places, collections, atlas, the LLM config resolver) and - * the DatabaseService (all inline SQL + the trip-access helper) are + * vacay, trips, places, collections, atlas, notifications, the LLM config + * resolver) and the DatabaseService (all inline SQL + the trip-access helper) are * constructor-injected; legacy `services/*` domains stay plain function * imports until their own migration lands (DI-MIGRATION.md), at which point * each swaps one import for one injected service. @@ -164,6 +164,7 @@ export class PluginHostDepsFactory { private readonly places: PlacesService, private readonly collections: CollectionsService, private readonly atlas: AtlasService, + private readonly notifications: NotificationsService, ) {} /** @@ -362,13 +363,13 @@ export class PluginHostDepsFactory { return { removed: true }; }, // --- Host-mediated notifications. Recipient resolution + channel fan-out + - // per-user preferences are all owned by notificationService.send; the plugin + // per-user preferences are all owned by NotificationsService.send; the plugin // supplies only the target (host-scoped by the router) + plain text. actorId is // null (no user sender), so the message body carries the plugin's content. --- canAccessTripForNotify: (tripId, userId) => !!this.db.canAccessTrip(tripId, userId), sendPluginNotification: async (_pluginId, input) => { if (!budgetFor(id).take('notify', Date.now())) throw new BadParams('daily notification budget exhausted (resets at UTC midnight)'); - await sendNotification({ + await this.notifications.send({ event: 'plugin_notification', actorId: null, params: { title: input.title, body: input.body, ...(input.link ? { link: input.link } : {}) }, diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index 37cf4bc1a..b70a895c9 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -43,6 +43,7 @@ import { AuditModule } from '../audit/audit.module'; import { AddonsModule } from '../addons/addons.module'; import { CollectionsModule } from '../collections/collections.module'; import { AtlasModule } from '../atlas/atlas.module'; +import { NotificationsModule } from '../notifications/notifications.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -54,7 +55,7 @@ import { AtlasModule } from '../atlas/atlas.module'; @Module({ // The DI-native domain services the plugin host wiring injects // (PluginHostDepsFactory); DatabaseModule is @Global, so not listed. - imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule, NotificationsModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, PdfSectionsController, AtlasLayersController, JournalEntryRowsController, PluginUserSettingsController, PluginOAuthController, PluginActivityController], providers: [PluginsService, PluginRuntimeService, PluginRegistryService, PluginOAuthService, PluginHostDepsFactory], // Exported so the admin addon-toggle handler can cascade-disable plugins whose diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 0e7c1572f..1c4867122 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -151,7 +151,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 { - import('../../services/notificationService') + import('../notifications/notifications.bridge') .then(({ send }) => { try { const actor = this.db.get<{ email: string }>('SELECT email FROM users WHERE id = ?', actorId); diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index 9529d3ff8..b28812a96 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -1225,7 +1225,7 @@ export class TripsService { /** Fire-and-forget trip-invite notification (mirrors the route's dynamic import). */ notifyInvite(tripId: string, actor: User, targetUserId: number, tripTitle: string, inviteeEmail: string): void { - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { send({ event: 'trip_invite', actorId: actor.id, diff --git a/server/src/nest/vacay/vacay.service.ts b/server/src/nest/vacay/vacay.service.ts index 94cd27e04..80b9ab0c8 100644 --- a/server/src/nest/vacay/vacay.service.ts +++ b/server/src/nest/vacay/vacay.service.ts @@ -634,7 +634,7 @@ export class VacayService { } catch { /* websocket not available */ } // Notify invited user - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { send({ event: 'vacay_invite', actorId: inviterId, scope: 'user', targetId: targetUserId, params: { actor: inviterEmail, planId: String(planId) } }).catch(() => {}); }); @@ -865,7 +865,7 @@ export class VacayService { this.realtime.broadcastToUser(ownerId, { type: 'vacay:share', from: { id: ownerId } }, socketId); } catch { /* websocket not available */ } - import('../../services/notificationService').then(({ send }) => { + import('../notifications/notifications.bridge').then(({ send }) => { send({ event: 'vacay_share', actorId: ownerId, scope: 'user', targetId: targetUserId, params: { actor: ownerEmail } }).catch(() => {}); }); diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index f53758b4e..5165308e4 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -169,7 +169,7 @@ function startTripReminders(): void { reminderTask = cron.schedule('0 9 * * *', async () => { try { const { db } = require('./db/database'); - const { send } = require('./services/notificationService'); + 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 @@ -214,7 +214,7 @@ function startTodoReminders(): void { const tz = readEnv().app.tz || 'UTC'; todoReminderTask = cron.schedule('0 9 * * *', async () => { try { - const { send } = require('./services/notificationService'); + 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 diff --git a/server/src/services/adminService.ts b/server/src/services/adminService.ts index 495d0d51a..fb15ae119 100644 --- a/server/src/services/adminService.ts +++ b/server/src/services/adminService.ts @@ -13,7 +13,7 @@ import { resolveAuthToggles } from '../nest/auth/auth.bridge'; import { avatarUrl } from './avatarUrl'; import { prepareLlmAddonConfigForWrite, maskLlmAddonConfig } from './llmConfig'; import { getPhotoProviderConfig } from './memories/helpersService'; -import { send as sendNotification } from './notificationService'; +import { send as sendNotification } from '../nest/notifications/notifications.bridge'; import { validatePassword } from './passwordPolicy'; import { getAllPermissions, savePermissions as savePerms, PERMISSION_ACTIONS } from '../nest/permissions/permissions.bridge'; import { deleteUserCompletely } from './userCleanupService'; diff --git a/server/src/services/inAppNotifications.ts b/server/src/services/inAppNotifications.ts deleted file mode 100644 index fbccd6569..000000000 --- a/server/src/services/inAppNotifications.ts +++ /dev/null @@ -1,371 +0,0 @@ -import { db } from '../db/database'; -import { avatarUrl } from './avatarUrl'; -import { broadcastToUser } from '../websocket'; -import { getAction } from './inAppNotificationActions'; -import { isEnabledForEvent, type NotifEventType } from './notificationPreferencesService'; - -// SQLite's CURRENT_TIMESTAMP is UTC but the string ('YYYY-MM-DD HH:MM:SS') has -// no 'T'/'Z', so `new Date(...)` parses it as LOCAL time. Normalize to ISO-UTC -// so the client renders notification times in the viewer's own timezone (#1149). -function toUtcIso(ts: string): string { - return ts.endsWith('Z') ? ts : ts.replace(' ', 'T') + 'Z'; -} - -type NotificationType = 'simple' | 'boolean' | 'navigate'; -type NotificationScope = 'trip' | 'user' | 'admin'; -type NotificationResponse = 'positive' | 'negative'; - -interface BaseNotificationInput { - type: NotificationType; - scope: NotificationScope; - target: number; - sender_id: number | null; - event_type?: NotifEventType; - title_key: string; - title_params?: Record; - text_key: string; - text_params?: Record; -} - -interface SimpleNotificationInput extends BaseNotificationInput { - type: 'simple'; -} - -interface BooleanNotificationInput extends BaseNotificationInput { - type: 'boolean'; - positive_text_key: string; - negative_text_key: string; - positive_callback: { action: string; payload: Record }; - negative_callback: { action: string; payload: Record }; -} - -interface NavigateNotificationInput extends BaseNotificationInput { - type: 'navigate'; - navigate_text_key: string; - navigate_target: string; -} - -type NotificationInput = SimpleNotificationInput | BooleanNotificationInput | NavigateNotificationInput; - -interface NotificationRow { - id: number; - type: NotificationType; - scope: NotificationScope; - target: number; - sender_id: number | null; - sender_username?: string | null; - sender_avatar?: string | 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; - response: NotificationResponse | null; - navigate_text_key: string | null; - navigate_target: string | null; - is_read: number; - created_at: string; -} - -export function resolveRecipients(scope: NotificationScope, target: number, excludeUserId?: number | null): number[] { - let userIds: number[] = []; - - // Guests (#1362) are trip members for assignment purposes but have no inbox/email, - // 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 = db.prepare('SELECT user_id FROM trips WHERE id = ?').get(target) as { user_id: number } | undefined; - const members = db.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(target) as { user_id: number }[]; - const ids = new Set(); - if (owner) ids.add(owner.user_id); - for (const m of members) ids.add(m.user_id); - userIds = Array.from(ids); - } else if (scope === 'user') { - // A guest can be a todo assignee (scope='user'); never notify them. - const u = db.prepare('SELECT is_guest FROM users WHERE id = ?').get(target) as { is_guest?: number } | undefined; - userIds = u && u.is_guest ? [] : [target]; - } else if (scope === 'admin') { - const admins = db.prepare("SELECT id FROM users WHERE role = ? AND COALESCE(is_guest, 0) = 0").all('admin') as { id: number }[]; - userIds = admins.map(a => a.id); - } - - // Only exclude sender for group scopes (trip/admin) — for user scope, the target is explicit - if (excludeUserId != null && scope !== 'user') { - userIds = userIds.filter(id => id !== excludeUserId); - } - - return userIds; -} - -function createNotification(input: NotificationInput): number[] { - const recipients = resolveRecipients(input.scope, input.target, input.sender_id); - if (recipients.length === 0) return []; - - const titleParams = JSON.stringify(input.title_params ?? {}); - const textParams = JSON.stringify(input.text_params ?? {}); - - // Track inserted id → recipientId pairs (some recipients may be skipped by pref check) - const insertedPairs: Array<{ id: number; recipientId: number }> = []; - - const insert = db.transaction(() => { - const stmt = 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 && !isEnabledForEvent(recipientId, input.event_type, 'inapp')) { - continue; - } - - let positiveTextKey: string | null = null; - let negativeTextKey: string | null = null; - let positiveCallback: string | null = null; - let negativeCallback: string | null = null; - let navigateTextKey: string | null = null; - let navigateTarget: string | null = null; - - if (input.type === 'boolean') { - positiveTextKey = input.positive_text_key; - negativeTextKey = input.negative_text_key; - positiveCallback = JSON.stringify(input.positive_callback); - negativeCallback = JSON.stringify(input.negative_callback); - } else if (input.type === 'navigate') { - navigateTextKey = input.navigate_text_key; - 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 - ); - - insertedPairs.push({ id: result.lastInsertRowid as number, recipientId }); - } - }); - - insert(); - - // Fetch sender info once for WS payloads - const sender = input.sender_id - ? (db.prepare('SELECT username, avatar FROM users WHERE id = ?').get(input.sender_id) as { username: string; avatar: string | null } | undefined) - : null; - - // Broadcast to each recipient - for (const { id: notificationId, recipientId } of insertedPairs) { - const row = db.prepare('SELECT * FROM notifications WHERE id = ?').get(notificationId) as NotificationRow; - if (!row) continue; - - broadcastToUser(recipientId, { - type: 'notification:new', - notification: { - ...row, - sender_username: sender?.username ?? null, - sender_avatar: avatarUrl({ avatar: sender?.avatar }), - }, - }); - } - - return insertedPairs.map(p => p.id); -} - -/** - * Insert a single in-app notification for one pre-resolved recipient and broadcast via WebSocket. - * Used by notificationService.send() which handles recipient resolution externally. - */ -export function createNotificationForRecipient( - input: NotificationInput, - recipientId: number, - sender: { username: string; avatar: string | null } | null -): number | null { - const titleParams = JSON.stringify(input.title_params ?? {}); - const textParams = JSON.stringify(input.text_params ?? {}); - - let positiveTextKey: string | null = null; - let negativeTextKey: string | null = null; - let positiveCallback: string | null = null; - let negativeCallback: string | null = null; - let navigateTextKey: string | null = null; - let navigateTarget: string | null = null; - - if (input.type === 'boolean') { - positiveTextKey = input.positive_text_key; - negativeTextKey = input.negative_text_key; - positiveCallback = JSON.stringify(input.positive_callback); - negativeCallback = JSON.stringify(input.negative_callback); - } else if (input.type === 'navigate') { - navigateTextKey = input.navigate_text_key; - navigateTarget = input.navigate_target; - } - - const result = 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `).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 notificationId = result.lastInsertRowid as number; - const row = db.prepare('SELECT * FROM notifications WHERE id = ?').get(notificationId) as NotificationRow | undefined; - if (!row) return null; - - broadcastToUser(recipientId, { - type: 'notification:new', - notification: { - ...row, - created_at: toUtcIso(row.created_at), - sender_username: sender?.username ?? null, - sender_avatar: avatarUrl({ avatar: sender?.avatar }), - }, - }); - - return notificationId; -} - -function getNotifications( - userId: number, - options: { limit?: number; offset?: number; unreadOnly?: boolean } = {} -): { notifications: NotificationRow[]; total: number; unread_count: number } { - const limit = Math.min(options.limit ?? 20, 50); - 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 = db.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(userId, limit, offset) as NotificationRow[]; - - const { total } = db.prepare(`SELECT COUNT(*) as total FROM notifications ${wherePlain}`).get(userId) as { total: number }; - const { unread_count } = db.prepare('SELECT COUNT(*) as unread_count FROM notifications WHERE recipient_id = ? AND is_read = 0').get(userId) as { unread_count: number }; - - const mapped = rows.map(r => ({ - ...r, - created_at: toUtcIso(r.created_at), - sender_avatar: avatarUrl({ avatar: r.sender_avatar }), - })); - - return { notifications: mapped, total, unread_count }; -} - -function getUnreadCount(userId: number): number { - const row = db.prepare('SELECT COUNT(*) as count FROM notifications WHERE recipient_id = ? AND is_read = 0').get(userId) as { count: number }; - return row.count; -} - -function markRead(notificationId: number, userId: number): boolean { - const result = db.prepare('UPDATE notifications SET is_read = 1 WHERE id = ? AND recipient_id = ?').run(notificationId, userId); - return result.changes > 0; -} - -function markUnread(notificationId: number, userId: number): boolean { - const result = db.prepare('UPDATE notifications SET is_read = 0 WHERE id = ? AND recipient_id = ?').run(notificationId, userId); - return result.changes > 0; -} - -function markAllRead(userId: number): number { - const result = db.prepare('UPDATE notifications SET is_read = 1 WHERE recipient_id = ? AND is_read = 0').run(userId); - return result.changes; -} - -function deleteNotification(notificationId: number, userId: number): boolean { - const result = db.prepare('DELETE FROM notifications WHERE id = ? AND recipient_id = ?').run(notificationId, userId); - return result.changes > 0; -} - -function deleteAll(userId: number): number { - const result = db.prepare('DELETE FROM notifications WHERE recipient_id = ?').run(userId); - return result.changes; -} - -async function respondToBoolean( - notificationId: number, - userId: number, - response: NotificationResponse -): Promise<{ success: boolean; error?: string; notification?: NotificationRow }> { - const notification = db.prepare('SELECT * FROM notifications WHERE id = ? AND recipient_id = ?').get(notificationId, userId) as NotificationRow | undefined; - - if (!notification) return { success: false, error: 'Notification not found' }; - if (notification.type !== 'boolean') return { success: false, error: 'Not a boolean notification' }; - if (notification.response !== null) return { success: false, error: 'Already responded' }; - - const callbackJson = response === 'positive' ? notification.positive_callback : notification.negative_callback; - if (!callbackJson) return { success: false, error: 'No callback defined' }; - - let callback: { action: string; payload: Record }; - try { - callback = JSON.parse(callbackJson); - } catch { - return { success: false, error: 'Invalid callback format' }; - } - - const handler = getAction(callback.action); - if (!handler) return { success: false, error: `Unknown action: ${callback.action}` }; - - try { - await handler(callback.payload, userId); - } catch (err) { - return { success: false, error: err instanceof Error ? err.message : 'Action failed' }; - } - - // Atomic update — only updates if response is still NULL (prevents double-response) - const result = db.prepare( - 'UPDATE notifications SET response = ?, is_read = 1 WHERE id = ? AND recipient_id = ? AND response IS NULL' - ).run(response, notificationId, userId); - - if (result.changes === 0) return { success: false, error: 'Already responded' }; - - const updated = db.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(notificationId) as NotificationRow; - - const mappedUpdated = { - ...updated, - sender_avatar: avatarUrl({ avatar: updated.sender_avatar }), - }; - - broadcastToUser(userId, { type: 'notification:updated', notification: mappedUpdated }); - - return { success: true, notification: mappedUpdated }; -} - -export { - createNotification, - getNotifications, - getUnreadCount, - markRead, - markUnread, - markAllRead, - deleteNotification, - deleteAll, - respondToBoolean, -}; - -export type { NotificationInput, NotificationRow, NotificationType, NotificationScope, NotificationResponse }; diff --git a/server/src/services/memories/synologyService.ts b/server/src/services/memories/synologyService.ts index 5a83201e9..f0972ee5c 100644 --- a/server/src/services/memories/synologyService.ts +++ b/server/src/services/memories/synologyService.ts @@ -19,7 +19,7 @@ import { SyncAlbumResult, AssetInfo } from './helpersService'; -import { send as sendNotification } from '../notificationService'; +import { send as sendNotification } from '../../nest/notifications/notifications.bridge'; const SYNOLOGY_PROVIDER = 'synologyphotos'; // Users provide the full base URL including the Photos app path (e.g. https://nas:5001/photo). @@ -374,13 +374,15 @@ export async function updateSynologySettings(userId: number, synologyUrl: string const sessionCleared = urlChanged || userChanged; if (sessionCleared) { _clearSynologySession(userId); + // Fire-and-forget like unifiedService's photos_shared send — without the + // .catch an in-app insert failure became an unhandled rejection. sendNotification({ event: 'synology_session_cleared', actorId: null, params: {}, scope: 'user', targetId: userId, - }); + }).catch(() => {}); } try { diff --git a/server/src/services/memories/unifiedService.ts b/server/src/services/memories/unifiedService.ts index 01ea556f9..821223288 100644 --- a/server/src/services/memories/unifiedService.ts +++ b/server/src/services/memories/unifiedService.ts @@ -1,5 +1,5 @@ import { db, canAccessTrip } from '../../db/database'; -import { send } from '../notificationService'; +import { send } from '../../nest/notifications/notifications.bridge'; import { broadcast } from '../../websocket'; import { ServiceResult, diff --git a/server/src/services/notificationService.ts b/server/src/services/notificationService.ts deleted file mode 100644 index 193e90869..000000000 --- a/server/src/services/notificationService.ts +++ /dev/null @@ -1,353 +0,0 @@ -import { readEnv } from '../app-config'; -import { db } from '../db/database'; -import { logDebug, logError } from '../nest/audit/audit-log.logger'; -import { - getActiveChannels, - isEnabledForEvent, - getAdminGlobalPref, - isAdminGlobalChannel, - ADMIN_SCOPED_EVENTS, - type NotifEventType, -} from './notificationPreferencesService'; -import { getEventText, getUserLanguage } from './notifications'; -import { getAppUrl } from '../app-config'; -import { listChannels, type ChannelMessage, type ExternalChannel } from './notifications/channelRegistry'; -import { - resolveRecipients, - createNotificationForRecipient, - type NotificationInput, -} from './inAppNotifications'; - -// ── Event config map ─────────────────────────────────────────────────────── - -interface EventNotifConfig { - inAppType: 'simple' | 'navigate'; - titleKey: string; - textKey: string; - navigateTextKey?: string; - navigateTarget: (params: Record) => string | null; -} - -const EVENT_NOTIFICATION_CONFIG: Record = { - // ── Dev-only test events ────────────────────────────────────────────────── - test_simple: { - inAppType: 'simple', - titleKey: 'notif.test.title', - textKey: 'notif.test.simple.text', - navigateTarget: () => null, - }, - test_boolean: { - inAppType: 'simple', // overridden by inApp.type at call site - titleKey: 'notif.test.title', - textKey: 'notif.test.boolean.text', - navigateTarget: () => null, - }, - test_navigate: { - inAppType: 'navigate', - titleKey: 'notif.test.title', - textKey: 'notif.test.navigate.text', - navigateTextKey: 'notif.action.view', - navigateTarget: () => '/dashboard', - }, - // ── Plugin-mediated notification (#plugins) — raw title/body carried as params, - // rendered by the passthrough keys. The plugin never picks a recipient directly; - // the host forces scope/target to the acting user or a trip they belong to. ────── - plugin_notification: { - inAppType: 'navigate', - titleKey: 'notif.plugin.title', - textKey: 'notif.plugin.text', - navigateTextKey: 'notif.action.view', - navigateTarget: p => p.link || null, - }, - // ── Production events ───────────────────────────────────────────────────── - trip_invite: { - inAppType: 'navigate', - titleKey: 'notif.trip_invite.title', - textKey: 'notif.trip_invite.text', - navigateTextKey: 'notif.action.view_trip', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - booking_change: { - inAppType: 'navigate', - titleKey: 'notif.booking_change.title', - textKey: 'notif.booking_change.text', - navigateTextKey: 'notif.action.view_trip', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - trip_reminder: { - inAppType: 'navigate', - titleKey: 'notif.trip_reminder.title', - textKey: 'notif.trip_reminder.text', - navigateTextKey: 'notif.action.view_trip', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - todo_due: { - inAppType: 'navigate', - titleKey: 'notif.todo_due.title', - textKey: 'notif.todo_due.text', - navigateTextKey: 'notif.action.view_trip', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - vacay_invite: { - inAppType: 'navigate', - titleKey: 'notif.vacay_invite.title', - textKey: 'notif.vacay_invite.text', - navigateTextKey: 'notif.action.view_vacay', - navigateTarget: p => (p.planId ? `/vacay/${p.planId}` : null), - }, - vacay_share: { - inAppType: 'navigate', - titleKey: 'notif.vacay_share.title', - textKey: 'notif.vacay_share.text', - navigateTextKey: 'notif.action.view_vacay', - navigateTarget: () => '/vacay', - }, - collection_invite: { - inAppType: 'navigate', - titleKey: 'notif.collection_invite.title', - textKey: 'notif.collection_invite.text', - navigateTextKey: 'notif.action.view_collection', - navigateTarget: p => (p.collectionId ? `/collections/${p.collectionId}` : '/collections'), - }, - photos_shared: { - inAppType: 'navigate', - titleKey: 'notif.photos_shared.title', - textKey: 'notif.photos_shared.text', - navigateTextKey: 'notif.action.view_trip', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - collab_message: { - inAppType: 'navigate', - titleKey: 'notif.collab_message.title', - textKey: 'notif.collab_message.text', - navigateTextKey: 'notif.action.view_collab', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - packing_tagged: { - inAppType: 'navigate', - titleKey: 'notif.packing_tagged.title', - textKey: 'notif.packing_tagged.text', - navigateTextKey: 'notif.action.view_packing', - navigateTarget: p => (p.tripId ? `/trips/${p.tripId}` : null), - }, - version_available: { - inAppType: 'navigate', - titleKey: 'notif.version_available.title', - textKey: 'notif.version_available.text', - navigateTextKey: 'notif.action.view_admin', - navigateTarget: () => '/admin', - }, - synology_session_cleared: { - inAppType: 'simple', - titleKey: 'notifications.synologySessionCleared.title', - textKey: 'notifications.synologySessionCleared.text', - navigateTarget: () => null, - }, -}; - -// ── Fallback config for unknown event types ──────────────────────────────── - -const FALLBACK_EVENT_CONFIG: EventNotifConfig = { - inAppType: 'simple', - titleKey: 'notif.generic.title', - textKey: 'notif.generic.text', - navigateTarget: () => null, -}; - -// ── Unified send() API ───────────────────────────────────────────────────── - -export interface NotificationPayload { - event: NotifEventType; - actorId: number | null; - params: Record; - scope: 'trip' | 'user' | 'admin'; - targetId: number; // tripId for trip scope, userId for user scope, 0 for admin - /** Optional in-app overrides (e.g. boolean type with callbacks) */ - inApp?: { - type?: 'simple' | 'boolean' | 'navigate'; - positiveTextKey?: string; - negativeTextKey?: string; - positiveCallback?: { action: string; payload: Record }; - negativeCallback?: { action: string; payload: Record }; - navigateTarget?: string; // override the auto-generated navigate target - }; -} - -/** - * Should this channel deliver this event to this recipient? - * - * Encodes, unchanged, the gating the four hand-written dispatch blocks used to do: - * - admin-scoped events (version_available) reach recipients only over a channel - * that bypasses the `notification_channels` toggle (email), gated by the admin - * global pref rather than the per-user opt-out. Their webhook/ntfy copies go out - * once, globally, via sendGlobal() below — not per recipient. - * - everything else: the admin enabled the channel, the user didn't opt out of this - * event on it, and the user has credentials for it. - */ -function shouldSendToUser( - channel: ExternalChannel, - event: NotifEventType, - recipientId: number, - activeChannels: string[], -): boolean { - if (!channel.supportsEvent(event)) return false; - if (channel.isInstanceConfigured && !channel.isInstanceConfigured()) return false; - - if (ADMIN_SCOPED_EVENTS.has(event)) { - if (!channel.bypassesActiveToggleForAdminEvents) return false; - if (!isAdminGlobalChannel(channel.id)) return false; - if (!getAdminGlobalPref(event, channel.id)) return false; - } else { - // A built-in needs the admin's explicit switch; a plugin channel is on because the - // admin enabled the plugin — that IS the opt-in (see getActiveChannels). - if (channel.source === 'builtin' && !activeChannels.includes(channel.id)) return false; - if (!isEnabledForEvent(recipientId, event, channel.id)) return false; - } - - return channel.isConfiguredFor(recipientId); -} - -export async function send(payload: NotificationPayload): Promise { - const { event, actorId, params, scope, targetId, inApp } = payload; - - // Resolve recipients based on scope - const recipients = resolveRecipients(scope, targetId, actorId); - if (recipients.length === 0) return; - - const configEntry = EVENT_NOTIFICATION_CONFIG[event]; - if (!configEntry) { - logDebug(`notificationService.send: unknown event type "${event}", using fallback`); - if (readEnv().app.isDevelopment && actorId != null) { - const devSender = (db.prepare('SELECT username, avatar FROM users WHERE id = ?').get(actorId) as { username: string; avatar: string | null } | undefined) ?? null; - createNotificationForRecipient({ - type: 'simple', - scope: 'user', - target: actorId, - sender_id: null, - title_key: 'notif.dev.unknown_event.title', - text_key: 'notif.dev.unknown_event.text', - text_params: { event }, - }, actorId, devSender); - } - } - const config = configEntry ?? FALLBACK_EVENT_CONFIG; - const activeChannels = getActiveChannels(); - const channels = listChannels(); - const appUrl = getAppUrl(); - - // Build navigate target (used by email/webhook CTA and in-app navigate) - const navigateTarget = inApp?.navigateTarget ?? config.navigateTarget(params); - const fullLink = navigateTarget ? `${appUrl}${navigateTarget}` : undefined; - - // Fetch sender info once for in-app WS payloads - const sender = actorId - ? (db.prepare('SELECT username, avatar FROM users WHERE id = ?').get(actorId) as { username: string; avatar: string | null } | undefined) ?? null - : null; - - logDebug(`notificationService.send event=${event} scope=${scope} targetId=${targetId} recipients=${recipients.length} channels=inapp,${activeChannels.join(',')}`); - - // Dispatch to each recipient in parallel - await Promise.all(recipients.map(async (recipientId) => { - const promises: Promise[] = []; - - // ── In-app ────────────────────────────────────────────────────────── - if (isEnabledForEvent(recipientId, event, 'inapp')) { - const inAppType = inApp?.type ?? config.inAppType; - let notifInput: NotificationInput; - - if (inAppType === 'boolean' && inApp?.positiveCallback && inApp?.negativeCallback) { - notifInput = { - type: 'boolean', - scope, - target: targetId, - sender_id: actorId, - event_type: event, - title_key: config.titleKey, - title_params: params, - text_key: config.textKey, - text_params: params, - positive_text_key: inApp.positiveTextKey ?? 'notif.action.accept', - negative_text_key: inApp.negativeTextKey ?? 'notif.action.decline', - positive_callback: inApp.positiveCallback, - negative_callback: inApp.negativeCallback, - }; - } else if (inAppType === 'navigate' && navigateTarget) { - notifInput = { - type: 'navigate', - scope, - target: targetId, - sender_id: actorId, - event_type: event, - title_key: config.titleKey, - title_params: params, - text_key: config.textKey, - text_params: params, - navigate_text_key: config.navigateTextKey ?? 'notif.action.view', - navigate_target: navigateTarget, - }; - } else { - notifInput = { - type: 'simple', - scope, - target: targetId, - sender_id: actorId, - event_type: event, - title_key: config.titleKey, - title_params: params, - text_key: config.textKey, - text_params: params, - }; - } - - promises.push( - Promise.resolve().then(() => createNotificationForRecipient(notifInput, recipientId, sender ?? null)) - ); - } - - // ── External channels (email, webhook, ntfy, plugin:*) ─────────────── - // One loop over the registry. The message is rendered once per recipient, in - // their language, and handed to every channel that wants it — so a plugin - // channel never touches i18n. - const deliverable = channels.filter(ch => shouldSendToUser(ch, event, recipientId, activeChannels)); - if (deliverable.length > 0) { - const lang = getUserLanguage(recipientId); - const { title, body } = getEventText(lang, event, params); - const msg: ChannelMessage = { - event, - title, - body, - navigateTarget: navigateTarget ?? undefined, - url: fullLink, - tripName: params.trip, - }; - for (const ch of deliverable) promises.push(ch.sendToUser(recipientId, msg)); - } - - const results = await Promise.allSettled(promises); - for (const result of results) { - if (result.status === 'rejected') { - logError(`notificationService.send channel dispatch failed event=${event} recipient=${recipientId}: ${result.reason}`); - } - } - })); - - // ── Admin-global copies (scope: admin) ─────────────────────────────── - // One send per channel, over the admin's own credentials, not per-recipient. - // Always rendered in English — there is no single recipient to take a language from. - if (scope === 'admin') { - const globalChannels = channels.filter( - ch => ch.sendGlobal && ch.supportsEvent(event) && isAdminGlobalChannel(ch.id) && getAdminGlobalPref(event, ch.id), - ); - if (globalChannels.length > 0) { - const { title, body } = getEventText('en', event, params); - const msg: ChannelMessage = { event, title, body, navigateTarget: navigateTarget ?? undefined, url: fullLink }; - await Promise.all( - globalChannels.map(ch => - ch.sendGlobal!(msg).catch((err: unknown) => { - logError(`notificationService.send admin ${ch.id} failed event=${event}: ${err instanceof Error ? err.message : err}`); - }), - ), - ); - } - } -} diff --git a/server/tests/e2e/admin.e2e.test.ts b/server/tests/e2e/admin.e2e.test.ts index f6ef6fdbd..e0104cb6a 100644 --- a/server/tests/e2e/admin.e2e.test.ts +++ b/server/tests/e2e/admin.e2e.test.ts @@ -39,7 +39,7 @@ vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: ( vi.mock('../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); vi.mock('../../src/mcp', () => ({ invalidateMcpSessions: vi.fn() })); vi.mock('../../src/services/notificationPreferencesService', () => ({ getPreferencesMatrix: vi.fn(() => ({})), setAdminPreferences: vi.fn() })); -vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); const { adminSvc } = vi.hoisted(() => ({ adminSvc: { listUsers: vi.fn(), createUser: vi.fn(), updatePlacesPhotos: vi.fn() }, diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts index 1b6e5610a..0357672b8 100644 --- a/server/tests/e2e/collab.e2e.test.ts +++ b/server/tests/e2e/collab.e2e.test.ts @@ -58,7 +58,7 @@ 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/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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 00357d7ce..0e4edbe30 100644 --- a/server/tests/e2e/collections.e2e.test.ts +++ b/server/tests/e2e/collections.e2e.test.ts @@ -34,7 +34,7 @@ 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/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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/notifications.e2e.test.ts b/server/tests/e2e/notifications.e2e.test.ts index 31d5ae95e..8b874b902 100644 --- a/server/tests/e2e/notifications.e2e.test.ts +++ b/server/tests/e2e/notifications.e2e.test.ts @@ -1,8 +1,10 @@ /** * Notifications module e2e — exercises the migrated /api/notifications endpoints - * through the real JwtAuthGuard against a temp SQLite db. The notification - * services are mocked; this focuses on auth, the inline admin gate on - * /test-smtp, routing (the /in-app/all ordering trap) and status/body shapes. + * through the real JwtAuthGuard against a temp SQLite db. NotificationsService + * runs its real (DI-native) in-app SQL — notifications DDL below; the channel + * transports and the preference matrix (still plain services/* modules) are + * mocked. Focuses on auth, the inline admin gate on /test-smtp, routing (the + * /in-app/all ordering trap) and status/body shapes. */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; import request from 'supertest'; @@ -17,18 +19,40 @@ const { db } = vi.hoisted(() => { const tmp = new Database(':memory:'); tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, - email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`); + email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, + avatar TEXT);`); + // NotificationsService runs its real SQL (DI-native since the notifications + // fold) — full notifications column set for listInApp/markRead/deleteAll. + tmp.exec(`CREATE TABLE notifications ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + type TEXT NOT NULL CHECK(type IN ('simple', 'boolean', 'navigate')), + scope TEXT NOT NULL CHECK(scope IN ('trip', 'user', 'admin')), + target INTEGER NOT NULL, + sender_id INTEGER REFERENCES users(id) ON DELETE SET NULL, + recipient_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, + title_key TEXT NOT NULL, + title_params TEXT DEFAULT '{}', + text_key TEXT NOT NULL, + text_params TEXT DEFAULT '{}', + positive_text_key TEXT, + negative_text_key TEXT, + positive_callback TEXT, + negative_callback TEXT, + response TEXT CHECK(response IN ('positive', 'negative')), + navigate_text_key TEXT, + navigate_target TEXT, + is_read INTEGER DEFAULT 0, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP + );`); return { db: tmp }; }); vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); -const { prefs, inapp, channels } = vi.hoisted(() => ({ +// The preference matrix and the channel transports stay plain services/* +// modules after the fold — mock them; the in-app store is real SQL now. +const { prefs, channels } = vi.hoisted(() => ({ prefs: { getPreferencesMatrix: vi.fn(), setPreferences: vi.fn() }, - inapp: { - getNotifications: vi.fn(), getUnreadCount: vi.fn(), markRead: vi.fn(), markUnread: vi.fn(), - markAllRead: vi.fn(), deleteNotification: vi.fn(), deleteAll: vi.fn(), respondToBoolean: vi.fn(), - }, channels: { testSmtp: vi.fn(), testWebhook: vi.fn(), testNtfy: vi.fn(), getUserWebhookUrl: vi.fn(), getAdminWebhookUrl: vi.fn(), @@ -36,18 +60,29 @@ const { prefs, inapp, channels } = vi.hoisted(() => ({ }, })); vi.mock('../../src/services/notificationPreferencesService', () => prefs); -vi.mock('../../src/services/inAppNotifications', () => inapp); vi.mock('../../src/services/notifications', () => channels); +import { DatabaseModule } from '../../src/nest/database/database.module'; +import { RealtimeModule } from '../../src/nest/realtime/realtime.module'; import { NotificationsModule } from '../../src/nest/notifications/notifications.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +function seedNotification(recipientId: number, overrides: { is_read?: number } = {}): number { + const r = db.prepare( + `INSERT INTO notifications (type, scope, target, recipient_id, title_key, text_key, is_read) + VALUES ('simple', 'user', ?, ?, 'notif.test.title', 'notif.test.text', ?)` + ).run(recipientId, recipientId, overrides.is_read ?? 0); + return r.lastInsertRowid as number; +} + describe('Notifications e2e (real auth guard + temp SQLite)', () => { let server: Server; let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [NotificationsModule] }).compile(); + const moduleRef = await Test.createTestingModule({ + imports: [DatabaseModule, RealtimeModule, NotificationsModule], + }).compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); @@ -61,8 +96,6 @@ describe('Notifications e2e (real auth guard + temp SQLite)', () => { app = await build(); server = app.getHttpServer(); prefs.getPreferencesMatrix.mockReturnValue({ preferences: {}, available_channels: {}, event_types: [], implemented_combos: {} }); - inapp.getUnreadCount.mockReturnValue(2); - inapp.deleteAll.mockReturnValue(4); channels.testSmtp.mockResolvedValue({ success: true }); }); @@ -94,18 +127,27 @@ describe('Notifications e2e (real auth guard + temp SQLite)', () => { expect(res.body).toEqual({ success: true }); }); - it('200 unread-count', async () => { + it('200 unread-count from the real notifications table', async () => { + db.prepare('DELETE FROM notifications').run(); + seedNotification(2); + seedNotification(2); + seedNotification(2, { is_read: 1 }); const res = await request(server).get('/api/notifications/in-app/unread-count').set('Cookie', sessionCookie(2)); expect(res.status).toBe(200); expect(res.body).toEqual({ count: 2 }); }); - it('DELETE /in-app/all hits deleteAll, not deleteNotification', async () => { + it('DELETE /in-app/all hits deleteAll, not the /:id handler', async () => { + db.prepare('DELETE FROM notifications').run(); + for (let i = 0; i < 4; i++) seedNotification(2); + const other = seedNotification(1); const res = await request(server).delete('/api/notifications/in-app/all').set('Cookie', sessionCookie(2)); expect(res.status).toBe(200); expect(res.body).toEqual({ success: true, count: 4 }); - expect(inapp.deleteAll).toHaveBeenCalledWith(2); - expect(inapp.deleteNotification).not.toHaveBeenCalled(); + // Only user 2's rows are gone — the static route deleted per-recipient, it + // did not fall through to the /:id param handler (which would 400 on 'all'). + const rows = db.prepare('SELECT id FROM notifications').all() as { id: number }[]; + expect(rows.map(r => r.id)).toEqual([other]); }); it('400 on a non-numeric in-app id', async () => { diff --git a/server/tests/e2e/packing.e2e.test.ts b/server/tests/e2e/packing.e2e.test.ts index 3c267355a..43ec296c1 100644 --- a/server/tests/e2e/packing.e2e.test.ts +++ b/server/tests/e2e/packing.e2e.test.ts @@ -110,7 +110,7 @@ vi.mock('../../src/db/database', () => ({ reinitialize: () => {}, })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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 f412976ae..4c11661a2 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -38,7 +38,7 @@ vi.mock('../../src/db/database', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); const { notificationSend } = vi.hoisted(() => ({ notificationSend: vi.fn().mockResolvedValue(undefined) })); -vi.mock('../../src/services/notificationService', () => ({ send: notificationSend })); +vi.mock('../../src/nest/notifications/notifications.bridge', () => ({ send: notificationSend })); import { PermissionsService } from '../../src/nest/permissions/permissions.service'; diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index daa59fa9e..8fccce0a1 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -104,7 +104,7 @@ 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/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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 42b07f653..5b161009f 100644 --- a/server/tests/e2e/vacay.e2e.test.ts +++ b/server/tests/e2e/vacay.e2e.test.ts @@ -69,7 +69,7 @@ const { db } = vi.hoisted(() => { vi.mock('../../src/db/database', () => ({ db, closeDb: () => {}, reinitialize: () => {} })); vi.mock('../../src/websocket', () => ({ broadcastToUser: vi.fn() })); -vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 82ec7d8b2..f79c880ae 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -22,6 +22,8 @@ import { DaysMcp } from '../../src/nest/days/days.mcp'; import { DaysService } from '../../src/nest/days/days.service'; import { MapsMcp } from '../../src/nest/maps/maps.mcp'; import { MapsService } from '../../src/nest/maps/maps.service'; +import { NotificationsMcp } from '../../src/nest/notifications/notifications.mcp'; +import { NotificationsService } from '../../src/nest/notifications/notifications.service'; import { PackingMcp } from '../../src/nest/packing/packing.mcp'; import { PackingService } from '../../src/nest/packing/packing.service'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; @@ -100,6 +102,7 @@ export function createMcpTestRegistry(): McpRegistry { new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService), dbService, authService), new TransitMcp(new TransitService(), daysService, reservationsService, dbService, authService), new AtlasMcp(new AtlasService(dbService)), + new NotificationsMcp(new NotificationsService(dbService, realtimeService), authService), ], { accessPolicy: trekMcpAccessPolicy, validateAccess: trekMcpValidateAccess }, ); diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 45f6ff020..5acd8c994 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -22,6 +22,7 @@ import { TripsService } from '../../src/nest/trips/trips.service'; import { PlacesService } from '../../src/nest/places/places.service'; import { CollectionsService } from '../../src/nest/collections/collections.service'; import { AtlasService } from '../../src/nest/atlas/atlas.service'; +import { NotificationsService } from '../../src/nest/notifications/notifications.service'; import { MapsService } from '../../src/nest/maps/maps.service'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { AuditService } from '../../src/nest/audit/audit.service'; @@ -72,6 +73,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto places, new CollectionsService(dbs, permissions, realtime), new AtlasService(dbs), + new NotificationsService(dbs, realtime), ); } diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index 578bdf023..640e173d5 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -74,10 +74,10 @@ beforeAll(async () => { nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); if (!fs.existsSync(uploadsDir)) fs.mkdirSync(uploadsDir, { recursive: true }); - // Warm the notificationService module: notifyCollab does a fire-and-forget + // 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/services/notificationService'); + 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 a3cf3a9df..ab673fca3 100644 --- a/server/tests/integration/packing.test.ts +++ b/server/tests/integration/packing.test.ts @@ -58,10 +58,10 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notificationService module: notifyTagged does a fire-and-forget + // 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/services/notificationService'); + 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 17d24c3c0..4726581e2 100644 --- a/server/tests/integration/trips.test.ts +++ b/server/tests/integration/trips.test.ts @@ -67,10 +67,10 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notificationService module: notifyInvite does a fire-and-forget + // 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/services/notificationService'); + 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 42fde5593..c66424d2e 100644 --- a/server/tests/integration/vacay.test.ts +++ b/server/tests/integration/vacay.test.ts @@ -75,10 +75,10 @@ beforeAll(async () => { runMigrations(testDb); nestApp = await buildApp(); app = nestApp.getHttpAdapter().getInstance(); - // Warm the notificationService module: sendInvite/shareCalendar do a + // 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/services/notificationService'); + 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 1e0ec358d..51d2b1dae 100644 --- a/server/tests/unit/mcp/tools-collections.test.ts +++ b/server/tests/unit/mcp/tools-collections.test.ts @@ -39,7 +39,7 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser })); const { notifSend } = vi.hoisted(() => ({ notifSend: vi.fn().mockResolvedValue(undefined) })); -vi.mock('../../../src/services/notificationService', () => ({ send: notifSend })); +vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifSend })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -70,10 +70,10 @@ 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 notificationService module: sendInvite does a fire-and-forget + // 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/services/notificationService'); + await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/mcp/tools-notifications.test.ts b/server/tests/unit/mcp/tools-notifications.test.ts index bf44b3d84..23c400ce2 100644 --- a/server/tests/unit/mcp/tools-notifications.test.ts +++ b/server/tests/unit/mcp/tools-notifications.test.ts @@ -1,9 +1,13 @@ /** - * Unit tests for MCP notification tools: + * Unit tests for the notifications MCP surface (NotificationsMcp, + * DI-discovered since the notifications fold): * list_notifications, get_unread_notification_count, mark_notification_read, - * mark_notification_unread, mark_all_notifications_read, delete_notification, - * delete_all_notifications. + * mark_notification_unread, mark_all_notifications_read. * Also covers the resource trek://notifications/in-app. + * + * All of it attaches via the nest-mcp registry inside registerTools, so every + * harness here keeps withTools on (the resource is NOT registered by the + * legacy registerResources fan-out anymore). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -80,7 +84,9 @@ async function withHarness(userId: number, fn: (h: McpHarness) => Promise) } async function withResourceHarness(userId: number, fn: (h: McpHarness) => Promise) { - const h = await createMcpHarness({ userId, withTools: false, withResources: true }); + // The in-app resource attaches via the nest-mcp registry (withTools), not + // the legacy registerResources fan-out. + const h = await createMcpHarness({ userId, withTools: true, withResources: false }); try { await fn(h); } finally { await h.cleanup(); } } diff --git a/server/tests/unit/mcp/tools-vacay.test.ts b/server/tests/unit/mcp/tools-vacay.test.ts index fac7bedba..6ba5a970b 100644 --- a/server/tests/unit/mcp/tools-vacay.test.ts +++ b/server/tests/unit/mcp/tools-vacay.test.ts @@ -48,7 +48,7 @@ vi.mock('../../../src/nest/addons/addons.bridge', () => ({ })); // share_vacay_calendar fires a user notification after inserting; stub it out -vi.mock('../../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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/admin.controller.test.ts b/server/tests/unit/nest/admin.controller.test.ts index d2c494906..d8274d632 100644 --- a/server/tests/unit/nest/admin.controller.test.ts +++ b/server/tests/unit/nest/admin.controller.test.ts @@ -4,13 +4,12 @@ import type { Request } from 'express'; vi.mock('../../../src/nest/audit/client-ip', () => ({ getClientIp: vi.fn(() => '1.2.3.4') })); vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ LOG_LEVEL: 'error', logInfo: vi.fn(), logDebug: vi.fn(), logError: vi.fn(), logWarn: vi.fn() })); -vi.mock('../../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { AdminController } from '../../../src/nest/admin/admin.controller'; import type { AdminService } from '../../../src/nest/admin/admin.service'; import type { PluginRuntimeService } from '../../../src/nest/plugins/plugin-runtime.service'; import type { AuditService } from '../../../src/nest/audit/audit.service'; -import { send as sendNotification } from '../../../src/services/notificationService'; +import type { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import type { User } from '../../../src/types'; const user = { id: 1, role: 'admin', email: 'admin@example.test' } as User; @@ -24,8 +23,12 @@ function svc(o: Partial = {}): AdminService { // wrapper keeps the historical construction sites positional. const writeAudit = vi.fn(); const audit = { writeAudit } as unknown as AuditService; +// Notifications are a constructor-injected stub since the notifications fold +// (same behavior as the old services/notificationService path mock). +const sendNotification = vi.fn().mockResolvedValue(undefined); +const notifications = { send: sendNotification } as unknown as NotificationsService; const adminCtl = (s: AdminService, rt?: PluginRuntimeService) => - new AdminController(s, rt as unknown as PluginRuntimeService, audit); + new AdminController(s, rt as unknown as PluginRuntimeService, audit, notifications); function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { if (err instanceof NotFoundException) return { status: 404, body: err.getResponse() }; diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 805b8ec0f..3e5de7fbe 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -42,7 +42,7 @@ vi.mock('../../../src/config', () => ({ })); vi.mock('../../../src/websocket', () => ({ broadcastToUser, broadcast: vi.fn() })); const notifSend = vi.fn().mockResolvedValue(undefined); -vi.mock('../../../src/services/notificationService', () => ({ send: notifSend })); +vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: notifSend })); import fs from 'fs'; import path from 'path'; @@ -78,10 +78,10 @@ function clearCollections() { beforeAll(async () => { createTables(testDb); runMigrations(testDb); - // Warm the notificationService module: sendInvite does a fire-and-forget + // 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/services/notificationService'); + await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/services/inAppNotificationPrefs.test.ts b/server/tests/unit/nest/notifications.inapp-prefs.test.ts similarity index 68% rename from server/tests/unit/services/inAppNotificationPrefs.test.ts rename to server/tests/unit/nest/notifications.inapp-prefs.test.ts index e713eca27..1c0764d4d 100644 --- a/server/tests/unit/services/inAppNotificationPrefs.test.ts +++ b/server/tests/unit/nest/notifications.inapp-prefs.test.ts @@ -1,6 +1,9 @@ /** - * Unit tests for in-app notification preference filtering in createNotification(). - * Covers INOTIF-001 to INOTIF-004. + * Unit tests for in-app notification preference filtering in + * NotificationsService.createNotification() and the respondToBoolean flow — + * INOTIF-001 to INOTIF-004 (moved 1:1 from the legacy + * tests/unit/services/inAppNotificationPrefs.test.ts when the in-app store + * SQL folded into nest/notifications). */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; @@ -30,13 +33,23 @@ vi.mock('../../../src/config', () => ({ // Mock WebSocket broadcast — must use vi.hoisted() so broadcastMock is available // when the vi.mock factory is evaluated (factories are hoisted before const declarations) const { broadcastMock } = vi.hoisted(() => ({ broadcastMock: vi.fn() })); -vi.mock('../../../src/websocket', () => ({ broadcastToUser: broadcastMock })); +// RealtimeService imports both names from src/websocket, so the mock must +// export both even though only broadcastToUser is asserted here. +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: broadcastMock })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, disableNotificationPref } from '../../helpers/factories'; -import { createNotification, createNotificationForRecipient, respondToBoolean } from '../../../src/services/inAppNotifications'; +import { registerAction } from '../../../src/services/inAppNotificationActions'; +import { DatabaseService } from '../../../src/nest/database/database.service'; +import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { NotificationsService } from '../../../src/nest/notifications/notifications.service'; + +const notifications = new NotificationsService(new DatabaseService(testDb), new RealtimeService()); +const createNotification = notifications.createNotification.bind(notifications); +const createNotificationForRecipient = notifications.createNotificationForRecipient.bind(notifications); +const respondToBoolean = notifications.respond.bind(notifications); beforeAll(() => { createTables(testDb); @@ -291,4 +304,97 @@ describe('respondToBoolean', () => { expect(result.success).toBe(false); expect(result.error).toMatch(/not found/i); }); + + // ── Regression pins from the post-fold fix(server) commit ────────────────── + + it('INOTIF-011 — a concurrent double-submit executes the action handler exactly once (CAS before handler)', async () => { + const { user } = createUser(testDb); + const calls: string[] = []; + // Slow handler: with the legacy handler-before-CAS order, both submits + // passed the response-IS-NULL pre-check while the first awaited here and + // the action ran twice. + registerAction('slow_approve', async () => { + calls.push('run'); + await new Promise((resolve) => setImmediate(resolve)); + }); + const id = 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', 'user', ?, NULL, ?, 'notif.test.title', '{}', 'notif.test.text', '{}', + 'notif.action.accept', 'notif.action.decline', + '{"action":"slow_approve","payload":{}}', '{"action":"slow_approve","payload":{}}' + ) + `).run(user.id, user.id).lastInsertRowid as number; + + const [first, second] = await Promise.all([ + respondToBoolean(id, user.id, 'positive'), + respondToBoolean(id, user.id, 'negative'), + ]); + + expect(calls).toHaveLength(1); + const outcomes = [first, second]; + expect(outcomes.filter((r) => r.success)).toHaveLength(1); + expect(outcomes.find((r) => !r.success)?.error).toMatch(/already responded/i); + }); + + it('INOTIF-012 — a handler failure releases the claim: error returned, notification stays unresponded, retry succeeds', async () => { + const { user } = createUser(testDb); + let shouldFail = true; + registerAction('flaky_approve', async () => { + if (shouldFail) throw new Error('downstream unavailable'); + }); + const id = 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', 'user', ?, NULL, ?, 'notif.test.title', '{}', 'notif.test.text', '{}', + 'notif.action.accept', 'notif.action.decline', + '{"action":"flaky_approve","payload":{}}', '{"action":"flaky_approve","payload":{}}' + ) + `).run(user.id, user.id).lastInsertRowid as number; + + const failed = await respondToBoolean(id, user.id, 'positive'); + expect(failed).toEqual({ success: false, error: 'downstream unavailable' }); + const row = testDb.prepare('SELECT response, is_read FROM notifications WHERE id = ?').get(id) as { response: string | null; is_read: number }; + expect(row.response).toBeNull(); + expect(row.is_read).toBe(0); + + shouldFail = false; + const retried = await respondToBoolean(id, user.id, 'positive'); + expect(retried.success).toBe(true); + }); + + it('INOTIF-013 — respond result + broadcast carry an ISO-UTC created_at (toUtcIso, #1149 parity across paths)', async () => { + const { user } = createUser(testDb); + const id = insertBooleanNotification(user.id); + + const result = await respondToBoolean(id, user.id, 'positive'); + + expect(result.notification?.created_at).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/); + const broadcastPayload = broadcastMock.mock.calls + .map((call) => call[1] as { type: string; notification: { created_at: string } }) + .find((p) => p.type === 'notification:updated'); + expect(broadcastPayload?.notification.created_at).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/); + }); + + it('INOTIF-014 — createNotification broadcast carries an ISO-UTC created_at (was the un-patched #1149 path)', () => { + const { user } = createUser(testDb); + + createNotification({ + type: 'simple', + scope: 'user', + target: user.id, + sender_id: null, + title_key: 'notif.test.title', + text_key: 'notif.test.text', + }); + + const broadcastPayload = broadcastMock.mock.calls + .map((call) => call[1] as { type: string; notification: { created_at: string } }) + .find((p) => p.type === 'notification:new'); + expect(broadcastPayload?.notification.created_at).toMatch(/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z$/); + }); }); diff --git a/server/tests/unit/services/notificationService.test.ts b/server/tests/unit/nest/notifications.service.test.ts similarity index 90% rename from server/tests/unit/services/notificationService.test.ts rename to server/tests/unit/nest/notifications.service.test.ts index 9efb7431f..a19e57f19 100644 --- a/server/tests/unit/services/notificationService.test.ts +++ b/server/tests/unit/nest/notifications.service.test.ts @@ -1,6 +1,11 @@ /** - * Unit tests for the unified notificationService.send(). - * Covers NSVC-001 to NSVC-014. + * Unit tests for the DI-native NotificationsService.send() — NSVC-001 through + * NSVC-019, NTFY-SVCB-*, NSVC-PLUG-001..007 (moved 1:1 from the legacy + * tests/unit/services/notificationService.test.ts when the send dispatcher + + * in-app SQL folded into nest/notifications), plus the NSVC-020 bridge + * delegation pin. Uses a real in-memory SQLite DB so the SQL is exercised + * faithfully; email/webhook/ntfy transports are mocked at the nodemailer/fetch + * boundary. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; @@ -32,10 +37,18 @@ vi.mock('../../../src/services/apiKeyCrypto', () => ({ encrypt_api_key: (v: string) => v, })); -const { sendMailMock, fetchMock, broadcastMock } = vi.hoisted(() => ({ +const { sendMailMock, fetchMock, broadcastMock, logErrorMock } = vi.hoisted(() => ({ sendMailMock: vi.fn().mockResolvedValue({ accepted: ['test@test.com'] }), fetchMock: vi.fn(), broadcastMock: vi.fn(), + logErrorMock: vi.fn(), +})); +vi.mock('../../../src/nest/audit/audit-log.logger', () => ({ + LOG_LEVEL: 'error', + logInfo: vi.fn(), + logDebug: vi.fn(), + logError: logErrorMock, + logWarn: vi.fn(), })); vi.mock('nodemailer', () => ({ @@ -48,7 +61,9 @@ vi.mock('nodemailer', () => ({ })); vi.stubGlobal('fetch', fetchMock); -vi.mock('../../../src/websocket', () => ({ broadcastToUser: broadcastMock })); +// RealtimeService imports both names from src/websocket, so the mock must +// export both even though only broadcastToUser is asserted here. +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: broadcastMock })); vi.mock('../../../src/utils/ssrfGuard', () => ({ checkSsrf: vi.fn(async () => ({ allowed: true, isPrivate: false, resolvedIp: '1.2.3.4' })), createPinnedDispatcher: vi.fn(() => ({})), @@ -58,9 +73,15 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, setAppSetting, setNotificationChannels, disableNotificationPref } from '../../helpers/factories'; -import { send } from '../../../src/services/notificationService'; +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 { setPluginChannelSource, type ExternalChannel } from '../../../src/services/notifications/channelRegistry'; +const notifications = new NotificationsService(new DatabaseService(testDb), new RealtimeService()); +const send = (payload: NotificationPayload) => notifications.send(payload); + // ── Helpers ──────────────────────────────────────────────────────────────── function setSmtp(): void { @@ -480,6 +501,7 @@ describe('send() — channel failure resilience', () => { expect(sendMailMock).toHaveBeenCalledTimes(1); expect(countAllNotifications()).toBe(1); }); + }); // ── Ntfy dispatch ───────────────────────────────────────────────────────────── @@ -675,4 +697,36 @@ describe('send() — plugin notification channels', () => { await send({ event: 'booking_change', actorId: null, scope: 'user', targetId: user.id, params: { trip: 'Rome', actor: 'Alice', tripId: '1' } }); expect(sendSpy).toHaveBeenCalledTimes(1); }); + + it('NSVC-021 — a rejected channel dispatch logs the unwrapped Error message (fix-commit pin)', async () => { + const { user } = createUser(testDb); + installPluginChannel({ sendToUser: vi.fn().mockRejectedValue(new Error('gotify is down')) }); + setNotificationChannels(testDb, 'plugin:gotify'); + logErrorMock.mockClear(); + + await send({ event: 'trip_invite', actorId: null, scope: 'user', targetId: user.id, params: { trip: 'Rome', actor: 'Alice', invitee: 'Bob', tripId: '1' } }); + + const dispatchLog = logErrorMock.mock.calls.map(([msg]) => String(msg)).find(m => m.includes('channel dispatch failed')); + // The legacy per-recipient log interpolated the raw reason ("Error: gotify + // is down"); the admin path always unwrapped — now both do. + expect(dispatchLog).toContain(': gotify is down'); + expect(dispatchLog).not.toContain('Error: gotify is down'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Bridge delegation (notifications.bridge.ts — outside-container entry point) +// ───────────────────────────────────────────────────────────────────────────── + +describe('notifications.bridge', () => { + 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' } }); + + expect(countAllNotifications()).toBe(1); + expect(broadcastMock).toHaveBeenCalledTimes(1); + expect(getInAppNotifications(user.id)[0].navigate_target).toBe('/trips/7'); + }); }); diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index 152823111..cb1aa85ab 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -49,7 +49,7 @@ const checkPermission = vi.fn(() => true); const permissionsStub = { checkPermission } as unknown as PermissionsService; const { send } = vi.hoisted(() => ({ send: vi.fn(() => Promise.resolve()) })); -vi.mock('../../../src/services/notificationService', () => ({ send })); +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/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 645ef6675..467f450d6 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -42,7 +42,7 @@ 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/services/notificationService', () => notif); +vi.mock('../../../src/nest/notifications/notifications.bridge', () => notif); 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 e2524d429..54803c559 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -44,7 +44,7 @@ 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/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +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 d12f4a26b..50a44ba36 100644 --- a/server/tests/unit/nest/vacay.service.test.ts +++ b/server/tests/unit/nest/vacay.service.test.ts @@ -26,7 +26,7 @@ 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/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); +vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -53,7 +53,7 @@ beforeAll(async () => { // 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/services/notificationService'); + await import('../../../src/nest/notifications/notifications.bridge'); }); beforeEach(() => { diff --git a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts index 17d55280b..64f3db383 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -5,7 +5,7 @@ * host, and trip broadcasts are force-namespaced to plugin:{id}:{event}. * DI-native domains (budget/exchange-rates/reservations/tags/categories/todo/ * packing/day-notes/days/assignments/oauth/llm-config/files/collab/vacay/ - * permissions/trips/places/collections/atlas) + * permissions/trips/places/collections/atlas/notifications) * are constructor-injected stubs; legacy services/* domains stay path-mocked * until their own DI migration lands. */ @@ -208,8 +208,10 @@ const collabStub = { vi.mock('../../../src/services/tripMembership', () => ({ joinTripAsMember: vi.fn((tripId: number, userId: number) => ({ joined: userId !== 5, tripId })), // owner add = no-op })); -const { notifySend } = vi.hoisted(() => ({ notifySend: vi.fn(async () => undefined) })); -vi.mock('../../../src/services/notificationService', () => ({ send: notifySend })); +// Notifications are a constructor-injected stub since the notifications fold +// (same behavior as the old services/notificationService path mock). +const notifySend = vi.fn(async () => undefined); +const notificationsStub = { send: notifySend } as unknown as NotificationsService; // userId 7 = no provider configured; everyone else resolves to a stub config. // Constructor-injected stub since the resolver became DI-native (settings migration). const llmConfigStub = { @@ -324,6 +326,7 @@ import type { PermissionsService } from '../../../src/nest/permissions/permissio import type { PlacesService } from '../../../src/nest/places/places.service'; import type { CollectionsService } from '../../../src/nest/collections/collections.service'; import type { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import type { NotificationsService } from '../../../src/nest/notifications/notifications.service'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { NotFoundError, ValidationError } from '../../../src/nest/trips/trips.service'; @@ -346,7 +349,7 @@ const tripsStub = { list: () => [{ id: 1 }], removeMember: vi.fn(), } as unknown as import('../../../src/nest/trips/trips.service').TripsService; -const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub, exchangeRatesStub, addonsStub, new RealtimeService(), tripsStub, placesStub, collectionsStub, atlasStub); +const factory = new PluginHostDepsFactory(budgetStub, reservationsStub, tagsStub, categoriesStub, todoStub, packingStub, oauthStub, dayNotesStub, assignmentsStub, llmConfigStub, new DatabaseService(mockDb), filesStub, collabStub, vacayStub, daysStub, permissionsStub, exchangeRatesStub, addonsStub, new RealtimeService(), tripsStub, placesStub, collectionsStub, atlasStub, notificationsStub); const stubRouter: PluginCallRouter = { callPlugin: async () => undefined, emitPluginEvent: () => {} }; const createRealRpcHost = (id: string, granted: ReadonlySet, router: PluginCallRouter = stubRouter) => factory.create(id, granted, router); diff --git a/server/tests/unit/services/channelOverride.test.ts b/server/tests/unit/services/channelOverride.test.ts index 5044b0792..ba5896be3 100644 --- a/server/tests/unit/services/channelOverride.test.ts +++ b/server/tests/unit/services/channelOverride.test.ts @@ -20,14 +20,16 @@ vi.mock('../../../src/services/apiKeyCrypto', () => ({ decrypt_api_key: (v: stri const { sendMailMock } = vi.hoisted(() => ({ sendMailMock: vi.fn().mockResolvedValue({ accepted: ['a@b.c'] }) })); vi.mock('nodemailer', () => ({ default: { createTransport: vi.fn(() => ({ sendMail: sendMailMock, verify: vi.fn() })) } })); vi.stubGlobal('fetch', vi.fn()); -vi.mock('../../../src/websocket', () => ({ broadcastToUser: vi.fn() })); +vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); vi.mock('../../../src/utils/ssrfGuard', () => ({ checkSsrf: vi.fn(async () => ({ allowed: true, resolvedIp: '1.2.3.4' })), createPinnedDispatcher: vi.fn(() => ({})) })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createAdmin, setAppSetting, setNotificationChannels } from '../../helpers/factories'; -import { send } from '../../../src/services/notificationService'; +// 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'; import { getPreferencesMatrix } from '../../../src/services/notificationPreferencesService'; import { setPluginChannelSource, diff --git a/server/tests/unit/services/memoriesUnified.test.ts b/server/tests/unit/services/memoriesUnified.test.ts index c0fb99a91..ed74f37f9 100644 --- a/server/tests/unit/services/memoriesUnified.test.ts +++ b/server/tests/unit/services/memoriesUnified.test.ts @@ -36,7 +36,7 @@ vi.mock('../../../src/config', () => ({ updateJwtSecret: () => {}, })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../../src/services/notificationService', () => ({ +vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send: vi.fn().mockResolvedValue(undefined), }));