From 939452f9d8d21b64497725952391230635d92f44 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:00:12 +0200 Subject: [PATCH] refactor(server): consolidate trip access and fold the query helpers into the container (#1824) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(server): route every trip-access check through DatabaseService Twelve Nest services carry the same verifyTripAccess/canEdit pair. Six already asked DatabaseService.canAccessTrip; the other six went through services/tripAccess, a nine-line re-export of the identical function. Same query, same result, two ways to reach it. All twelve now take the injected route, so services/tripAccess is gone. The bigger duplication was in the types. Thirteen files declared their own `Trip` for the same return value, in three different shapes: NonNullable> 6x { user_id: number } 4x TripAccess 3x The narrow one is why assignments, days, places and accommodations all wrote `as Trip | null | undefined` on the call — the cast existed to shrink TripAccess down to a local type that was never meant to differ from it. All thirteen now say TripAccess, and the four casts are gone with no other change. middleware/tripAccess.ts goes too: it was already dead in production — nothing under src/ imported requireTripAccess or requireTripOwner, only comments in the services that had absorbed them. Its test went with it. * refactor(server): fold the query helpers and trip joins into the container Two more legacy files whose only reason for staying outside the container was that they predated it. services/queryHelpers.ts held five exports doing two different jobs. Three are the batch loaders that keep the list endpoints off N+1 queries — they hit the database and are now QueryHelpersService over the injected connection, imported by assignments, days, places and share. The other two, formatAssignmentWithPlace and ratingAggregate, touch no database at all; they sat next to the loaders only because the loaders were free functions too. Those stay free functions, in common/rowShape.ts. The README note on the assignments service claimed the loaders had to stay in services/ because the day, share and place services were unmigrated. All three have since moved, so the reason is gone. services/tripMembership.ts becomes TripMembershipService in its own module. It could have folded into auth or trip-invite, but both call it — as do oidc and the plugin RPC host — and putting it in either would have made AuthModule and TripInviteModule import each other. Neither module is @Global, matching PermissionsModule: e2e TestingModules resolve them transitively through each consumer's explicit import. Test seams changed shape accordingly. Two path mocks of services/tripMembership became injected stubs; the trip-invite e2e overrides the provider in the container instead. The oidc test keeps the real implementation, which is what OIDC-SVC-045 was always asserting. ratingAggregate moved out of the ungated services/ tree into the coverage-gated one, so it gets the direct tests it never had — as does the tags/participants pass-through, which two long-dead fixture variables in the moved test file had been waiting for. Also drops four stale comments that outlived their subject: the requireTripAccess middleware they pointed at was deleted in the previous commit. --- server/src/middleware/tripAccess.ts | 37 ----- server/src/nest/admin/admin.bridge.ts | 3 +- .../nest/assignments/assignments.bridge.ts | 3 +- .../assignments/assignments.controller.ts | 2 +- .../nest/assignments/assignments.module.ts | 3 +- .../nest/assignments/assignments.service.ts | 22 +-- server/src/nest/auth/auth.bridge.ts | 2 + server/src/nest/auth/auth.module.ts | 3 +- server/src/nest/auth/auth.service.ts | 5 +- .../booking-import/booking-import.service.ts | 7 +- server/src/nest/budget/budget.service.ts | 7 +- server/src/nest/collab/collab.service.ts | 7 +- server/src/nest/common/rowShape.ts | 63 ++++++++ server/src/nest/days/day-notes.service.ts | 5 +- server/src/nest/days/days.bridge.ts | 3 +- server/src/nest/days/days.module.ts | 3 +- server/src/nest/days/days.service.ts | 18 ++- .../airtrail-import.controller.ts | 9 +- server/src/nest/oidc/oidc.module.ts | 3 +- server/src/nest/oidc/oidc.service.ts | 5 +- server/src/nest/packing/packing.service.ts | 7 +- server/src/nest/places/places.module.ts | 3 +- server/src/nest/places/places.service.ts | 16 +- .../plugins/host/plugin-host-deps.factory.ts | 9 +- server/src/nest/plugins/plugins.module.ts | 3 +- .../query-helpers/query-helpers.module.ts | 12 ++ .../query-helpers/query-helpers.service.ts | 94 ++++++++++++ .../reservations/accommodations.service.ts | 8 +- .../nest/reservations/reservations.service.ts | 7 +- server/src/nest/share/share.module.ts | 3 +- server/src/nest/share/share.service.ts | 5 +- server/src/nest/todo/todo.service.ts | 7 +- .../nest/trip-invite/trip-invite.module.ts | 3 +- .../nest/trip-invite/trip-invite.service.ts | 5 +- .../trip-membership/trip-membership.module.ts | 12 ++ .../trip-membership.service.ts | 39 +++++ server/src/nest/trips/trips.bridge.ts | 5 +- server/src/services/queryHelpers.ts | 133 ----------------- server/src/services/tripAccess.ts | 9 -- server/src/services/tripMembership.ts | 29 ---- server/tests/e2e/trip-invite.e2e.test.ts | 10 +- server/tests/helpers/mcp-test-controllers.ts | 10 +- server/tests/helpers/plugin-host.ts | 8 +- server/tests/integration/dayReorder.test.ts | 3 +- .../tests/unit/middleware/tripAccess.test.ts | 137 ------------------ .../unit/nest/assignments.service.test.ts | 3 +- server/tests/unit/nest/auth.service.test.ts | 10 +- .../booking-import.service.test.ts | 8 +- .../tests/unit/nest/budget.service.db.test.ts | 3 +- server/tests/unit/nest/budget.service.test.ts | 2 +- .../common/rowShape.test.ts} | 32 +++- server/tests/unit/nest/days.service.test.ts | 3 +- server/tests/unit/nest/oidc.service.test.ts | 11 +- server/tests/unit/nest/places.service.test.ts | 5 +- server/tests/unit/nest/share.service.test.ts | 3 +- .../trip-membership.service.test.ts} | 18 ++- server/tests/unit/nest/trips.service.test.ts | 5 +- .../plugins/plugin-host-deps.factory.test.ts | 8 +- .../unit/services/conflictUpdate.test.ts | 3 +- 59 files changed, 424 insertions(+), 477 deletions(-) delete mode 100644 server/src/middleware/tripAccess.ts create mode 100644 server/src/nest/common/rowShape.ts create mode 100644 server/src/nest/query-helpers/query-helpers.module.ts create mode 100644 server/src/nest/query-helpers/query-helpers.service.ts create mode 100644 server/src/nest/trip-membership/trip-membership.module.ts create mode 100644 server/src/nest/trip-membership/trip-membership.service.ts delete mode 100644 server/src/services/queryHelpers.ts delete mode 100644 server/src/services/tripAccess.ts delete mode 100644 server/src/services/tripMembership.ts delete mode 100644 server/tests/unit/middleware/tripAccess.test.ts rename server/tests/unit/{services/queryHelpers.test.ts => nest/common/rowShape.test.ts} (72%) rename server/tests/unit/{services/tripMembership.test.ts => nest/trip-membership/trip-membership.service.test.ts} (75%) diff --git a/server/src/middleware/tripAccess.ts b/server/src/middleware/tripAccess.ts deleted file mode 100644 index 2c973f50d..000000000 --- a/server/src/middleware/tripAccess.ts +++ /dev/null @@ -1,37 +0,0 @@ -import { Request, Response, NextFunction } from 'express'; -import { canAccessTrip, isOwner } from '../db/database'; -import { AuthRequest } from '../types'; - -/** Middleware: verifies the authenticated user is an owner or member of the trip, then attaches trip to req. */ -function requireTripAccess(req: Request, res: Response, next: NextFunction): void { - const authReq = req as AuthRequest; - const tripId = req.params.tripId || req.params.id; - if (!tripId) { - res.status(400).json({ error: 'Trip ID required' }); - return; - } - const trip = canAccessTrip(Number(tripId), authReq.user.id); - if (!trip) { - res.status(404).json({ error: 'Trip not found' }); - return; - } - authReq.trip = trip; - next(); -} - -/** Middleware: verifies the authenticated user is the trip owner (not just a member). */ -function requireTripOwner(req: Request, res: Response, next: NextFunction): void { - const authReq = req as AuthRequest; - const tripId = req.params.tripId || req.params.id; - if (!tripId) { - res.status(400).json({ error: 'Trip ID required' }); - return; - } - if (!isOwner(Number(tripId), authReq.user.id)) { - res.status(403).json({ error: 'Only the trip owner can do this' }); - return; - } - next(); -} - -export { requireTripAccess, requireTripOwner }; diff --git a/server/src/nest/admin/admin.bridge.ts b/server/src/nest/admin/admin.bridge.ts index 592f92d2b..d6c66965c 100644 --- a/server/src/nest/admin/admin.bridge.ts +++ b/server/src/nest/admin/admin.bridge.ts @@ -6,6 +6,7 @@ import { SettingsService } from '../settings/settings.service'; import { AuthService } from '../auth/auth.service'; import { PasskeyService } from '../auth/passkey.service'; import { AtlasService } from '../atlas/atlas.service'; +import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { PackingService } from '../packing/packing.service'; import { PermissionsService } from '../permissions/permissions.service'; import { NotificationsService } from '../notifications/notifications.service'; @@ -30,7 +31,7 @@ import { AdminService } from './admin.service'; const dbs = new DatabaseService(db); const realtime = new RealtimeService(); const permissions = new PermissionsService(dbs); -const auth = new AuthService(dbs, permissions, new AtlasService(dbs)); +const auth = new AuthService(dbs, permissions, new AtlasService(dbs), new TripMembershipService(dbs)); const admin = new AdminService( dbs, new SettingsService(dbs), diff --git a/server/src/nest/assignments/assignments.bridge.ts b/server/src/nest/assignments/assignments.bridge.ts index 165a1053f..d08074abb 100644 --- a/server/src/nest/assignments/assignments.bridge.ts +++ b/server/src/nest/assignments/assignments.bridge.ts @@ -3,6 +3,7 @@ import { DatabaseService } from '../database/database.service'; import { RealtimeService } from '../realtime/realtime.service'; import { AssignmentsService } from './assignments.service'; import { PermissionsService } from '../permissions/permissions.service'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; /** * Non-Nest entry point for the assignments domain — for code running OUTSIDE @@ -27,7 +28,7 @@ import { PermissionsService } from '../permissions/permissions.service'; */ let instance: AssignmentsService | undefined; function assignments(): AssignmentsService { - return (instance ??= new AssignmentsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService())); + return (instance ??= new AssignmentsService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), new QueryHelpersService(new DatabaseService(db)))); } export function createAssignment(dayId: string | number, placeId: string | number, notes: string | null) { diff --git a/server/src/nest/assignments/assignments.controller.ts b/server/src/nest/assignments/assignments.controller.ts index f003f2781..065ecca71 100644 --- a/server/src/nest/assignments/assignments.controller.ts +++ b/server/src/nest/assignments/assignments.controller.ts @@ -25,7 +25,7 @@ import { CurrentUser } from '../auth/current-user.decorator'; type Trip = NonNullable>; -/** Shared trip-access guard (mirrors requireTripAccess → 404 "Trip not found"). */ +/** Shared trip-access guard (no access is a 404 "Trip not found", never a 403). */ function requireTrip(svc: AssignmentsService, tripId: string, user: User): Trip { const trip = svc.verifyTripAccess(tripId, user.id); if (!trip) { diff --git a/server/src/nest/assignments/assignments.module.ts b/server/src/nest/assignments/assignments.module.ts index bdb0eb936..d645310b6 100644 --- a/server/src/nest/assignments/assignments.module.ts +++ b/server/src/nest/assignments/assignments.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { DaysModule } from '../days/days.module'; import { PermissionsModule } from '../permissions/permissions.module'; +import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { DayAssignmentsController, AssignmentOpsController } from './assignments.controller'; import { AssignmentsService } from './assignments.service'; import { AssignmentsMcp } from './assignments.mcp'; @@ -14,7 +15,7 @@ import { AuthModule } from '../auth/auth.module'; */ @Module({ // DaysModule: AssignmentsMcp injects DaysService for the target-day checks. - imports: [DaysModule, PermissionsModule, AuthModule], + imports: [DaysModule, PermissionsModule, QueryHelpersModule, AuthModule], controllers: [DayAssignmentsController, AssignmentOpsController], providers: [AssignmentsService, AssignmentsMcp], exports: [AssignmentsService], diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 609f2efd5..ab41ef5ea 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -1,13 +1,14 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from '../../services/queryHelpers'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; +import { formatAssignmentWithPlace } from '../common/rowShape'; import type { AssignmentRow, DayAssignment, User } from '../../types'; import { reconcileTripSkeletons } from '../../services/journeyService'; -type Trip = { user_id: number }; +type Trip = TripAccess; /** * Assignments domain service — owns the day-assignment SQL (relocated from the @@ -15,9 +16,9 @@ type Trip = { user_id: number }; * write runs in a transaction, moveAssignment derives the source day from the * row instead of trusting the caller, empty-string times clear like null, and * single-assignment reads embed the same compact tag projection the list path - * uses). Trip access mirrors the requireTripAccess middleware (canAccessTrip); - * mutations use 'day_edit'. The batch tag/participant loaders stay in - * services/queryHelpers (shared with the unmigrated day/share/place services). + * uses). Trip access rides DatabaseService.canAccessTrip; mutations use + * 'day_edit'. The batch tag/participant loaders are injected as + * QueryHelpersService (shared with the day, share and place services). * Non-Nest consumers (the legacy places/reservations MCP registrars) go * through assignments.bridge.ts; the plugin RPC host and the assignments MCP * tools inject this class. @@ -28,10 +29,11 @@ export class AssignmentsService { private readonly dbs: DatabaseService, private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, + private readonly queryHelpers: QueryHelpersService, ) {} verifyTripAccess(tripId: string | number, userId: number) { - return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; + return this.dbs.canAccessTrip(Number(tripId), userId); } canEdit(trip: Trip, user: User): boolean { @@ -70,7 +72,7 @@ export class AssignmentsService { // Same compact tag projection as listDayAssignments, so an assignment has // one wire shape regardless of which read path produced it. - const tags = loadTagsByPlaceIds([a.place_id], { compact: true })[a.place_id] || []; + const tags = this.queryHelpers.loadTagsByPlaceIds([a.place_id], { compact: true })[a.place_id] || []; const participants = this.dbs.all(` SELECT ap.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar @@ -139,10 +141,10 @@ export class AssignmentsService { `, dayId); const placeIds = [...new Set(assignments.map(a => a.place_id))]; - const tagsByPlaceId = loadTagsByPlaceIds(placeIds, { compact: true }); + const tagsByPlaceId = this.queryHelpers.loadTagsByPlaceIds(placeIds, { compact: true }); const assignmentIds = assignments.map(a => a.id); - const participantsByAssignment = loadParticipantsByAssignmentIds(assignmentIds); + const participantsByAssignment = this.queryHelpers.loadParticipantsByAssignmentIds(assignmentIds); return assignments.map(a => { return formatAssignmentWithPlace(a, tagsByPlaceId[a.place_id] || [], participantsByAssignment[a.id] || []); diff --git a/server/src/nest/auth/auth.bridge.ts b/server/src/nest/auth/auth.bridge.ts index f712d6cd6..ecf6f7d31 100644 --- a/server/src/nest/auth/auth.bridge.ts +++ b/server/src/nest/auth/auth.bridge.ts @@ -2,6 +2,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { AtlasService } from '../atlas/atlas.service'; +import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { AuthService } from './auth.service'; import { User } from '../../types'; @@ -32,6 +33,7 @@ const auth = new AuthService( new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new AtlasService(new DatabaseService(db)), + new TripMembershipService(new DatabaseService(db)), ); export { stripUserForClient } from './auth.helpers'; diff --git a/server/src/nest/auth/auth.module.ts b/server/src/nest/auth/auth.module.ts index 8e36c87b8..8a82cdf37 100644 --- a/server/src/nest/auth/auth.module.ts +++ b/server/src/nest/auth/auth.module.ts @@ -8,6 +8,7 @@ import { PasskeyService } from './passkey.service'; import { AuditModule } from '../audit/audit.module'; import { PermissionsModule } from '../permissions/permissions.module'; import { AtlasModule } from '../atlas/atlas.module'; +import { TripMembershipModule } from '../trip-membership/trip-membership.module'; /** * Auth module — public flows (login/register/reset/mfa-verify/logout) and the @@ -22,7 +23,7 @@ import { AtlasModule } from '../atlas/atlas.module'; * everything outside the container goes through auth.bridge.ts. */ @Module({ - imports: [RateLimitModule, AuditModule, PermissionsModule, AtlasModule], + imports: [RateLimitModule, AuditModule, PermissionsModule, AtlasModule, TripMembershipModule], controllers: [AuthPublicController, AuthController, PasskeyController], providers: [AuthService, PasskeyService], exports: [AuthService, PasskeyService], diff --git a/server/src/nest/auth/auth.service.ts b/server/src/nest/auth/auth.service.ts index 5e8a1bd62..4485bbc6b 100644 --- a/server/src/nest/auth/auth.service.ts +++ b/server/src/nest/auth/auth.service.ts @@ -29,7 +29,7 @@ import { verifyJwtAndLoadUser } from '../../middleware/auth'; import { User } from '../../types'; import { DEMO_EMAIL_PRIMARY, isDemoEmail } from '../common/demo'; import { avatarUrl } from '../common/avatarUrl'; -import { joinTripAsMember } from '../../services/tripMembership'; +import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { isPasskeyConfigured } from '../../services/webauthnConfig'; import { setAuthCookie, clearAuthCookie } from '../common/cookie'; import { sendPasswordResetEmail } from '../../services/notifications'; @@ -124,6 +124,7 @@ export class AuthService { private readonly db: DatabaseService, private readonly permissions: PermissionsService, private readonly atlas: AtlasService, + private readonly membership: TripMembershipService, ) {} // Cookie @@ -383,7 +384,7 @@ export class AuthService { // Trip-bound invite (#1402): auto-add the freshly registered user to the // trip. Idempotent + owner-safe; no-ops if the bound trip was since deleted. if (validInvite.trip_id) { - joinTripAsMember(Number(validInvite.trip_id), Number(result.lastInsertRowid), validInvite.created_by ?? null); + this.membership.joinTripAsMember(Number(validInvite.trip_id), Number(result.lastInsertRowid), validInvite.created_by ?? null); } } diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index d6abee72d..f48ab4d97 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -1,14 +1,13 @@ import { Injectable, HttpException } from '@nestjs/common'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import { ReservationsService } from '../reservations/reservations.service'; import { PlacesService } from '../places/places.service'; import { BudgetService } from '../budget/budget.service'; import { AddonsService } from '../addons/addons.service'; import { ADDON_IDS } from '../../addons'; import { MapsService } from '../maps/maps.service'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import type { User } from '../../types'; import { KitineraryExtractorService } from './kitinerary-extractor.service'; import { LlmParseService } from '../llm-parse/llm-parse.service'; @@ -58,10 +57,10 @@ export class BookingImportService { } verifyTripAccess(tripId: string, userId: number) { - return verifyTripAccess(tripId, userId); + return this.dbs.canAccessTrip(tripId, userId); } - canEdit(trip: NonNullable>, user: User): boolean { + canEdit(trip: TripAccess, user: User): boolean { return this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id); } diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 3a97ba961..a3966847d 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -1,14 +1,13 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../common/avatarUrl'; import type { User, BudgetItem, BudgetItemMember, BudgetItemPayer } from '../../types'; import { ExchangeRatesService } from './exchange-rates.service'; -type Trip = NonNullable>; +type Trip = TripAccess; type SettlementRow = { id: number; trip_id: string; from_user_id: number; to_user_id: number; @@ -46,7 +45,7 @@ export class BudgetService { ) {} verifyTripAccess(tripId: string | number, userId: number) { - return verifyTripAccess(tripId, userId); + return this.db.canAccessTrip(tripId, userId); } canEdit(trip: Trip, user: User): boolean { diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 32bb005b3..0a5339f88 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -1,16 +1,15 @@ import path from 'path'; import fs from 'fs'; import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../common/avatarUrl'; import { checkSsrf, createPinnedDispatcher } from '../../utils/ssrfGuard'; import type { CollabNote, CollabPoll, CollabMessage, TripFile, User } from '../../types'; -type Trip = NonNullable>; +type Trip = TripAccess; export interface ReactionRow { emoji: string; @@ -71,7 +70,7 @@ export class CollabService { ) {} verifyTripAccess(tripId: string | number, userId: number) { - return verifyTripAccess(tripId, userId); + return this.db.canAccessTrip(tripId, userId); } canEdit(trip: Trip, user: User): boolean { diff --git a/server/src/nest/common/rowShape.ts b/server/src/nest/common/rowShape.ts new file mode 100644 index 000000000..3b7f2c2a8 --- /dev/null +++ b/server/src/nest/common/rowShape.ts @@ -0,0 +1,63 @@ +import type { AssignmentRow, Tag, Participant } from '../../types'; +import type { PlaceRatingRow } from '../query-helpers/query-helpers.service'; + +/** + * Pure reshaping over rows the query helpers load. Neither function touches the + * database, so neither became a provider — they shipped in + * services/queryHelpers.ts next to the loaders only because the loaders were + * free functions too. + */ + +/** Reshape a flat assignment+place DB row into the nested API response shape with embedded place, tags, and participants. */ +export function formatAssignmentWithPlace(a: AssignmentRow, tags: Partial[], participants: Participant[]) { + return { + id: a.id, + day_id: a.day_id, + place_id: a.place_id, + order_index: a.order_index, + notes: a.notes, + assignment_time: a.assignment_time ?? null, + assignment_end_time: a.assignment_end_time ?? null, + leg_transport_mode: a.leg_transport_mode ?? null, + participants: participants || [], + created_at: a.created_at, + place: { + id: a.place_id, + name: a.place_name, + description: a.place_description, + lat: a.lat, + lng: a.lng, + address: a.address, + category_id: a.category_id, + price: a.price, + currency: a.place_currency, + place_time: a.place_time, + end_time: a.end_time, + duration_minutes: a.duration_minutes, + notes: a.place_notes, + image_url: a.image_url, + transport_mode: a.transport_mode, + google_place_id: a.google_place_id, + google_ftid: a.google_ftid, + osm_id: a.osm_id, + website: a.website, + phone: a.phone, + category: a.category_id ? { + id: a.category_id, + name: a.category_name, + color: a.category_color, + icon: a.category_icon, + } : null, + tags: tags || [], + } + }; +} + +/** avg/count aggregate for a place's rating rows. */ +export function ratingAggregate(ratings: PlaceRatingRow[] | undefined) { + const rows = ratings || []; + return { + rating_avg: rows.length > 0 ? rows.reduce((s, r) => s + r.rating, 0) / rows.length : null, + rating_count: rows.length, + }; +} diff --git a/server/src/nest/days/day-notes.service.ts b/server/src/nest/days/day-notes.service.ts index 793d9f8f0..98b100555 100644 --- a/server/src/nest/days/day-notes.service.ts +++ b/server/src/nest/days/day-notes.service.ts @@ -8,9 +8,8 @@ import { DatabaseService, type TripAccess } from '../database/database.service'; /** * Day-notes domain service — the legacy dayNoteService SQL folded in over * the injected DatabaseService (byte-identical statements and - * coercions). Trip access rides DatabaseService.canAccessTrip (absorbing the - * legacy services/tripAccess re-export); the 'day_edit' permission reuses the - * legacy check. + * coercions). Trip access rides DatabaseService.canAccessTrip; the 'day_edit' + * permission reuses the legacy check. */ @Injectable() export class DayNotesService { diff --git a/server/src/nest/days/days.bridge.ts b/server/src/nest/days/days.bridge.ts index 8f056776e..856b8d9d3 100644 --- a/server/src/nest/days/days.bridge.ts +++ b/server/src/nest/days/days.bridge.ts @@ -3,6 +3,7 @@ import { DatabaseService } from '../database/database.service'; import { RealtimeService } from '../realtime/realtime.service'; import { DaysService } from './days.service'; import { PermissionsService } from '../permissions/permissions.service'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; /** * Non-Nest entry point for the day domain — for code running OUTSIDE the @@ -20,7 +21,7 @@ import { PermissionsService } from '../permissions/permissions.service'; * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto * the shared better-sqlite3 singleton. */ -const days = new DaysService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService()); +const days = new DaysService(new DatabaseService(db), new PermissionsService(new DatabaseService(db)), new RealtimeService(), new QueryHelpersService(new DatabaseService(db))); export function getDay(id: string | number, tripId: string | number) { return days.getDay(id, tripId); diff --git a/server/src/nest/days/days.module.ts b/server/src/nest/days/days.module.ts index c6f846f6e..87a7fe538 100644 --- a/server/src/nest/days/days.module.ts +++ b/server/src/nest/days/days.module.ts @@ -6,6 +6,7 @@ import { DayNotesController } from './day-notes.controller'; import { DayNotesService } from './day-notes.service'; import { DayNotesMcp } from './day-notes.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; +import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { PlacesModule } from '../places/places.module'; import { AuthModule } from '../auth/auth.module'; @@ -19,7 +20,7 @@ import { AuthModule } from '../auth/auth.module'; * controllers). */ @Module({ - imports: [PermissionsModule, PlacesModule, AuthModule], + imports: [PermissionsModule, QueryHelpersModule, PlacesModule, AuthModule], controllers: [DaysController, DayNotesController], providers: [DaysService, DaysMcp, DayNotesService, DayNotesMcp], exports: [DaysService, DayNotesService], diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 7d9ece2e0..9e8c5e05f 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -1,12 +1,13 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace } from '../../services/queryHelpers'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; +import { formatAssignmentWithPlace } from '../common/rowShape'; import type { AssignmentRow, Day, DayNote, User } from '../../types'; -type Trip = { user_id: number }; +type Trip = TripAccess; const MS_PER_DAY = 24 * 60 * 60 * 1000; @@ -77,7 +78,7 @@ export interface CreateAccommodationData { * createAccommodation/deleteAccommodation run their multi-statement writes in * a transaction, and getAssignmentsForDay batch-loads tags instead of one * query per assignment. Trip access - * mirrors the requireTripAccess middleware (canAccessTrip); mutations use the + * rides DatabaseService.canAccessTrip; mutations use the * 'day_edit' permission; the WebSocket broadcast keeps its legacy call path. * Non-Nest consumers (legacy tripService and the transit/transports MCP * registrars) go through days.bridge.ts instead of importing this class @@ -89,10 +90,11 @@ export class DaysService { private readonly db: DatabaseService, private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, + private readonly queryHelpers: QueryHelpersService, ) {} verifyTripAccess(tripId: string | number, userId: number) { - return this.db.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; + return this.db.canAccessTrip(Number(tripId), userId); } canEdit(trip: Trip, user: User): boolean { @@ -126,7 +128,7 @@ export class DaysService { // One batched tag load instead of the legacy per-assignment query; the // non-compact loader returns the same full tag rows (t.* minus the join // key), so the output shape is unchanged. - const tagsByPlaceId = loadTagsByPlaceIds([...new Set(assignments.map(a => a.place_id))]); + const tagsByPlaceId = this.queryHelpers.loadTagsByPlaceIds([...new Set(assignments.map(a => a.place_id))]); return assignments.map(a => { const tags = tagsByPlaceId[a.place_id] || []; @@ -200,10 +202,10 @@ export class DaysService { `, ...dayIds); const placeIds = [...new Set(allAssignments.map(a => a.place_id))]; - const tagsByPlaceId = loadTagsByPlaceIds(placeIds, { compact: true }); + const tagsByPlaceId = this.queryHelpers.loadTagsByPlaceIds(placeIds, { compact: true }); const allAssignmentIds = allAssignments.map(a => a.id); - const participantsByAssignment = loadParticipantsByAssignmentIds(allAssignmentIds); + const participantsByAssignment = this.queryHelpers.loadParticipantsByAssignmentIds(allAssignmentIds); const assignmentsByDayId: Record[]> = {}; for (const a of allAssignments) { diff --git a/server/src/nest/integrations/airtrail-import.controller.ts b/server/src/nest/integrations/airtrail-import.controller.ts index 1607f81aa..26e160a97 100644 --- a/server/src/nest/integrations/airtrail-import.controller.ts +++ b/server/src/nest/integrations/airtrail-import.controller.ts @@ -5,8 +5,8 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { AirtrailAddonGuard } from './airtrail-addon.guard'; import { AirtrailImportDto } from './airtrail.dto'; import type { AirtrailImportResult } from '@trek/shared'; -import { verifyTripAccess } from '../../services/tripAccess'; import { PermissionsService } from '../permissions/permissions.service'; +import { DatabaseService } from '../database/database.service'; import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; /** @@ -17,10 +17,13 @@ import { importAirtrailFlights } from '../../services/airtrail/airtrailImport'; @Controller('api/trips/:tripId/reservations/import') @UseGuards(AirtrailAddonGuard, JwtAuthGuard) export class AirtrailImportController { - constructor(private readonly permissions: PermissionsService) {} + constructor( + private readonly permissions: PermissionsService, + private readonly db: DatabaseService, + ) {} private requireEdit(tripId: string, user: User): void { - const trip = verifyTripAccess(tripId, user.id); + const trip = this.db.canAccessTrip(tripId, user.id); if (!trip) throw new HttpException({ error: 'Trip not found' }, 404); if (!this.permissions.checkPermission('reservation_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id)) { throw new HttpException({ error: 'No permission' }, 403); diff --git a/server/src/nest/oidc/oidc.module.ts b/server/src/nest/oidc/oidc.module.ts index 090ec21ce..ee939bf06 100644 --- a/server/src/nest/oidc/oidc.module.ts +++ b/server/src/nest/oidc/oidc.module.ts @@ -2,9 +2,10 @@ import { Module } from '@nestjs/common'; import { OidcController } from './oidc.controller'; import { OidcService } from './oidc.service'; import { AuthModule } from '../auth/auth.module'; +import { TripMembershipModule } from '../trip-membership/trip-membership.module'; @Module({ - imports: [AuthModule], + imports: [AuthModule, TripMembershipModule], controllers: [OidcController], providers: [OidcService], }) diff --git a/server/src/nest/oidc/oidc.service.ts b/server/src/nest/oidc/oidc.service.ts index 02f1a5bb5..0fbf606e1 100644 --- a/server/src/nest/oidc/oidc.service.ts +++ b/server/src/nest/oidc/oidc.service.ts @@ -9,7 +9,7 @@ import { readEnv, getAppUrl } from '../../app-config'; import { JWT_SECRET, SESSION_DURATION_SECONDS } from '../../config'; import { User } from '../../types'; import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; -import { joinTripAsMember } from '../../services/tripMembership'; +import { TripMembershipService } from '../trip-membership/trip-membership.service'; import { setAuthCookie } from '../common/cookie'; import { AuthService } from '../auth/auth.service'; import { DatabaseService } from '../database/database.service'; @@ -183,6 +183,7 @@ export class OidcService implements OnModuleDestroy { constructor( private readonly db: DatabaseService, private readonly auth: AuthService, + private readonly membership: TripMembershipService, ) { this.stateSweeper = setInterval(() => { const now = Date.now(); @@ -594,7 +595,7 @@ export class OidcService implements OnModuleDestroy { // Trip-bound invite (#1402): auto-add the new SSO user to the trip inside the // same atomic step as the invite consume. Idempotent + owner-safe. if (validInvite?.trip_id) { - joinTripAsMember(Number(validInvite.trip_id), Number(ins.lastInsertRowid), validInvite.created_by ?? null); + this.membership.joinTripAsMember(Number(validInvite.trip_id), Number(ins.lastInsertRowid), validInvite.created_by ?? null); } return ins; }) as { lastInsertRowid: number | bigint }; diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index b6fb013c9..bfc590425 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -2,16 +2,15 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../common/avatarUrl'; import type { UpdateConflict } from '../common/conflictResult'; import type { User } from '../../types'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; /** Privacy fields stamped on a packing item (#858). */ type PrivacyFields = { is_private?: number; owner_id?: number | null }; -type Trip = NonNullable>; +type Trip = TripAccess; export type PackingVisibility = 'common' | 'personal' | 'shared'; @@ -47,7 +46,7 @@ export class PackingService { ) {} verifyTripAccess(tripId: string | number, userId: number) { - return verifyTripAccess(tripId, userId); + return this.db.canAccessTrip(tripId, userId); } /** Mirrors the inline checkPermission('packing_edit', ...) the legacy route runs. */ diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index a1b269ad5..c0a72fb5b 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -3,6 +3,7 @@ import { PlacesController } from './places.controller'; import { PlacesService } from './places.service'; import { PlacesMcp } from './places.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; +import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { MapsModule } from '../maps/maps.module'; import { AuthModule } from '../auth/auth.module'; @@ -15,7 +16,7 @@ import { AuthModule } from '../auth/auth.module'; * is no places.bridge.ts: nothing outside the container consumes this domain. */ @Module({ - imports: [PermissionsModule, MapsModule, AuthModule], + imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule], controllers: [PlacesController], providers: [PlacesService, PlacesMcp], exports: [PlacesService], diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index 5b6d45129..a485d66e6 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -3,12 +3,13 @@ import { XMLValidator } from 'fast-xml-parser'; import { TRACK_COLORS } from '@trek/shared'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import type { PlaceWithTags } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { MapsService } from '../maps/maps.service'; import type { Place, User } from '../../types'; -import { loadTagsByPlaceIds, loadRatingsByPlaceIds, ratingAggregate } from '../../services/queryHelpers'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; +import { ratingAggregate } from '../common/rowShape'; import { checkSsrf, safeFetchFollow, SsrfBlockedError } from '../../utils/ssrfGuard'; import { buildCategoryNameLookup, @@ -50,7 +51,7 @@ import { type PlaceWithCategory, } from './places.helpers'; -type Trip = { user_id: number }; +type Trip = TripAccess; type ImportedPlace = { id: number; route_geometry?: string | null; route_color?: string | null }; @@ -89,7 +90,7 @@ export interface PlaceUpdateInput { * (a place on the equator lost its coordinates). Every other `x || null` is * string-valued, where empty-string-means-absent is the intended reading. * - * Trip access mirrors the requireTripAccess middleware (canAccessTrip); + * Trip access rides DatabaseService.canAccessTrip; * mutations use 'place_edit'. Pure helpers and the frozen XML parsers live in * places.helpers.ts. Nothing outside the Nest container consumes this domain * any more, so there is no places.bridge.ts: the MCP surface is the @@ -103,10 +104,11 @@ export class PlacesService { private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, private readonly maps: MapsService, + private readonly queryHelpers: QueryHelpersService, ) {} verifyTripAccess(tripId: string, userId: number) { - return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; + return this.dbs.canAccessTrip(Number(tripId), userId); } canEdit(trip: Trip, user: User): boolean { @@ -164,8 +166,8 @@ export class PlacesService { const places = this.dbs.prepare(query).all(...params) as PlaceWithCategory[]; const placeIds = places.map(p => p.id); - const tagsByPlaceId = loadTagsByPlaceIds(placeIds); - const ratingsByPlaceId = loadRatingsByPlaceIds(placeIds); + const tagsByPlaceId = this.queryHelpers.loadTagsByPlaceIds(placeIds); + const ratingsByPlaceId = this.queryHelpers.loadRatingsByPlaceIds(placeIds); return places.map(p => ({ ...p, 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 7a322af33..5ad987a25 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -5,7 +5,7 @@ import { RealtimeService } from '../../realtime/realtime.service'; import { isUpdateConflict } from '../../common/conflictResult'; import { getWeather } from '../../../services/weatherService'; import { BLOCKED_EXTENSIONS, filesDir } from '../../files/files.constants'; -import { joinTripAsMember } from '../../../services/tripMembership'; +import { TripMembershipService } from '../../trip-membership/trip-membership.service'; import { NotificationsService } from '../../notifications/notifications.service'; import { createLlmClient } from '../../llm-parse/llm-client.factory'; import { readUserSettingDecrypted } from '../plugins.service'; @@ -165,6 +165,7 @@ export class PluginHostDepsFactory { private readonly collections: CollectionsService, private readonly atlas: AtlasService, private readonly notifications: NotificationsService, + private readonly membership: TripMembershipService, ) {} /** @@ -174,7 +175,7 @@ export class PluginHostDepsFactory { * RESOURCE_FORBIDDEN. */ private canEditTripAs(action: string, tripId: number, userId: number): boolean { - const trip = this.db.canAccessTrip(tripId, userId) as { user_id: number } | undefined; + const trip = this.db.canAccessTrip(tripId, userId); if (!trip) return false; const u = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; if (!u) return false; @@ -228,7 +229,7 @@ export class PluginHostDepsFactory { // Same gate as a REST/MCP budget mutation: the acting user must have trip // access AND the 'budget_edit' permission for their global role. canEditCosts: (tripId, userId) => { - const trip = this.db.canAccessTrip(tripId, userId) as { user_id: number } | undefined; + const trip = this.db.canAccessTrip(tripId, userId); if (!trip) return false; const u = this.db.prepare('SELECT role FROM users WHERE id = ?').get(userId) as { role?: string } | undefined; if (!u) return false; @@ -352,7 +353,7 @@ export class PluginHostDepsFactory { addTripMember: (tripId, targetUserId, invitedBy) => { const target = this.db.prepare('SELECT id FROM users WHERE id = ?').get(targetUserId) as { id: number } | undefined; if (!target) throw new ForbiddenResource(`no user ${targetUserId}`); - return joinTripAsMember(tripId, targetUserId, invitedBy); + return this.membership.joinTripAsMember(tripId, targetUserId, invitedBy); }, removeTripMember: (tripId, targetUserId) => { // Never remove the OWNER via this path — that would orphan the trip. Ownership diff --git a/server/src/nest/plugins/plugins.module.ts b/server/src/nest/plugins/plugins.module.ts index bf3bc052b..1402d1641 100644 --- a/server/src/nest/plugins/plugins.module.ts +++ b/server/src/nest/plugins/plugins.module.ts @@ -45,6 +45,7 @@ import { AddonsModule } from '../addons/addons.module'; import { CollectionsModule } from '../collections/collections.module'; import { AtlasModule } from '../atlas/atlas.module'; import { NotificationsModule } from '../notifications/notifications.module'; +import { TripMembershipModule } from '../trip-membership/trip-membership.module'; /** * Plugin system (#plugins). M0 read side + M2 isolated runtime + M3 frontend: @@ -56,7 +57,7 @@ import { NotificationsModule } from '../notifications/notifications.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, NotificationsModule], + imports: [TagsModule, CategoriesModule, BudgetModule, ReservationsModule, TodoModule, PackingModule, DaysModule, AssignmentsModule, LlmParseModule, FilesModule, CollabModule, VacayModule, TripsModule, PermissionsModule, AuditModule, AddonsModule, PlacesModule, CollectionsModule, AtlasModule, NotificationsModule, TripMembershipModule], controllers: [PluginsController, PluginsFeedController, PluginsProxyController, PluginFrameController, PlaceDetailsController, TripWarningsController, ViewContributionsController, TripCardContributionsController, PluginPhotosController, PluginCalendarController, MapMarkersController, MapLayersController, PluginRoutesController, DayScheduleController, DayTintsController, 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/query-helpers/query-helpers.module.ts b/server/src/nest/query-helpers/query-helpers.module.ts new file mode 100644 index 000000000..30910f087 --- /dev/null +++ b/server/src/nest/query-helpers/query-helpers.module.ts @@ -0,0 +1,12 @@ +import { Module } from '@nestjs/common'; +import { QueryHelpersService } from './query-helpers.service'; + +/** Shared batch loaders for the list endpoints. No controller or MCP surface of + * its own — assignments, days, places and share import it for the loaders. + * Deliberately NOT @Global so e2e TestingModules resolve it transitively + * through each consumer's explicit import, same as PermissionsModule. */ +@Module({ + providers: [QueryHelpersService], + exports: [QueryHelpersService], +}) +export class QueryHelpersModule {} diff --git a/server/src/nest/query-helpers/query-helpers.service.ts b/server/src/nest/query-helpers/query-helpers.service.ts new file mode 100644 index 000000000..311ffb31e --- /dev/null +++ b/server/src/nest/query-helpers/query-helpers.service.ts @@ -0,0 +1,94 @@ +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../database/database.service'; +import type { Tag, Participant } from '../../types'; + +interface TagRow extends Tag { + place_id: number; +} + +interface ParticipantRow { + assignment_id: number; + user_id: number; + username: string; + avatar: string | null; +} + +export interface PlaceRatingRow { + user_id: number; + username: string; + avatar: string | null; + rating: number; +} + +/** + * The batch loaders that keep the list endpoints off N+1 queries — one query per + * collection instead of one per row. Moved 1:1 from services/queryHelpers.ts + * (same SQL, same indexing) onto the injected connection. + * + * Only the loaders live here. The two pure reshaping functions that shipped in + * the same legacy file touch no database and stayed free functions in + * common/rowShape.ts. + */ +@Injectable() +export class QueryHelpersService { + constructor(private readonly db: DatabaseService) {} + + /** Batch-load tags for multiple places in a single query, indexed by place ID. */ + loadTagsByPlaceIds(placeIds: number[], { compact }: { compact?: boolean } = {}): Record[]> { + const tagsByPlaceId: Record[]> = {}; + if (placeIds.length > 0) { + const placeholders = placeIds.map(() => '?').join(','); + const allTags = this.db.all(` + SELECT t.*, pt.place_id FROM tags t + JOIN place_tags pt ON t.id = pt.tag_id + WHERE pt.place_id IN (${placeholders}) + `, ...placeIds); + + for (const tag of allTags) { + const pid = tag.place_id; + if (!tagsByPlaceId[pid]) tagsByPlaceId[pid] = []; + if (compact) { + tagsByPlaceId[pid].push({ id: tag.id, name: tag.name, color: tag.color, created_at: tag.created_at }); + } else { + const { place_id, ...rest } = tag; + tagsByPlaceId[pid].push(rest); + } + } + } + return tagsByPlaceId; + } + + /** Batch-load collaborative ratings (#1435) for multiple places in one query, indexed by place ID. */ + loadRatingsByPlaceIds(placeIds: number[]): Record { + const ratingsByPlaceId: Record = {}; + if (placeIds.length > 0) { + const rows = this.db.all(` + SELECT pr.place_id, pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr + JOIN users u ON pr.user_id = u.id + WHERE pr.place_id IN (${placeIds.map(() => '?').join(',')}) + ORDER BY pr.created_at + `, ...placeIds); + for (const { place_id, ...rest } of rows) { + if (!ratingsByPlaceId[place_id]) ratingsByPlaceId[place_id] = []; + ratingsByPlaceId[place_id].push(rest); + } + } + return ratingsByPlaceId; + } + + /** Batch-load participants for multiple day-assignments in a single query, indexed by assignment ID. */ + loadParticipantsByAssignmentIds(assignmentIds: number[]): Record { + const participantsByAssignment: Record = {}; + if (assignmentIds.length > 0) { + const allParticipants = this.db.all( + `SELECT ap.assignment_id, ap.user_id, u.username, u.avatar FROM assignment_participants ap JOIN users u ON ap.user_id = u.id WHERE ap.assignment_id IN (${assignmentIds.map(() => '?').join(',')})`, + ...assignmentIds, + ); + for (const p of allParticipants) { + if (!participantsByAssignment[p.assignment_id]) participantsByAssignment[p.assignment_id] = []; + participantsByAssignment[p.assignment_id].push({ user_id: p.user_id, username: p.username, avatar: p.avatar }); + } + } + return participantsByAssignment; + } +} diff --git a/server/src/nest/reservations/accommodations.service.ts b/server/src/nest/reservations/accommodations.service.ts index 1340db511..46b2958a7 100644 --- a/server/src/nest/reservations/accommodations.service.ts +++ b/server/src/nest/reservations/accommodations.service.ts @@ -1,12 +1,12 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import type { User } from '../../types'; import { DaysService } from '../days/days.service'; -type Trip = { user_id: number }; +type Trip = TripAccess; /** * Thin Nest wrapper around the accommodation parts of the day domain. @@ -23,9 +23,9 @@ export class AccommodationsService { private readonly realtime: RealtimeService, ) {} - /** Mirrors the requireTripAccess middleware (owner or member), returning the trip. */ + /** Owner or member, returning the trip. */ verifyTripAccess(tripId: string, userId: number) { - return this.dbs.canAccessTrip(Number(tripId), userId) as Trip | null | undefined; + return this.dbs.canAccessTrip(Number(tripId), userId); } canEdit(trip: Trip, user: User): boolean { diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 99c55c9c2..62ebd4030 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -1,15 +1,14 @@ import { Injectable } from '@nestjs/common'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import { avatarUrl } from '../common/avatarUrl'; import type { Reservation, User } from '../../types'; import { BudgetService } from '../budget/budget.service'; import { typeToCostCategory } from '@trek/shared'; -type Trip = NonNullable>; +type Trip = TripAccess; type BudgetEntry = { total_price?: number; category?: string } | undefined; export interface ReservationEndpoint { @@ -138,7 +137,7 @@ export class ReservationsService { ) {} verifyTripAccess(tripId: string | number, userId: number) { - return verifyTripAccess(tripId, userId); + return this.db.canAccessTrip(tripId, userId); } canEdit(trip: Trip, user: User): boolean { diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index cb32e6d52..d35c52f95 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -4,10 +4,11 @@ import { ShareService } from './share.service'; import { ShareMcp } from './share.mcp'; import { SettingsModule } from '../settings/settings.module'; import { PermissionsModule } from '../permissions/permissions.module'; +import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { AuthModule } from '../auth/auth.module'; @Module({ - imports: [SettingsModule, PermissionsModule, AuthModule], + imports: [SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule], controllers: [TripShareController, SharedController], providers: [ShareService, ShareMcp], }) diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index fa5a92d05..c80bdb45b 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -3,7 +3,7 @@ import crypto from 'crypto'; import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { loadTagsByPlaceIds } from '../../services/queryHelpers'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { serveFilePath } from '../../services/placePhotoCache'; import { SettingsService } from '../settings/settings.service'; import type { User } from '../../types'; @@ -56,6 +56,7 @@ export class ShareService { private readonly dbs: DatabaseService, private readonly settings: SettingsService, private readonly permissions: PermissionsService, + private readonly queryHelpers: QueryHelpersService, ) {} verifyTripAccess(tripId: string, userId: number) { @@ -187,7 +188,7 @@ export class ShareService { `, ...dayIds); const placeIds = [...new Set(allAssignments.map((a: any) => a.place_id))]; - const tagsByPlace = loadTagsByPlaceIds(placeIds, { compact: true }); + const tagsByPlace = this.queryHelpers.loadTagsByPlaceIds(placeIds, { compact: true }); const byDay: Record = {}; for (const a of allAssignments as any[]) { diff --git a/server/src/nest/todo/todo.service.ts b/server/src/nest/todo/todo.service.ts index 6f464c633..8d2025ba0 100644 --- a/server/src/nest/todo/todo.service.ts +++ b/server/src/nest/todo/todo.service.ts @@ -2,11 +2,10 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { verifyTripAccess } from '../../services/tripAccess'; import type { User } from '../../types'; -import { DatabaseService } from '../database/database.service'; +import { DatabaseService, type TripAccess } from '../database/database.service'; -type Trip = NonNullable>; +type Trip = TripAccess; /** * Todo domain service — owns the todo SQL (moved 1:1 from the legacy @@ -26,7 +25,7 @@ export class TodoService { ) {} verifyTripAccess(tripId: string | number, userId: number) { - return verifyTripAccess(tripId, userId); + return this.db.canAccessTrip(tripId, userId); } canEdit(trip: Trip, user: User): boolean { diff --git a/server/src/nest/trip-invite/trip-invite.module.ts b/server/src/nest/trip-invite/trip-invite.module.ts index 502ef80e3..85ba77804 100644 --- a/server/src/nest/trip-invite/trip-invite.module.ts +++ b/server/src/nest/trip-invite/trip-invite.module.ts @@ -4,9 +4,10 @@ import { TripInviteLinkController, TripInviteController } from './trip-invite.co import { TripInviteService } from './trip-invite.service'; import { PermissionsModule } from '../permissions/permissions.module'; import { AuditModule } from '../audit/audit.module'; +import { TripMembershipModule } from '../trip-membership/trip-membership.module'; @Module({ - imports: [RateLimitModule, PermissionsModule, AuditModule], + imports: [RateLimitModule, PermissionsModule, AuditModule, TripMembershipModule], controllers: [TripInviteLinkController, TripInviteController], providers: [TripInviteService], }) diff --git a/server/src/nest/trip-invite/trip-invite.service.ts b/server/src/nest/trip-invite/trip-invite.service.ts index 547804453..8aa66fbfd 100644 --- a/server/src/nest/trip-invite/trip-invite.service.ts +++ b/server/src/nest/trip-invite/trip-invite.service.ts @@ -3,7 +3,7 @@ import crypto from 'crypto'; import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; -import { joinTripAsMember } from '../../services/tripMembership'; +import { TripMembershipService } from '../trip-membership/trip-membership.service'; import type { User } from '../../types'; type Trip = TripAccess; @@ -31,6 +31,7 @@ export class TripInviteService { constructor( private readonly dbs: DatabaseService, private readonly permissions: PermissionsService, + private readonly membership: TripMembershipService, ) {} verifyTripAccess(tripId: string, userId: number) { @@ -113,5 +114,5 @@ export class TripInviteService { /** Join the resolved trip as the current (authenticated, non-guest) user. * invited_by is null — they joined via a link, not a personal invite. */ - join(tripId: number, userId: number) { return joinTripAsMember(tripId, userId, null); } + join(tripId: number, userId: number) { return this.membership.joinTripAsMember(tripId, userId, null); } } diff --git a/server/src/nest/trip-membership/trip-membership.module.ts b/server/src/nest/trip-membership/trip-membership.module.ts new file mode 100644 index 000000000..a2b1d7045 --- /dev/null +++ b/server/src/nest/trip-membership/trip-membership.module.ts @@ -0,0 +1,12 @@ +import { Module } from '@nestjs/common'; +import { TripMembershipService } from './trip-membership.service'; + +/** Trip membership joins. No controller of its own — the HTTP surface lives with + * auth, oidc and trip-invite, which all import this for the join itself. Kept + * separate so those three (plus the plugin host) share one implementation + * without AuthModule and TripInviteModule importing each other. */ +@Module({ + providers: [TripMembershipService], + exports: [TripMembershipService], +}) +export class TripMembershipModule {} diff --git a/server/src/nest/trip-membership/trip-membership.service.ts b/server/src/nest/trip-membership/trip-membership.service.ts new file mode 100644 index 000000000..cabfc0a39 --- /dev/null +++ b/server/src/nest/trip-membership/trip-membership.service.ts @@ -0,0 +1,39 @@ +import { Injectable } from '@nestjs/common'; +import { DatabaseService } from '../database/database.service'; + +/** + * Adding an existing user to a trip as a member. Its own domain rather than a + * method on auth or trip-invite: both of those call it, as does OIDC and the + * plugin RPC host, and folding it into either would put AuthModule and + * TripInviteModule in a cycle. + */ +@Injectable() +export class TripMembershipService { + constructor(private readonly db: DatabaseService) {} + + /** + * Add an existing user to a trip as a member, by user id. + * + * Idempotent and safe: it skips the trip owner and anyone who is already a + * member, and no-ops if the trip no longer exists. Shared by trip invite-link + * joins (#1143) and the trip-bound admin invite auto-join (#1402). The caller is + * responsible for authenticating/creating the user first, so the id always + * belongs to a real (non-guest) account. + * + * Returns whether a new membership row was actually created. + */ + joinTripAsMember( + tripId: number, + userId: number, + invitedBy: number | null, + ): { joined: boolean; tripId: number } { + const trip = this.db.get<{ id: number; user_id: number }>('SELECT id, user_id FROM trips WHERE id = ?', tripId); + if (!trip) return { joined: false, tripId }; + // The owner already has full access; never add them as a member. + if (trip.user_id === userId) return { joined: false, tripId }; + const existing = this.db.get('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?', tripId, userId); + if (existing) return { joined: false, tripId }; + this.db.run('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)', tripId, userId, invitedBy); + return { joined: true, tripId }; + } +} diff --git a/server/src/nest/trips/trips.bridge.ts b/server/src/nest/trips/trips.bridge.ts index 0526a11da..be335b62a 100644 --- a/server/src/nest/trips/trips.bridge.ts +++ b/server/src/nest/trips/trips.bridge.ts @@ -2,6 +2,7 @@ import { db } from '../../db/database'; import { DatabaseService } from '../database/database.service'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; +import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { TripsService } from './trips.service'; import { TodoService } from '../todo/todo.service'; import { PackingService } from '../packing/packing.service'; @@ -38,13 +39,13 @@ const trips = new TripsService( new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new ReservationsService(dbs(), new PermissionsService(dbs()), budget, new RealtimeService()), - new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new QueryHelpersService(dbs())), new PermissionsService(dbs()), budget, new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new VacayService(dbs(), new RealtimeService()), new RealtimeService(), - new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs())), + new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs()), new QueryHelpersService(dbs())), ); export function getTripOwner(tripId: string | number) { diff --git a/server/src/services/queryHelpers.ts b/server/src/services/queryHelpers.ts deleted file mode 100644 index f4f0781e2..000000000 --- a/server/src/services/queryHelpers.ts +++ /dev/null @@ -1,133 +0,0 @@ -import { db } from '../db/database'; -import { AssignmentRow, Tag, Participant } from '../types'; - -interface TagRow extends Tag { - place_id: number; -} - -interface ParticipantRow { - assignment_id: number; - user_id: number; - username: string; - avatar: string | null; -} - -/** Batch-load tags for multiple places in a single query, indexed by place ID. */ -function loadTagsByPlaceIds(placeIds: number[], { compact }: { compact?: boolean } = {}): Record[]> { - const tagsByPlaceId: Record[]> = {}; - if (placeIds.length > 0) { - const placeholders = placeIds.map(() => '?').join(','); - const allTags = db.prepare(` - SELECT t.*, pt.place_id FROM tags t - JOIN place_tags pt ON t.id = pt.tag_id - WHERE pt.place_id IN (${placeholders}) - `).all(...placeIds) as TagRow[]; - - for (const tag of allTags) { - const pid = tag.place_id; - if (!tagsByPlaceId[pid]) tagsByPlaceId[pid] = []; - if (compact) { - tagsByPlaceId[pid].push({ id: tag.id, name: tag.name, color: tag.color, created_at: tag.created_at }); - } else { - const { place_id, ...rest } = tag; - tagsByPlaceId[pid].push(rest); - } - } - } - return tagsByPlaceId; -} - -export interface PlaceRatingRow { - user_id: number; - username: string; - avatar: string | null; - rating: number; -} - -/** Batch-load collaborative ratings (#1435) for multiple places in one query, indexed by place ID. */ -function loadRatingsByPlaceIds(placeIds: number[]): Record { - const ratingsByPlaceId: Record = {}; - if (placeIds.length > 0) { - const rows = db.prepare(` - SELECT pr.place_id, pr.user_id, u.username, u.avatar, pr.rating FROM place_ratings pr - JOIN users u ON pr.user_id = u.id - WHERE pr.place_id IN (${placeIds.map(() => '?').join(',')}) - ORDER BY pr.created_at - `).all(...placeIds) as (PlaceRatingRow & { place_id: number })[]; - for (const { place_id, ...rest } of rows) { - if (!ratingsByPlaceId[place_id]) ratingsByPlaceId[place_id] = []; - ratingsByPlaceId[place_id].push(rest); - } - } - return ratingsByPlaceId; -} - -/** avg/count aggregate for a place's rating rows. */ -function ratingAggregate(ratings: PlaceRatingRow[] | undefined) { - const rows = ratings || []; - return { - rating_avg: rows.length > 0 ? rows.reduce((s, r) => s + r.rating, 0) / rows.length : null, - rating_count: rows.length, - }; -} - -/** Batch-load participants for multiple day-assignments in a single query, indexed by assignment ID. */ -function loadParticipantsByAssignmentIds(assignmentIds: number[]): Record { - const participantsByAssignment: Record = {}; - if (assignmentIds.length > 0) { - const allParticipants = db.prepare(`SELECT ap.assignment_id, ap.user_id, u.username, u.avatar FROM assignment_participants ap JOIN users u ON ap.user_id = u.id WHERE ap.assignment_id IN (${assignmentIds.map(() => '?').join(',')})`) - .all(...assignmentIds) as ParticipantRow[]; - for (const p of allParticipants) { - if (!participantsByAssignment[p.assignment_id]) participantsByAssignment[p.assignment_id] = []; - participantsByAssignment[p.assignment_id].push({ user_id: p.user_id, username: p.username, avatar: p.avatar }); - } - } - return participantsByAssignment; -} - -/** Reshape a flat assignment+place DB row into the nested API response shape with embedded place, tags, and participants. */ -function formatAssignmentWithPlace(a: AssignmentRow, tags: Partial[], participants: Participant[]) { - return { - id: a.id, - day_id: a.day_id, - place_id: a.place_id, - order_index: a.order_index, - notes: a.notes, - assignment_time: a.assignment_time ?? null, - assignment_end_time: a.assignment_end_time ?? null, - leg_transport_mode: a.leg_transport_mode ?? null, - participants: participants || [], - created_at: a.created_at, - place: { - id: a.place_id, - name: a.place_name, - description: a.place_description, - lat: a.lat, - lng: a.lng, - address: a.address, - category_id: a.category_id, - price: a.price, - currency: a.place_currency, - place_time: a.place_time, - end_time: a.end_time, - duration_minutes: a.duration_minutes, - notes: a.place_notes, - image_url: a.image_url, - transport_mode: a.transport_mode, - google_place_id: a.google_place_id, - google_ftid: a.google_ftid, - osm_id: a.osm_id, - website: a.website, - phone: a.phone, - category: a.category_id ? { - id: a.category_id, - name: a.category_name, - color: a.category_color, - icon: a.category_icon, - } : null, - tags: tags || [], - } - }; -} - -export { loadTagsByPlaceIds, loadParticipantsByAssignmentIds, formatAssignmentWithPlace, loadRatingsByPlaceIds, ratingAggregate }; diff --git a/server/src/services/tripAccess.ts b/server/src/services/tripAccess.ts deleted file mode 100644 index 10b91c3fa..000000000 --- a/server/src/services/tripAccess.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { canAccessTrip } from '../db/database'; - -/** - * Returns the trip row if the user is the owner or a member, otherwise undefined. - * Shared by the domain services so each one exposes the same access check. - */ -export function verifyTripAccess(tripId: string | number, userId: number) { - return canAccessTrip(tripId, userId); -} diff --git a/server/src/services/tripMembership.ts b/server/src/services/tripMembership.ts deleted file mode 100644 index 2a13d2459..000000000 --- a/server/src/services/tripMembership.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { db } from '../db/database'; - -/** - * Add an existing user to a trip as a member, by user id. - * - * Idempotent and safe: it skips the trip owner and anyone who is already a - * member, and no-ops if the trip no longer exists. Shared by trip invite-link - * joins (#1143) and the trip-bound admin invite auto-join (#1402). The caller is - * responsible for authenticating/creating the user first, so the id always - * belongs to a real (non-guest) account. - * - * Returns whether a new membership row was actually created. - */ -export function joinTripAsMember( - tripId: number, - userId: number, - invitedBy: number | null, -): { joined: boolean; tripId: number } { - const trip = db.prepare('SELECT id, user_id FROM trips WHERE id = ?').get(tripId) as - | { id: number; user_id: number } - | undefined; - if (!trip) return { joined: false, tripId }; - // The owner already has full access; never add them as a member. - if (trip.user_id === userId) return { joined: false, tripId }; - const existing = db.prepare('SELECT id FROM trip_members WHERE trip_id = ? AND user_id = ?').get(tripId, userId); - if (existing) return { joined: false, tripId }; - db.prepare('INSERT INTO trip_members (trip_id, user_id, invited_by) VALUES (?, ?, ?)').run(tripId, userId, invitedBy); - return { joined: true, tripId }; -} diff --git a/server/tests/e2e/trip-invite.e2e.test.ts b/server/tests/e2e/trip-invite.e2e.test.ts index 856c989f4..279346b2a 100644 --- a/server/tests/e2e/trip-invite.e2e.test.ts +++ b/server/tests/e2e/trip-invite.e2e.test.ts @@ -50,14 +50,16 @@ import { PermissionsService } from '../../src/nest/permissions/permissions.servi // PermissionsService singleton (created in beforeAll, after build()). let checkPermission: MockInstance; -const { joinTripAsMember } = vi.hoisted(() => ({ joinTripAsMember: vi.fn() })); -vi.mock('../../src/services/tripMembership', () => ({ joinTripAsMember })); +// The join itself is stubbed out of the container (TRIP-JOIN-* cover what it +// writes); these cases assert the invite route's own decision to call it. +const joinTripAsMember = vi.fn(); // 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() })); import { TripInviteModule } from '../../src/nest/trip-invite/trip-invite.module'; +import { TripMembershipService } from '../../src/nest/trip-membership/trip-membership.service'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; @@ -66,7 +68,9 @@ describe('Trip invite-link e2e (real auth guard + temp SQLite)', () => { let app: Awaited>; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, TripInviteModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, TripInviteModule] }) + .overrideProvider(TripMembershipService).useValue({ joinTripAsMember }) + .compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalPipes(new ZodValidationPipe()); diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index f79c880ae..ca0a6ebd4 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -46,6 +46,7 @@ import { TripsService } from '../../src/nest/trips/trips.service'; import { VacayMcp } from '../../src/nest/vacay/vacay.mcp'; import { VacayService } from '../../src/nest/vacay/vacay.service'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -59,14 +60,15 @@ export function createMcpTestRegistry(): McpRegistry { const permissionsService = new PermissionsService(dbService); const authService = new AuthService(dbService, permissionsService, new AtlasService(dbService)); const realtimeService = new RealtimeService(); - const daysService = new DaysService(dbService, permissionsService, realtimeService); + const queryHelpersService = new QueryHelpersService(dbService); + const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService); const exchangeRatesService = new ExchangeRatesService(); const budgetService = new BudgetService(dbService, permissionsService, exchangeRatesService, realtimeService); const todoService = new TodoService(dbService, permissionsService, realtimeService); const packingService = new PackingService(dbService, permissionsService, realtimeService); const collabService = new CollabService(dbService, permissionsService, realtimeService); const mapsService = new MapsService(dbService); - const placesService = new PlacesService(dbService, permissionsService, realtimeService, mapsService); + const placesService = new PlacesService(dbService, permissionsService, realtimeService, mapsService, queryHelpersService); const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService); const tripsService = new TripsService( dbService, @@ -92,11 +94,11 @@ export function createMcpTestRegistry(): McpRegistry { new ReservationsMcp(reservationsService, daysService, budgetService, authService), new DayNotesMcp(new DayNotesService(dbService, permissionsService, realtimeService), authService), new DaysMcp(daysService, dbService, placesService, authService), - new AssignmentsMcp(new AssignmentsService(dbService, permissionsService, realtimeService), daysService, authService), + new AssignmentsMcp(new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService), daysService, authService), new CollabMcp(collabService, authService), new VacayMcp(new VacayService(dbService, realtimeService), authService), new TripsMcp(tripsService, todoService, collabService, authService), - new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService), authService), + new ShareMcp(new ShareService(dbService, new SettingsService(dbService), permissionsService, queryHelpersService), authService), new MapsMcp(mapsService), new PlacesMcp(placesService, mapsService, dbService, authService), new CollectionsMcp(new CollectionsService(dbService, permissionsService, realtimeService), dbService, authService), diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 5acd8c994..185802a9c 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -28,6 +28,7 @@ import { PermissionsService } from '../../src/nest/permissions/permissions.servi import { AuditService } from '../../src/nest/audit/audit.service'; import { AddonsService } from '../../src/nest/addons/addons.service'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -41,14 +42,15 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto const realtime = new RealtimeService(); const budget = new BudgetService(dbs, permissions, exchangeRates, realtime); const addons = new AddonsService(dbs); + const queryHelpers = new QueryHelpersService(dbs); const todos = new TodoService(dbs, permissions, realtime); const packing = new PackingService(dbs, permissions, realtime); const files = new FilesService(dbs, permissions, realtime); const reservations = new ReservationsService(dbs, permissions, budget, realtime); const collab = new CollabService(dbs, permissions, realtime); const vacay = new VacayService(dbs, realtime); - const days = new DaysService(dbs, permissions, realtime); - const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs)); + const days = new DaysService(dbs, permissions, realtime, queryHelpers); + const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs), queryHelpers); return new PluginHostDepsFactory( budget, reservations, @@ -58,7 +60,7 @@ export function createHostDepsFactory(dbs: DatabaseService): PluginHostDepsFacto packing, new PluginOAuthService(dbs), new DayNotesService(dbs, permissions, realtime), - new AssignmentsService(dbs, permissions, realtime), + new AssignmentsService(dbs, permissions, realtime, queryHelpers), new LlmConfigResolver(new SettingsService(dbs), dbs, addons), dbs, files, diff --git a/server/tests/integration/dayReorder.test.ts b/server/tests/integration/dayReorder.test.ts index 1feec6eea..3626efa30 100644 --- a/server/tests/integration/dayReorder.test.ts +++ b/server/tests/integration/dayReorder.test.ts @@ -32,8 +32,9 @@ import { DatabaseService } from '../../src/nest/database/database.service'; import { PermissionsService } from '../../src/nest/permissions/permissions.service'; import { DaysService, DayReorderError } from '../../src/nest/days/days.service'; import { RealtimeService } from '../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; -const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); +const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb))); const reorderDays = (tripId: number, orderedIds: number[]) => svc.reorder(tripId, orderedIds); const insertDay = (tripId: number, position?: number) => svc.insert(tripId, position); diff --git a/server/tests/unit/middleware/tripAccess.test.ts b/server/tests/unit/middleware/tripAccess.test.ts deleted file mode 100644 index fb15709f4..000000000 --- a/server/tests/unit/middleware/tripAccess.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -/** - * Unit tests for requireTripAccess and requireTripOwner middleware. - * TRIP-ACCESS-001 through TRIP-ACCESS-010. - * canAccessTrip and isOwner are mocked; no DB required. - */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import type { Request, Response, NextFunction } from 'express'; - -const mockCanAccessTrip = vi.fn(); -const mockIsOwner = vi.fn(); - -vi.mock('../../../src/db/database', () => ({ - canAccessTrip: (...args: any[]) => mockCanAccessTrip(...args), - isOwner: (...args: any[]) => mockIsOwner(...args), -})); -vi.mock('../../../src/config', () => ({ JWT_SECRET: 'test-secret' })); - -import { requireTripAccess, requireTripOwner } from '../../../src/middleware/tripAccess'; - -function makeRes(): { res: Response; status: ReturnType; json: ReturnType } { - const json = vi.fn(); - const status = vi.fn(() => ({ json })); - const res = { status } as unknown as Response; - return { res, status, json }; -} - -function makeReq(params: Record = {}, userId = 1): Request { - return { - params, - user: { id: userId }, - } as unknown as Request; -} - -beforeEach(() => { - mockCanAccessTrip.mockReset(); - mockIsOwner.mockReset(); -}); - -// ── requireTripAccess ───────────────────────────────────────────────────────── - -describe('requireTripAccess', () => { - it('TRIP-ACCESS-001: returns 400 when no tripId param', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - requireTripAccess(makeReq({}), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(400); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.any(String) })); - }); - - it('TRIP-ACCESS-002: returns 404 when canAccessTrip returns null (not a member)', () => { - mockCanAccessTrip.mockReturnValue(null); - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - requireTripAccess(makeReq({ tripId: '42' }), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(404); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.any(String) })); - }); - - it('TRIP-ACCESS-003: calls next and attaches trip when user has access', () => { - const fakeTrip = { id: 42, user_id: 1 }; - mockCanAccessTrip.mockReturnValue(fakeTrip); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - const req = makeReq({ tripId: '42' }, 1); - requireTripAccess(req, res, next); - expect(next).toHaveBeenCalledOnce(); - expect((req as any).trip).toEqual(fakeTrip); - }); - - it('TRIP-ACCESS-004: accepts req.params.id as fallback when tripId is absent', () => { - const fakeTrip = { id: 7, user_id: 2 }; - mockCanAccessTrip.mockReturnValue(fakeTrip); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - requireTripAccess(makeReq({ id: '7' }), res, next); - expect(mockCanAccessTrip).toHaveBeenCalledWith(7, expect.any(Number)); - expect(next).toHaveBeenCalledOnce(); - }); - - it('TRIP-ACCESS-005: passes numeric tripId to canAccessTrip', () => { - mockCanAccessTrip.mockReturnValue({ id: 99, user_id: 3 }); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - requireTripAccess(makeReq({ tripId: '99' }, 3), res, next); - expect(mockCanAccessTrip).toHaveBeenCalledWith(99, 3); - }); -}); - -// ── requireTripOwner ────────────────────────────────────────────────────────── - -describe('requireTripOwner', () => { - it('TRIP-ACCESS-006: returns 400 when no tripId param', () => { - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - requireTripOwner(makeReq({}), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(400); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.any(String) })); - }); - - it('TRIP-ACCESS-007: returns 403 when user is not the owner', () => { - mockIsOwner.mockReturnValue(false); - const next = vi.fn() as unknown as NextFunction; - const { res, status, json } = makeRes(); - requireTripOwner(makeReq({ tripId: '10' }, 2), res, next); - expect(next).not.toHaveBeenCalled(); - expect(status).toHaveBeenCalledWith(403); - expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.any(String) })); - }); - - it('TRIP-ACCESS-008: calls next when user is the owner', () => { - mockIsOwner.mockReturnValue(true); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - requireTripOwner(makeReq({ tripId: '10' }, 1), res, next); - expect(next).toHaveBeenCalledOnce(); - }); - - it('TRIP-ACCESS-009: accepts req.params.id as fallback when tripId is absent', () => { - mockIsOwner.mockReturnValue(true); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - requireTripOwner(makeReq({ id: '5' }, 1), res, next); - expect(mockIsOwner).toHaveBeenCalledWith(5, 1); - expect(next).toHaveBeenCalledOnce(); - }); - - it('TRIP-ACCESS-010: passes numeric tripId to isOwner', () => { - mockIsOwner.mockReturnValue(true); - const next = vi.fn() as unknown as NextFunction; - const { res } = makeRes(); - requireTripOwner(makeReq({ tripId: '77' }, 4), res, next); - expect(mockIsOwner).toHaveBeenCalledWith(77, 4); - }); -}); diff --git a/server/tests/unit/nest/assignments.service.test.ts b/server/tests/unit/nest/assignments.service.test.ts index 0c89490b1..712aee6d8 100644 --- a/server/tests/unit/nest/assignments.service.test.ts +++ b/server/tests/unit/nest/assignments.service.test.ts @@ -53,8 +53,9 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { AssignmentsService } from '../../../src/nest/assignments/assignments.service'; import { createAssignment, dayExists, placeExists, getAssignmentForTrip } from '../../../src/nest/assignments/assignments.bridge'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; -const svc = new AssignmentsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); +const svc = new AssignmentsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/auth.service.test.ts b/server/tests/unit/nest/auth.service.test.ts index 1f9a4dd63..80d7e9509 100644 --- a/server/tests/unit/nest/auth.service.test.ts +++ b/server/tests/unit/nest/auth.service.test.ts @@ -55,7 +55,6 @@ vi.mock('../../../src/nest/common/crypto/apiKeyCrypto', () => ({ })); vi.mock('../../../src/services/ephemeralTokens', () => ({ createEphemeralToken: vi.fn() })); vi.mock('../../../src/services/notifications', () => ({ sendPasswordResetEmail: vi.fn() })); -vi.mock('../../../src/services/tripMembership', () => ({ joinTripAsMember: vi.fn() })); vi.mock('../../../src/mcp/sessionManager', () => ({ revokeUserSessions: vi.fn() })); vi.mock('../../../src/scheduler', () => ({ startTripReminders: vi.fn(), @@ -82,13 +81,18 @@ import { verifyJwtAndLoadUser } from '../../../src/middleware/auth'; import { authenticator } from 'otplib'; import { hashBackupCode } from '../../../src/nest/auth/auth.helpers'; import { createEphemeralToken } from '../../../src/services/ephemeralTokens'; -import { joinTripAsMember } from '../../../src/services/tripMembership'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { revokeUserSessions } from '../../../src/mcp/sessionManager'; +// Stubbed rather than real: these tests assert that the invite path CALLS the +// join, not what the join writes (TRIP-JOIN-* cover that). +const joinTripAsMember = vi.fn(); +const membershipStub = { joinTripAsMember } as unknown as TripMembershipService; const svc = new AuthService( new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new AtlasService(new DatabaseService(testDb)), + membershipStub, ); // --------------------------------------------------------------------------- @@ -1282,7 +1286,7 @@ describe('auth quirk fixes', () => { const trip = createTrip(testDb, owner.id); const invite = createInviteToken(testDb, { max_uses: 5 }); testDb.prepare('UPDATE invite_tokens SET trip_id = ? WHERE id = ?').run(trip.id, invite.id); - vi.mocked(joinTripAsMember).mockImplementationOnce(() => { throw new Error('boom'); }); + joinTripAsMember.mockImplementationOnce(() => { throw new Error('boom'); }); const result = svc.registerUser({ username: 'rollback', email: 'rollback@x.com', password: 'Secure123!', invite_token: invite.token }); diff --git a/server/tests/unit/nest/booking-import/booking-import.service.test.ts b/server/tests/unit/nest/booking-import/booking-import.service.test.ts index 3f00e7e00..316381ecb 100644 --- a/server/tests/unit/nest/booking-import/booking-import.service.test.ts +++ b/server/tests/unit/nest/booking-import/booking-import.service.test.ts @@ -3,12 +3,16 @@ import { HttpException } from '@nestjs/common'; // Mock the heavy side-effect imports so the service module loads cleanly; the // preview() path under test only touches the extractor + llmParse deps. -vi.mock('../../../../src/db/database', () => ({ db: { prepare: vi.fn() }, closeDb: () => {}, reinitialize: () => {} })); +vi.mock('../../../../src/db/database', () => ({ + db: { prepare: vi.fn() }, closeDb: () => {}, reinitialize: () => {}, + // Trip access reaches these through DatabaseService; preview() never calls + // them, but the module-level import has to resolve. + canAccessTrip: vi.fn(), isOwner: () => false, getPlaceWithTags: () => null, +})); import { db as dbConn } from '../../../../src/db/database'; import { DatabaseService } from '../../../../src/nest/database/database.service'; vi.mock('../../../../src/websocket', () => ({ broadcast: vi.fn() })); const permissionsStub = { checkPermission: vi.fn(() => true) }; -vi.mock('../../../../src/services/tripAccess', () => ({ verifyTripAccess: vi.fn() })); import { BookingImportService } from '../../../../src/nest/booking-import/booking-import.service'; diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index ec94c523d..d5f88a97e 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -70,6 +70,7 @@ import { ReservationsService } from '../../../src/nest/reservations/reservations import { DaysService } from '../../../src/nest/days/days.service'; import { CollabService } from '../../../src/nest/collab/collab.service'; import { VacayService } from '../../../src/nest/vacay/vacay.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; const budget = new BudgetService( new DatabaseService(testDb), @@ -88,7 +89,7 @@ const tripsSvc = new TripsService( new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new ReservationsService(dbs(), new PermissionsService(dbs()), budget, new RealtimeService()), - new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService()), + new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new QueryHelpersService(dbs())), new PermissionsService(dbs()), budget, new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService()), diff --git a/server/tests/unit/nest/budget.service.test.ts b/server/tests/unit/nest/budget.service.test.ts index 4f067642a..501598d79 100644 --- a/server/tests/unit/nest/budget.service.test.ts +++ b/server/tests/unit/nest/budget.service.test.ts @@ -50,7 +50,7 @@ beforeEach(() => { }); describe('BudgetService', () => { - it('verifyTripAccess resolves through canAccessTrip (services/tripAccess)', () => { + it('verifyTripAccess resolves through DatabaseService.canAccessTrip', () => { canAccessTrip.mockReturnValue({ id: 5, user_id: 2 }); expect(svc().verifyTripAccess('5', 2)).toEqual({ id: 5, user_id: 2 }); expect(canAccessTrip).toHaveBeenCalledWith('5', 2); diff --git a/server/tests/unit/services/queryHelpers.test.ts b/server/tests/unit/nest/common/rowShape.test.ts similarity index 72% rename from server/tests/unit/services/queryHelpers.test.ts rename to server/tests/unit/nest/common/rowShape.test.ts index 68c9dfa76..a8ac363f3 100644 --- a/server/tests/unit/services/queryHelpers.test.ts +++ b/server/tests/unit/nest/common/rowShape.test.ts @@ -1,11 +1,7 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect } from 'vitest'; -vi.mock('../../../src/db/database', () => ({ - db: { prepare: () => ({ all: () => [], get: vi.fn() }) }, -})); - -import { formatAssignmentWithPlace } from '../../../src/services/queryHelpers'; -import type { AssignmentRow, Tag, Participant } from '../../../src/types'; +import { formatAssignmentWithPlace, ratingAggregate } from '../../../../src/nest/common/rowShape'; +import type { AssignmentRow, Tag, Participant } from '../../../../src/types'; function makeRow(overrides: Partial = {}): AssignmentRow { return { @@ -91,4 +87,26 @@ describe('formatAssignmentWithPlace', () => { const result = formatAssignmentWithPlace(makeRow({ category_id: 0 as any }), [], []); expect(result.place.category).toBeNull(); }); + + it('passes tags and participants through untouched', () => { + const result = formatAssignmentWithPlace(makeRow(), sampleTags, sampleParticipants); + expect(result.place.tags).toEqual(sampleTags); + expect(result.participants).toEqual(sampleParticipants); + }); +}); + +describe('ratingAggregate', () => { + const rate = (rating: number, user_id = 1) => ({ user_id, username: 'u', avatar: null, rating }); + + it('averages the ratings and counts them', () => { + expect(ratingAggregate([rate(5), rate(2, 2)])).toEqual({ rating_avg: 3.5, rating_count: 2 }); + }); + + it('reports a null average for an empty list rather than NaN', () => { + expect(ratingAggregate([])).toEqual({ rating_avg: null, rating_count: 0 }); + }); + + it('treats a place with no rating rows at all the same as an empty list', () => { + expect(ratingAggregate(undefined)).toEqual({ rating_avg: null, rating_count: 0 }); + }); }); diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 35b74db16..f0674d402 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -54,8 +54,9 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { DaysService, addDays } from '../../../src/nest/days/days.service'; import { getDay as bridgeGetDay, listDays as bridgeListDays } from '../../../src/nest/days/days.bridge'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; -const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); +const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/oidc.service.test.ts b/server/tests/unit/nest/oidc.service.test.ts index 0da50481d..33df092e0 100644 --- a/server/tests/unit/nest/oidc.service.test.ts +++ b/server/tests/unit/nest/oidc.service.test.ts @@ -48,9 +48,9 @@ vi.mock('../../../src/config', () => ({ })); // Construction insurance for the injected AuthService's import graph (same set -// as auth.service.test.ts) — services/tripMembership is deliberately NOT -// mocked: OIDC-SVC-045 asserts the real trip_members row a trip-bound invite -// creates. +// as auth.service.test.ts) — TripMembershipService is deliberately the real one, +// not a stub: OIDC-SVC-045 asserts the actual trip_members row a trip-bound +// invite creates. vi.mock('../../../src/nest/common/crypto/mfaCrypto', () => ({ encryptMfaSecret: vi.fn((s) => `enc:${s}`), decryptMfaSecret: vi.fn((s: string) => s.replace('enc:', '')), @@ -91,15 +91,18 @@ import { createUser, createTrip } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { AtlasService } from '../../../src/nest/atlas/atlas.service'; +import { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.service'; import { AuthService } from '../../../src/nest/auth/auth.service'; import { OidcService } from '../../../src/nest/oidc/oidc.service'; +const membership = new TripMembershipService(new DatabaseService(testDb)); const auth = new AuthService( new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new AtlasService(new DatabaseService(testDb)), + membership, ); -const svc = new OidcService(new DatabaseService(testDb), auth); +const svc = new OidcService(new DatabaseService(testDb), auth, membership); const MOCK_CONFIG = { issuer: 'https://oidc.example.com', diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts index e32d63ba3..b19888fe6 100644 --- a/server/tests/unit/nest/places.service.test.ts +++ b/server/tests/unit/nest/places.service.test.ts @@ -67,13 +67,14 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { PlacesService } from '../../../src/nest/places/places.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { PLACE_IMAGES_DIR } from '../../../src/services/placeImage'; const GPX_FIXTURE = path.join(__dirname, '../../fixtures/test.gpx'); const KML_FIXTURE = path.join(__dirname, '../../fixtures/test.kml'); const dbs = new DatabaseService(testDb); -const svc = new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), new MapsService(dbs)); +const svc = new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), new MapsService(dbs), new QueryHelpersService(dbs)); beforeAll(() => { createTables(testDb); @@ -1011,7 +1012,7 @@ describe('PlacesService — automatic track colours (#776)', () => { describe('enrichImportedPlaces', () => { function enrichSvc(maps: Partial) { - return new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), maps as MapsService); + return new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), maps as MapsService, new QueryHelpersService(dbs)); } it('PLACE-SVC-058 — no-ops when no Google Maps key is configured', async () => { diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index 271d270bc..01a460476 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -56,9 +56,10 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import type { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { ShareService } from '../../../src/nest/share/share.service'; import { SettingsService } from '../../../src/nest/settings/settings.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import type { User } from '../../../src/types'; -const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb)), permissionsStub); +const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb)), permissionsStub, new QueryHelpersService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/services/tripMembership.test.ts b/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts similarity index 75% rename from server/tests/unit/services/tripMembership.test.ts rename to server/tests/unit/nest/trip-membership/trip-membership.service.test.ts index c5f254bfe..eae424bb4 100644 --- a/server/tests/unit/services/tripMembership.test.ts +++ b/server/tests/unit/nest/trip-membership/trip-membership.service.test.ts @@ -1,5 +1,5 @@ /** - * Unit tests for joinTripAsMember — TRIP-JOIN-001..004. + * Unit tests for TripMembershipService.joinTripAsMember — TRIP-JOIN-001..004. * The shared add-by-id helper behind trip invite links (#1143) and trip-bound * admin invites (#1402): idempotent, owner-safe, missing-trip-safe. */ @@ -11,13 +11,17 @@ const { testDb, dbMock } = vi.hoisted(() => { db.exec('PRAGMA foreign_keys = ON'); return { testDb: db, dbMock: { db } }; }); -vi.mock('../../../src/db/database', () => dbMock); +vi.mock('../../../../src/db/database', () => dbMock); -import { createTables } from '../../../src/db/schema'; -import { runMigrations } from '../../../src/db/migrations'; -import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip } from '../../helpers/factories'; -import { joinTripAsMember } from '../../../src/services/tripMembership'; +import { createTables } from '../../../../src/db/schema'; +import { runMigrations } from '../../../../src/db/migrations'; +import { resetTestDb } from '../../../helpers/test-db'; +import { createUser, createTrip } from '../../../helpers/factories'; +import { DatabaseService } from '../../../../src/nest/database/database.service'; +import { TripMembershipService } from '../../../../src/nest/trip-membership/trip-membership.service'; + +const joinTripAsMember = (tripId: number, userId: number, invitedBy: number | null) => + new TripMembershipService(new DatabaseService(testDb)).joinTripAsMember(tripId, userId, invitedBy); beforeAll(() => { createTables(testDb); runMigrations(testDb); }); beforeEach(() => resetTestDb(testDb)); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 54803c559..888a2e80a 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -66,14 +66,15 @@ import { TripsService } from '../../../src/nest/trips/trips.service'; import { PlacesService } from '../../../src/nest/places/places.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { getTripOwner, listMembers as bridgeListMembers } from '../../../src/nest/trips/trips.bridge'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import fs from 'fs'; // Real sibling services over the same in-memory DB — updateTrip's date-shift // resyncs and the summary/bundle aggregation run their actual SQL. const dbs = () => new DatabaseService(testDb); const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService()); -const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService()); -const placesSvc = new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs())); +const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new QueryHelpersService(dbs())); +const placesSvc = new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs()), new QueryHelpersService(dbs())); const createAccommodation = daysSvc.createAccommodation.bind(daysSvc); const svc = new TripsService( 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 bd1e794a6..2a5e5591e 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -205,9 +205,10 @@ const collabStub = { createMessage: vi.fn((tid: number, uid: number, text: string) => (text === 'toolong' ? { error: 'Message too long' } : { message: { id: 142, trip_id: Number(tid), user_id: uid, text } })), } as unknown as CollabService; -vi.mock('../../../src/services/tripMembership', () => ({ +// Injected stub since the membership fold (was a path mock of services/tripMembership). +const membershipStub = { joinTripAsMember: vi.fn((tripId: number, userId: number) => ({ joined: userId !== 5, tripId })), // owner add = no-op -})); +} as unknown as TripMembershipService; // 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); @@ -327,6 +328,7 @@ 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 type { TripMembershipService } from '../../../src/nest/trip-membership/trip-membership.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'; @@ -349,7 +351,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, notificationsStub); +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, membershipStub); 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/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index 3e3bcd93b..26a88fd53 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -45,9 +45,10 @@ import { PlacesService } from '../../../src/nest/places/places.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; +import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; const packing = new PackingService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); -const places = new PlacesService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new MapsService(new DatabaseService(testDb))); +const places = new PlacesService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new MapsService(new DatabaseService(testDb)), new QueryHelpersService(new DatabaseService(testDb))); beforeAll(() => { createTables(testDb);